You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "Fabian-Schneider01 (via GitHub)" <gi...@apache.org> on 2026/07/04 15:36:33 UTC

[PR] [SPARK-57922][SQL] Migrate pipeline-dataset parser guards off _LEGACY_ERROR_TEMP_0035 to named error conditions) [spark]

Fabian-Schneider01 opened a new pull request, #57001:
URL: https://github.com/apache/spark/pull/57001

   ### What changes were proposed in this pull request?
   
   Migrate the pipeline-dataset guards in `SparkSqlParser.visitCreatePipelineDataset` off the legacy `_LEGACY_ERROR_TEMP_0035` ("Operation not allowed: ...") error onto named error conditions. Six new sub-conditions are added under `UNSUPPORTED_FEATURE` (SQLSTATE `0A000`), following the existing `UNSUPPORTED_FEATURE.CREATE_PIPELINE_DATASET_QUERY_EXECUTION` precedent, each with a `pipelineDatasetType` message parameter (`MATERIALIZED VIEW` or `STREAMING TABLE`):
   
   - `PIPELINE_DATASET_BUCKETING`
   - `PIPELINE_DATASET_COLUMN_CONSTRAINTS`
   - `PIPELINE_DATASET_HIVE_SERDE`
   - `PIPELINE_DATASET_LOCATION`
   - `PIPELINE_DATASET_OPTIONS`
   - `PIPELINE_DATASET_STORED_AS`
   
   The guards now throw via new `QueryParsingErrors` helpers instead of `ParserUtils.operationNotAllowed`.
   
   Scope notes:
   - The column-constraints guard is included as well: it lives in the same method, has the same shape, and previously raised the same legacy condition.
   - The JIRA also mentions the MATERIALIZED-VIEW-with-AUTO-CDC guard and the AUTO CDC streaming-source check; those guards are introduced by the still-open PR #56419 (SPARK-56249) and can adopt this pattern when they land.
   - The "Unable to find query for CREATE ..." guard in the same method is intentionally left unchanged: a missing `AS` query is a malformed statement rather than an unsupported feature, so it deserves a differently-shaped condition and can be handled separately.
   
   ### Why are the changes needed?
   
   Per `common/utils/src/main/resources/error/README.md`, user-facing errors should carry a proper error condition and SQLSTATE rather than a `_LEGACY_ERROR_TEMP_*` placeholder. These guards are user-reachable from pipeline SQL definitions, and named conditions make the errors machine-identifiable via `getCondition()` and consistent with the other declarative-pipelines error conditions.
   
   ### Does this PR introduce _any_ user-facing change?
   
   Yes, but only for the declarative pipelines SQL syntax that has not shipped in a release yet (master only): the error condition and message for invalid `CREATE MATERIALIZED VIEW` / `CREATE STREAMING TABLE` pipeline statements change.
   
   Before:
   
   ```
   [_LEGACY_ERROR_TEMP_0035] Operation not allowed: Specifying location is not supported for CREATE STREAMING TABLE statements. The storage location for a pipeline dataset is managed by the pipeline itself.(line 1, pos 0)
   ```
   
   After:
   
   ```
   [UNSUPPORTED_FEATURE.PIPELINE_DATASET_LOCATION] The feature is not supported: The LOCATION clause in a CREATE STREAMING TABLE statement. The storage location for a pipeline dataset is managed by the pipeline itself. SQLSTATE: 0A000(line 1, pos 0)
   ```
   
   ### How was this patch tested?
   
   Updated the existing tests to check the new conditions with `checkError()`:
   - `CreatePipelineDatasetAsSelectParserSuiteBase` (exercised for both dataset types via `CreateMaterializedViewAsSelectParserSuite` and `CreateStreamingTableAsSelectParserSuite`)
   - `SqlPipelineSuite` ("Setting dataset location is disallowed") migrated from a message-substring assertion to `checkError()`
   
   Ran locally:
   
   ```
   build/sbt 'core/testOnly *SparkThrowableSuite' \
     'sql/testOnly *CreateMaterializedViewAsSelectParserSuite *CreateStreamingTableAsSelectParserSuite' \
     'pipelines/testOnly *SqlPipelineSuite -- -z "location is disallowed"'
   ```
   
   All passed.
   
   ### Was this patch authored or co-authored using generative AI tooling?
   
   Generated-by: Claude Code (claude-fable-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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-57922][SQL] Migrate pipeline-dataset parser guards off _LEGACY_ERROR_TEMP_0035 to named error conditions [spark]

Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #57001:
URL: https://github.com/apache/spark/pull/57001#discussion_r3524871491


##########
common/utils/src/main/resources/error/error-conditions.json:
##########
@@ -8402,6 +8402,36 @@
           "Invalid partitioning: <cols> is missing or is in a map or array."
         ]
       },
+      "PIPELINE_DATASET_BUCKETING" : {
+        "message" : [
+          "Bucketing in a CREATE <pipelineDatasetType> statement. Remove the bucket specification from the statement."
+        ]
+      },
+      "PIPELINE_DATASET_COLUMN_CONSTRAINTS" : {
+        "message" : [
+          "Column constraints in a CREATE <pipelineDatasetType> statement. Remove the CHECK, UNIQUE, PRIMARY KEY and FOREIGN KEY constraints from the statement."
+        ]
+      },
+      "PIPELINE_DATASET_HIVE_SERDE" : {
+        "message" : [
+          "Hive SerDe format options in a CREATE <pipelineDatasetType> statement."

Review Comment:
   Nit: this is the only new condition omitting a remediation hint, so it's slightly less actionable than the other five. A one-liner like "Use the USING clause or a non-SerDe format instead." would align it better with the other ones.



-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-57922][SQL] Migrate pipeline-dataset parser guards off _LEGACY_ERROR_TEMP_0035 to named error conditions [spark]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on PR #57001:
URL: https://github.com/apache/spark/pull/57001#issuecomment-6227448655

   We're closing this PR because it hasn't been updated in a while. This isn't a judgement on the merit of the PR in any way. It's just a way of keeping the PR queue manageable.
   If you'd like to revive this PR, please reopen it and ask a committer to remove the Stale tag!


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-57922][SQL] Migrate pipeline-dataset parser guards off _LEGACY_ERROR_TEMP_0035 to named error conditions [spark]

Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] closed pull request #57001: [SPARK-57922][SQL] Migrate pipeline-dataset parser guards off _LEGACY_ERROR_TEMP_0035 to named error conditions
URL: https://github.com/apache/spark/pull/57001


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


Re: [PR] [SPARK-57922][SQL] Migrate pipeline-dataset parser guards off _LEGACY_ERROR_TEMP_0035 to named error conditions [spark]

Posted by "Fabian-Schneider01 (via GitHub)" <gi...@apache.org>.
Fabian-Schneider01 commented on PR #57001:
URL: https://github.com/apache/spark/pull/57001#issuecomment-4912216279

   Thanks for the feedback @uros-b @szehon-ho. I've pushed b50c0e9 which adds a remediation hint to PIPELINE_DATASET_HIVE_SERDE, bringing it in line with the other five conditions.


-- 
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: reviews-unsubscribe@spark.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org