LuciferYang opened a new pull request, #58333:
URL: https://github.com/apache/spark/pull/58333

   ### What changes were proposed in this pull request?
   
   This PR assigns proper names to two placeholder error conditions, both with 
SQLSTATE `42601`:
   
   | legacy condition | new condition |
   |---|---|
   | `_LEGACY_ERROR_TEMP_1166` | `BUCKET_COLUMN_IN_PARTITION_COLUMNS` |
   | `_LEGACY_ERROR_TEMP_1167` | `BUCKET_SORT_COLUMN_IN_PARTITION_COLUMNS` |
   
   Both message templates are kept word for word, and the two builders keep 
their Scala method names, so the throw sites in 
`PreprocessTableCreation.normalizeCatalogTable` stay out of the diff. The one 
existing test that covers both conditions, `BucketedWriteSuite`'s "write 
bucketed data with the overlapping bucketBy/sortBy and partitionBy columns", 
switches to the new names and now pins the SQLSTATE as well.
   
   ### Why are the changes needed?
   
   This continues the work of replacing `_LEGACY_ERROR_TEMP_*` placeholders 
with real condition names. Both of these are reachable from user code: 
`normalizeCatalogTable` runs for `CREATE TABLE`, CTAS and 
`df.write.partitionBy(...).bucketBy(...).saveAsTable(...)`, and either error 
fires when a column is named in both the partition spec and the bucket spec. A 
proper name is the right outcome here, not an internal error.
   
   The names follow how this file already splits the vocabulary: `BUCKET_*` 
when it means the object, `BUCKETING` when it means the feature or the clause. 
Hence `BUCKET_COLUMN` and `BUCKET_SORT_COLUMN`, alongside 
`INVALID_BUCKET_COLUMN_DATA_TYPE` and `CANNOT_ALTER_COLLATION_BUCKET_COLUMN` on 
one side and `SORT_BY_WITHOUT_BUCKETING` and `SPECIFY_BUCKETING_IS_NOT_ALLOWED` 
on the other. `BucketSpec` calls the fields `bucketColumnNames` and 
`sortColumnNames`. The `X_IN_Y` shape follows 
`STATIC_PARTITION_COLUMN_IN_INSERT_COLUMN_LIST`, and 48 existing non-legacy 
conditions carry `_IN_`. Both stay top-level rather than becoming an umbrella 
with two subclasses, matching the pair just above them in the same file, 
`SPECIFY_PARTITION_IS_NOT_ALLOWED` and `SPECIFY_BUCKETING_IS_NOT_ALLOWED`: one 
rule, one SQLSTATE, two names, differing only in which clause tripped it.
   
   For the SQLSTATE, the five closest siblings all use `42601`: 
`SORT_BY_WITHOUT_BUCKETING`, `SPECIFY_BUCKETING_IS_NOT_ALLOWED`, 
`SPECIFY_PARTITION_IS_NOT_ALLOWED`, `INVALID_BUCKET_COLUMN_DATA_TYPE` and 
`PARTITION_BY_NOT_ALLOWED_WITH_INSERT_INTO` (44 conditions use `42601` before 
this PR). `42713`, "a duplicate object was detected in a list", is the obvious 
alternative and does not fit: nothing is duplicated within either list, and 
that reading of this code path already belongs to `COLUMN_ALREADY_EXISTS` 
(`42711`), which `SchemaUtils.checkColumnNameDuplication` throws from the same 
function a few lines later. What is wrong here is the `CLUSTERED BY` clause 
given the `PARTITIONED BY` clause, which is what `42601` describes.
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes, in the rendered message. The sentence itself does not change, but a 
named condition gains the `[CONDITION] ` prefix and the ` SQLSTATE: ...` suffix 
that `SparkThrowableHelper.formatErrorMessage` suppresses for 
`_LEGACY_ERROR_`-prefixed names:
   
   ```
   -Bucketing column 'j' should not be part of partition columns 'i, j'.
   +[BUCKET_COLUMN_IN_PARTITION_COLUMNS] Bucketing column 'j' should not be 
part of partition columns 'i, j'. SQLSTATE: 42601
   ```
   
   The two conditions also start showing up in the generated error conditions 
documentation, which skips `_LEGACY_ERROR_*` entries.
   
   ### How was this patch tested?
   
   `BucketedWriteSuite`'s existing assertions for both conditions now check the 
new names plus `sqlState = "42601"`. Ran `core/testOnly 
org.apache.spark.SparkThrowableSuite` (37 tests) and `sql/testOnly 
*BucketedWriteWithoutHiveSupportSuite` (15 tests), all passing. Each new 
SQLSTATE assertion was verified to bite by temporarily setting the JSON value 
to `42602` and watching the test go red.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (Opus 5)
   


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

Reply via email to