[SPARK-58945][SQL] Fix mismatched messageParameters keys that cause INTERNAL_ERROR

[subhramit]

Closes SPARK-58945

Note that the following description was initially generated using GPT 5.4, and then edited by me.

What changes were proposed in this pull request?

This fixes several broken Spark error-reporting paths caused by mismatches between messageParameters keys and the placeholders declared in error-conditions.json.

The changes fall into two groups:

1. Fix Scala-side parameter key mismatches

   • sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryCompilationErrors.scala
     • _LEGACY_ERROR_TEMP_2450: use clazz instead of invalidClass
   • sql/catalyst/src/main/scala/org/apache/spark/sql/errors/QueryExecutionErrors.scala
     • INVALID_PARAMETER_VALUE.EXTENSION: use invalidValue instead of fileExtension / acceptable
     • INVALID_WRITER_COMMIT_MESSAGE: use detail instead of details
   • sql/core/src/main/scala/org/apache/spark/sql/execution/streaming/state/StateStoreErrors.scala
     • STATE_STORE_COLUMN_FAMILY_SCHEMA_INCOMPATIBLE: use colFamilyName / oldSchema / newSchema
   • sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveCursors.scala and sql/core/src/main/scala/org/apache/spark/sql/execution/command/v2/CursorCommandUtils.scala
     • CURSOR_OUTSIDE_SCRIPT: declare and populate cursorName consistently
   • sql/core/src/main/scala/org/apache/spark/sql/jdbc/H2Dialect.scala
     • TABLE_OR_VIEW_NOT_FOUND: add the missing searchPath parameter using the shared empty-search-path rendering from NoSuchItemExceptionHelper.formatSearchPath(Seq.empty), and fall back to oldName when tableName is absent so missing-source rename errors render correctly
   • sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/parser/AstBuilder.scala
     • PARSE_SYNTAX_ERROR: provide the required error / hint parameters for invalid semi-structured extract paths
   • sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/ApproxCountDistinctForIntervals.scala
     • DATATYPE_MISMATCH.NON_FOLDABLE_INPUT: provide the missing inputExpr parameter

2. Fix one duplicated JSON message template

   • common/utils/src/main/resources/error/error-conditions.json
   • _LEGACY_ERROR_TEMP_3069 and _LEGACY_ERROR_TEMP_3070 had byte-identical message templates.
   • _LEGACY_ERROR_TEMP_3069 is the reserved-column-name collision case and its message is correct.
   • _LEGACY_ERROR_TEMP_3070 is the unrecognized file metadata field fallback, and its Scala call site (Map("field" -> field.toString)) was already correct.

This PR fixes only the template for _LEGACY_ERROR_TEMP_3070, restoring the pre-error class-migration wording: Unrecognized file metadata field: <field>. This is the text the branch threw before the error-class migration in SPARK-46351, still visible at the v3.5.0 tag.

A minimal repro for one affected path is:

spark.range(1).write.option("extension", "12").csv(path)

Before this change, that path fails during message rendering and surfaces INTERNAL_ERROR instead of INVALID_PARAMETER_VALUE.EXTENSION.

Why are the changes needed?

Spark's StringSubstitutor defaults to enableUndefinedVariableException = true, so an unresolved placeholder throws IllegalArgumentException, which ErrorClassesJSONReader converts into SparkException.internalError.

Error messages are rendered eagerly when constructing many exceptions. The relevant exception constructors call SparkThrowableHelper.getMessage during construction (example). When substitution fails, the intended exception instance is never created, so the affected paths return INTERNAL_ERROR in place of the actual diagnosis.

These are user-visible diagnostics bugs rather than behavior changes in the main execution path.

One of the fixed paths is not demonstrated by an end-to-end repro today. stateStoreColumnFamilyMismatch currently has no callers, so that path is latent. It would have failed during message construction before this fix, so the factory is covered directly by a unit test. This PR only fixes its parameter mapping so the path is correct if the transformWithState side wires it up in follow-up work.

The H2Dialect.scala:230 change is covered through the direct renameTable(...) API path. Missing-table loadTable is intercepted earlier by JDBCRDD.resolveTable(...), so that path does not exercise the dialect branch.

For _LEGACY_ERROR_TEMP_3070 the failure mode differs: the Scala call site was already correct, but the JSON template was duplicated from _LEGACY_ERROR_TEMP_3069 during the error-class migration in SPARK-46351, so the fallback reports a reserved-column-name collision that never occurred.

Does this PR introduce any user-facing change?

Yes.

This PR fixes user-facing error reporting for the currently reachable affected paths on master, and also fixes one currently latent path.

Examples:

• Invalid CSV writer extension values now report INVALID_PARAMETER_VALUE.EXTENSION instead of failing during message construction.
• Invalid writer commit message counts now report INVALID_WRITER_COMMIT_MESSAGE with the intended detail text.
• CURSOR_OUTSIDE_SCRIPT now reports the cursor name in the diagnostic.
• _LEGACY_ERROR_TEMP_3070 now reports Unrecognized file metadata field: <field> instead of the unrelated reserved-column-name message.

The H2 dialect change is a correctness fix in the JDBC classification path and is covered through the direct renameTable(...) API path. The state store fix is latent today because stateStoreColumnFamilyMismatch currently has no callers.

This does not change the semantics of the underlying operations. It fixes the diagnostics that Spark surfaces when those error paths are hit.

How was this patch tested?

Added/updated regression tests:

• QueryCompilationErrorsSuite
• QueryExecutionErrorsSuite
• JDBCTableCatalogSuite
• FileSourceCustomMetadataStructSuite
• CursorCommandUtilsSuite
• SqlScriptingCursorE2eSuite
• StateStoreErrorsSuite
• ExpressionParserSuite
• ApproxCountDistinctForIntervalsSuite

Executed:

build/sbt "core/testOnly *SparkThrowableSuite"
build/sbt "sql/testOnly *QueryCompilationErrorsSuite *QueryExecutionErrorsSuite"
build/sbt "sql/testOnly *JDBCTableCatalogSuite *FileSourceCustomMetadataStructSuite"
build/sbt "sql/testOnly *CursorCommandUtilsSuite *SqlScriptingCursorE2eSuite *StateStoreErrorsSuite"
build/sbt "catalyst/testOnly *ExpressionParserSuite *ApproxCountDistinctForIntervalsSuite"
./dev/scalastyle
./dev/run-tests

and all of them passed.

Also performed manual repro -

• On main:fa6f713:

• On this branch:

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

Generated-by: Claude Opus 5 + GPT-5.4 (Zed coding agent) + manually driven, and touched up in IntelliJ IDEA

[uros-b]

Thank you @subhramit!

[subhramit]

Thank you @subhramit!

Thank you for the quick review :)

I will be filing a small follow-up after this for SPARK-58946 (currently this doesn't surface because due to INTERNAL_ERROR that was fixed here).

Update - filed as #58226.

[subhramit]

@uros-b hey, I wanted to know if Spark has any merge cycles after which approved PRs go in, as the suite of #58226 depends on this.

[uros-b]

All parameter keys match their templates, and regression coverage is appropriate, added @cloud-fan to also take a look

[subhramit]

@uros-b this now has two approvals.
Bumping again for @cloud-fan

[subhramit]

@LuciferYang hey, adding you in case this can be merged as the PR has now reached page 5.
More so because:

I wanted to know if Spark has any merge cycles after which approved PRs go in, as the suite of #58226 depends on this.

[LuciferYang]

@LuciferYang hey, adding you in case this can be merged as the PR has now reached page 5. More so because:

I wanted to know if Spark has any merge cycles after which approved PRs go in, as the suite of #58226 depends on this.

I will take a look at this PR during today's working hours.

[subhramit]

I will take a look at this PR during today's working hours.

Sure, thank you!

[LuciferYang]

Went through all six sites; the direction checks out and each rename matches its template. Since the old keys always failed to render, no caller could have been reading them, so the renames carry no compatibility risk.

Two things I would fold into this PR: the same mismatch is still open on CURSOR_OUTSIDE_SCRIPT, where ResolveCursors.scala:85 and CursorCommandUtils.scala:41 both pass a cursorName the template does not declare, and the new H2Dialect comment states the opposite of what the surrounding code shows. The rest of the comments are non-blocking.

[subhramit]

Hi guys, I know everyone's quite busy, and I understand as I maintain multiple projects myself.
But can someone please take a look or at least tag someone who can? It would really help carry the work forward. Especially because one of my other PRs depends on this.

[uros-b]

Thank you @subhramit, I don't have any additional concerns here - but let's wait a bit more for @LuciferYang's re-review

[LuciferYang]

AstBuilder.scala:4542 is one more instance of the same defect. new ParseException(errorClass = "PARSE_SYNTAX_ERROR", ctx = ctx) takes the two-arg overload (parsers.scala:200), which defaults messageParameters to Map.empty, while the template needs <error> and <hint>. The throw happens inside the constructor, so the object raised is a SparkException, not a ParseException, and none of the three catch branches in AbstractParser.parse match. The user gets a bare INTERNAL_ERROR with no SQL position instead of a syntax error. SELECT c:[''] FROM t reaches it: JsonPathParser's named rule requires a non-empty name between [' and '], so the path fails to parse.

A sweep anchored on the Map(...) literal next to errorClass cannot see this one, because the line has no Map on it. Either add error/hint here too, or file a separate JIRA for it.

[subhramit]

AstBuilder.scala:4542 is one more instance of the same defect. new ParseException(errorClass = "PARSE_SYNTAX_ERROR", ctx = ctx) takes the two-arg overload (parsers.scala:200), which defaults messageParameters to Map.empty, while the template needs <error> and <hint>. The throw happens inside the constructor, so the object raised is a SparkException, not a ParseException, and none of the three catch branches in AbstractParser.parse match. The user gets a bare INTERNAL_ERROR with no SQL position instead of a syntax error. SELECT c:[''] FROM t reaches it: JsonPathParser's named rule requires a non-empty name between [' and '], so the path fails to parse.

A sweep anchored on the Map(...) literal next to errorClass cannot see this one, because the line has no Map on it. Either add error/hint here too, or file a separate JIRA for it.

Yep, had missed that one. fixed in the latest commit by wiring PARSE_SYNTAX_ERROR with the required error/hint parameters and also added a parser regression test.

[LuciferYang]

The new "invalid semi-structured extract path" test expects one extra single quote: for input c:[''], '' lexes as one empty STRING_LITERAL, ctx.path.getText is [''], so s"'$field'" yields '['']' (two inner quotes) while the test expects '[''']' (three). The catalyst CI job on the head commit already fails on exactly this test: parameters: expected Map(error -> '[''']', hint -> ) but got HashMap(hint -> , error -> '['']').

Changing the expectation to "'['']'" fixes it.

[subhramit]

fixed in 856cc11
Should have run it locally or looked at the ci. Thanks

[LuciferYang]

856cc11 fixes one of the two mismatches, but the test still fails on the new head's CI: the full old-CI failure reads checkError found 2 mismatch(es), and the second one, queryContext.length: expected 0 but got 1, is still there. The ParseException carries one SQL QueryContext by default (fragment c:[''], 0 to 5), while this checkError passes no queryContext, so it compares against an empty array and the lengths can never match.

Adding queryContext = Array(ExpectedContext("c:['']", 0, 5)) to the checkError call makes it pass. My earlier comment only looked at the parameters line and missed this one, sorry about that.

[subhramit]

Yes figured that, this is now fixed

[subhramit]

Somehow the test still fails. Let me take a deeper look.

[LuciferYang]

One more reachable site of the same bug class, in sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/ApproxCountDistinctForIntervals.scala (~line 88): checkInputDataTypes raises DATATYPE_MISMATCH.NON_FOLDABLE_INPUT when the endpoints expression is not foldable, but passes only inputName/inputType while the template also needs <inputExpr>, and the dataTypeMismatch wrapper only adds sqlExpr. The result is INTERNAL_ERROR at analysis time instead of the intended error. Pre-existing code, not introduced here, but the same bug class this PR fixes.

Adding "inputExpr" -> toSQLExpr(endpointsExpression) covers it; the class already mixes in QueryErrorsBase and binByNonFoldableInputError already builds all four keys.

[subhramit]

One more reachable site of the same bug class, in sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/aggregate/ApproxCountDistinctForIntervals.scala (~line 88): checkInputDataTypes raises DATATYPE_MISMATCH.NON_FOLDABLE_INPUT when the endpoints expression is not foldable, but passes only inputName/inputType while the template also needs <inputExpr>, and the dataTypeMismatch wrapper only adds sqlExpr. The result is INTERNAL_ERROR at analysis time instead of the intended error. Pre-existing code, not introduced here, but the same bug class this PR fixes.

Adding "inputExpr" -> toSQLExpr(endpointsExpression) covers it; the class already mixes in QueryErrorsBase and binByNonFoldableInputError already builds all four keys.

Covered in c4237cf, test adapted in 9059389

[subhramit]

K8s integration test failed in https://github.com/subhramit/spark/actions/runs/33940800548/job/101240221054 but seemed unrelated to my changes, so re-triggered the CI with an empty commit.

Edit - I am unsure why this is happening every time now:

ERROR: failed to build: failed to inspect pulled image moby/buildkit:buildx-stable-1: Error response from daemon: 404 page not found

[subhramit]

K8s integration test failed in https://github.com/subhramit/spark/actions/runs/33940800548/job/101240221054 but seemed unrelated to my changes, so re-triggered the CI with an empty commit.

Edit - I am unsure why this is happening every time now:

ERROR: failed to build: failed to inspect pulled image moby/buildkit:buildx-stable-1: Error response from daemon: 404 page not found

Okay apparently this happened in other PRs as well and is indeed unrelated. See comment #58503 (comment).

[subhramit]

@LuciferYang CI is now green, could you take a final look?