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

[subhramit]

Closes SPARK-58945

Note that the following description was initially structured 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), fall back to oldName when tableName is absent so missing-source rename errors render correctly, and handle the related H2 object-not-found variants in the same branch
   • 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.

Also note that this 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.

Why are the changes needed?

Described in the linked ticket that I filed.

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.

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.

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?

1. Performed manual repro -

• On main:fa6f713:

• On this branch:

2. Added/updated regression tests:

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

and executed:

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

and all of them passed.

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

Assisted-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]

@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.

[LuciferYang]

CSVOptions.scala:127 reads ext.size != 3 && !ext.forall(_.isLetter), so both conditions must hold before it rejects anything. All-letter values like ab and toolong, and three-character values like 123 and a/b, all pass, while the message says the extension is limited to exactly 3 letters. The intent is clearly ||. The value is concatenated into the output file name (CSVFileFormat.scala:91), so a/b puts a path separator there.

That line is not part of this PR, and 12 is just one of the values the current condition does reject. Flipping && to || and adding an ab case is the cheap fix. If that is out of scope, a separate ticket plus a comment in the test saying which shapes are actually covered would do.

[subhramit]

Please see #58226 where I already covered this. That fix is based on this PR as otherwise only INTERNAL_ERROR was being surfaced.

For now, I clarified the test scope in 9a47641

[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.

[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

[LuciferYang]

Went through the parameter-key changes one by one and they all line up with their templates. I also swept every literal messageParameters = Map(...) sitting next to an errorClass = "..." under sql/, core/ and common/ main and found nothing left in either direction, neither a missing key nor an unused one; running the same script against the merge-base prints exactly the sites this PR fixes, so the empty result is not the script idling.

Seven comments below. Only the first is worth doing before merge: a top-level DECLARE ... CURSOR is the only way SQL reaches the CursorCommandUtils throw, and that path has no test. The rest are non-blocking. Two of them, the CURSOR_NOT_FOUND rendering and the H2 42103/42104 gap, are pre-existing code you happen to be standing next to; the others are wording and test placement. A follow-up JIRA is fine for any of them.

The && vs || extension predicate is already covered by #58226, so nothing is needed here for that.

[LuciferYang]

Dropping the template 3070 had copied from 3069 is right, and 3069 keeps the wording. What is left is the condition itself: error/README.md says You should not introduce new uncategorized errors. Instead, convert them to proper errors whenever encountering them in new code., and _LEGACY_ERROR_TEMP_3070 has no sqlState.

Reaching this throw site requires a third-party FileFormat that declares a metadata field without marking it constant or generated, since every built-in format marks them. That makes it an implementation error rather than a user error, so converting it to INTERNAL_ERROR fits better than minting a user-visible condition. Converting means touching the error-conditions.json entry and the new FileSourceCustomMetadataStructSuite case as well; if that widens this PR too far, naming the follow-up JIRA in the description would do, and the other comment about the <field> rendering then follows the same route.

[subhramit]

IMO it's better if we keep this PR scoped to fixing the broken existing path and leave reclassification of _LEGACY_ERROR_TEMP_3070 to a follow-up
I can raise a ticket

[subhramit]

Reason being, it will be semantic reclassification, not just a broken-rendering fix which I'm dealing with here

[subhramit]

@LuciferYang @uros-b @HyukjinKwon Thanks everyone for patiently reviewing!

Once this goes in, I will merge master in #58226 and trigger the CI.

[LuciferYang]

Merge Summary:

• merged into master 44db1eb
• merged into branch-4.x ee81278

Posted by merge_spark_pr.py

[LuciferYang]

Thank you @subhramit @HyukjinKwon and @uros-b