Ma77Ball opened a new pull request, #57384: URL: https://github.com/apache/spark/pull/57384
### What changes were proposed in this pull request? This PR converts the legacy error condition `_LEGACY_ERROR_TEMP_1058` into an internal error, because it is unreachable by users. The condition was raised in `ResolveSessionCatalog.getStorageFormatAndProvider` when a `CREATE TABLE` / `CREATE TABLE AS SELECT` plan carried both a data-source provider (`USING`) and Hive SerDe information (`STORED AS` / `ROW FORMAT`). That combination cannot actually reach analysis: - The SQL parser (`AstBuilder`) already rejects `USING` together with `STORED AS` / `ROW FORMAT` before analysis, throwing a user-facing parse error. - No non-SQL entry point populates SerDe information: `DataFrameWriterV2` and `Catalog.createTable` both set `serde = None`, and `SerdeInfo` is only ever produced by the SQL grammar. So the check is a defensive guard that can only fire if an invalid logical plan is constructed internally, which is a bug, not user input. Concretely, this PR: - Replaces the `throw QueryCompilationErrors.cannotCreateTableWithBothProviderAndSerdeError(...)` call in `ResolveSessionCatalog` with `throw SparkException.internalError(...)`. - Removes the `_LEGACY_ERROR_TEMP_1058` condition from `error-conditions.json`. - Removes the now-unused `cannotCreateTableWithBothProviderAndSerdeError` helper from `QueryCompilationErrors`. This is part of the ongoing effort under [SPARK-37935](https://issues.apache.org/jira/browse/SPARK-37935) to migrate `_LEGACY_ERROR_TEMP_*` conditions. ### Why are the changes needed? `_LEGACY_ERROR_TEMP_1058` is one of the placeholder conditions being migrated to proper error classes under SPARK-37935. Unlike a typical migration, this condition is not user-facing at all: the parser handles the invalid DDL first, so the branch is unreachable through any public API. Assigning it a user-facing name would misclassify an internal invariant as a user error. Converting it to `SparkException.internalError` labels it correctly (a Spark bug if it ever triggers) and removes one more `_LEGACY_ERROR_TEMP_*` from the backlog. ### Does this PR introduce _any_ user-facing change? No. Users cannot reach this code path. `CREATE TABLE ... USING ... STORED AS/ROW FORMAT ...` is rejected by the parser before analysis, so the user-visible behavior and error are unchanged. The converted throw is an internal-only guard. ### How was this patch tested? No new test is added, because the branch is unreachable from SQL and from every public API, so it cannot be triggered from a user-facing test. Existing coverage is unaffected: - `DDLParserSuite` already asserts that `CREATE TABLE` / `REPLACE TABLE ... USING ... STORED AS` / `ROW FORMAT ...` is rejected at parse time (the actual user-facing behavior). - Removing the condition keeps `error-conditions.json` consistent, as checked by `SparkThrowableSuite`. Both suites pass locally: `DDLParserSuite` (151 tests) and `SparkThrowableSuite` (34 tests). ### Was this patch authored or co-authored using generative AI tooling? <!-- If generative AI tooling has been used in the process of authoring this patch, please include the phrase: 'Generated-by: ' followed by the name of the tool and its version. If no, write 'No'. Please refer to the [ASF Generative Tooling Guidance](https://www.apache.org/legal/generative-tooling.html) for details. --> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
