[SPARK-58098][SQL] Convert _LEGACY_ERROR_TEMP_0033 to an internal error

[LuciferYang]

What changes were proposed in this pull request?

Convert the legacy error condition _LEGACY_ERROR_TEMP_0033 to a SparkException.internalError and remove it from error-conditions.json.

Why are the changes needed?

The error-conditions README disallows new _LEGACY_ERROR_TEMP_* entries and asks existing ones to be resolved. This resolves one of them.

The condition is raised in AstBuilder.visitCreateFileFormat, in the branch where both STORED AS and STORED BY are present. That branch is not reachable from normal parsing:

• The grammar rule createFileFormat is STORED AS fileFormat | STORED BY storageHandler, so one clause carries only one of the two.
• Two clauses (STORED AS ... STORED BY ...) are rejected earlier by checkDuplicateClauses as DUPLICATE_CLAUSES.

As it only signals an internal inconsistency, it does not need a user-facing error condition and is converted to an internal error, following precedents such as SPARK-42839.

One trade-off: SparkException.internalError has no overload that carries the parser context, so the ParseException position information (offending fragment, line/column) is no longer attached. This is acceptable here because the branch is unreachable from user queries.

Does this PR introduce any user-facing change?

No. The code path is unreachable from user queries, so the change is not observable in practice.

How was this patch tested?

build/sbt "core/testOnly org.apache.spark.SparkThrowableSuite" passes, covering the JSON validation gates (alphabetical ordering, mandatory SQLSTATE, round-trip). No test is added because the path is unreachable.

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Code (Opus 4.8)

[MaxGekk]

If it is an internal error, why can't you just throw SparkException.internalError?

[LuciferYang]

Good point, done. I've switched it to SparkException.internalError and removed the condition from error-conditions.json.

One thing worth flagging: internalError has no overload that takes the parser context, so the ParseException position info (offending fragment, line/column) is dropped. Since this branch is unreachable from normal parsing — the grammar allows only one of STORED AS / STORED BY, and a duplicated clause is already caught earlier as DUPLICATE_CLAUSES — losing the context has no user-facing impact here. Let me know if you'd prefer to keep the context anyway.

[MaxGekk]

LGTM. This implements the SparkException.internalError conversion from the earlier thread.

I verified the case _ branch is not reachable via any public API, not just "normal parsing":

• visitCreateFileFormat has a single definition (AstBuilder.scala:5599) overriding the generated ANTLR visitor — no subclass (incl. SparkSqlParser) reimplements it.
• Its only caller is createFileFormatCtx.map(visitCreateFileFormat) (AstBuilder.scala:5779); the contexts come from the grammar rule createFileFormat : STORED AS fileFormat | STORED BY storageHandler (SqlBaseParser.g4:726-729), a strict alternation, so a single CreateFileFormatContext can never carry both children.
• The statement-level "both clauses" case is caught earlier by checkDuplicateClauses(ctx.createFileFormat, "STORED AS/BY", ctx) → DUPLICATE_CLAUSES (AstBuilder.scala:5733), before the visitor runs.
• AstBuilder/getSerdeInfo are internal, so there's no programmatic path that hands a synthetic context to the visitor.

So it can only fire on a violated grammar invariant — internalError is the right call, and dropping the parser position info has no user-facing impact. The removal is complete (no dangling _LEGACY_ERROR_TEMP_0033 references, alphabetical order preserved, sole caller updated) and consistent with the existing internalError precedent at QueryParsingErrors.scala:44.

[MaxGekk]

+1, LGTM. Merging to master/4.x.
Thank you, @LuciferYang.