[SPARK-57402][SQL][SDP] Register AUTO CDC SQL syntax with the dataflow graph - #57176
[SPARK-57402][SQL][SDP] Register AUTO CDC SQL syntax with the dataflow graph#57176anew wants to merge 6 commits into
Conversation
### What changes were proposed in this pull request? Hook up the two AUTO CDC SQL constructs introduced in SPARK-56249 to the Spark Declarative Pipelines dataflow graph in `SqlGraphRegistrationContext`: 1. `CREATE STREAMING TABLE <name> FLOW AUTO CDC FROM <source> ...`, parsed into `CreateStreamingTableAutoCdc`, now registers the streaming table and an `AutoCdcFlow` that targets it. 2. `CREATE FLOW <name> AS AUTO CDC INTO <target> FROM <source> ...`, parsed into a `CreateFlowCommand` wrapping an `AutoCdcIntoCommand`, now registers an `AutoCdcFlow` from the named flow into the target dataset. A shared `buildChangeArgs` helper converts the parse-time expressions and unresolved attributes into the `ChangeArgs` consumed by `AutoCdcFlow` (keys, sequencing, delete condition, include/exclude column selection). SQL AUTO CDC only supports SCD Type 1, matching the Connect/proto path. ### Why are the changes needed? Before this change the AUTO CDC SQL syntax parsed successfully but was rejected during graph registration, so it could not be used to define pipeline datasets or flows. ### Does this PR introduce _any_ user-facing change? Yes. AUTO CDC SQL statements now register datasets and flows in a pipeline. ### How was this patch tested? Added registration and end-to-end execution tests to `SqlPipelineSuite` covering both syntaxes, the optional clauses (APPLY AS DELETE WHEN, COLUMNS, COLUMNS * EXCEPT), and the multipart-flow-name error. Full `SqlPipelineSuite` passes (50 tests). ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Opus 4.8 Co-authored-by: Isaac
| identifier = stIdentifier, | ||
| comment = cst.tableSpec.comment, | ||
| specifiedSchema = | ||
| Option.when(cst.columns.nonEmpty)(StructType(cst.columns.map(_.toV1Column))), |
There was a problem hiding this comment.
If users write CREATE STREAMING TABLE target (id INT, name STRING) FLOW AUTO CDC ..., this stores the listed columns as the table’s full specified schema. But Auto CDC later appends _cdc_metadata to the flow schema, so validation can reject an otherwise natural data-column-only declaration. Could we either reject column lists for this SQL form, or interpret them as the data schema and add the metadata field internally before validation?
There was a problem hiding this comment.
This is a very interesting question. What if the given schema does not match the keys+columns? That should be rejected, I guess. A similar case is where the the streaming table is declared without a flow but with a schema. In that case we do not know(at parse time) whether the flow that is also declared on its own is an Auto CDC flow? It appears to me that need to allow specifying the schema, but interpret it as exclusive of the metadata column.
There was a problem hiding this comment.
Thanks for catching this — this matches where I landed too, and I went and investigated both the current behavior and the "user specifies data-columns-only" approach. Two findings worth sharing:
- The two-statement form doesn't fail at validation today — it silently skips validation entirely. I expected this case (CREATE STREAMING TABLE target (id INT, name STRING) + separate CREATE FLOW ... AS AUTO CDC INTO target) to at least be caught by
validateUserSpecifiedSchemas, but it isn't. That check keys the table lookup on the flow's identifier:
flows.flatMap(f => table.get(f.identifier))
which only matches when the flow is an implicit/default flow (flow id == table id) — i.e. the combined form. For a named flow, table.get(f.identifier) is None, so the declared schema is never validated (its sibling validateFlowStreamingness correctly keys on the destination identifier). So the mismatch isn't even caught at graph-validation time for the two-statement pattern — it surfaces later as a mid-stream UNRESOLVED_COLUMN ... _cdc_metadata at materialization. That looks like a pre-existing bug independent of this PR, it pre-exists also for Python, and I think it deserves its own fix (keying on destinationIdentifier) + regression test.
- How to fix it? I propose to make a quick fix for both SQL and Python to use the destinationIdentifier for lookup, in a separate PR. Then in a separate Jira, we can discuss whether we want to allow an explicit schema that omits the metadata column, in all cases. This would be beyond the scope of this PR because it changes existing, released behavior in a non-regressing way, but a change nonetheless.
What do you think, @szehon-ho ?
There was a problem hiding this comment.
I created SPARK-58116 for the quick fix.
There was a problem hiding this comment.
... and SPARK-58118 for the longer-term data-columns-only approach.
| case createStreamingTableCommand: CreateStreamingTable => | ||
| // CREATE STREAMING TABLE [ streaming_table_name ] [ options ] | ||
| CreateStreamingTableHandler.handle(createStreamingTableCommand, queryOrigin) | ||
| case createStreamingTableAutoCdcCommand: CreateStreamingTableAutoCdc => |
There was a problem hiding this comment.
This registers CreateStreamingTableAutoCdc in the pipeline graph path, but the normal Spark execution guard in SparkStrategies.Pipelines only rejects CreateFlowCommand and CreateStreamingTableAsSelect today. If someone runs this statement through regular spark.sql outside pipeline registration, it may miss the curated unsupported STREAMING TABLE error. Can we add a matching CreateStreamingTableAutoCdc case there?
There was a problem hiding this comment.
I addressed this, and in the process discovered that the same issue existed for a blank CREATE STREAMING TABLE (without a flow), and also for the CREATE FLOW AS AUTO CDC. The latter actually resulted in a CreateFlow command with a child AutoCdcIntoCommand that was a command in itself. I realized that this should not be a command (because it can never occur by itself, only within a CREATE FLOW), so I changed this to be named just AutoCdcInto and not extending Command anymore. Also added tests for all these cases. I think this is cleaner now.
…peline ### What changes were proposed in this pull request? Extend the `Pipelines` planner strategy so that all pipeline-only dataset and flow commands fail with a friendly error when executed outside a pipeline: - `CreateStreamingTable` (the no-subquery variant) and `CreateStreamingTableAutoCdc` are now rejected alongside `CreateStreamingTableAsSelect`. - `CREATE FLOW ... AS AUTO CDC` is now rejected via the existing `CreateFlowCommand` path. Previously it produced an `INTERNAL_ERROR` because its flow operation, `AutoCdcIntoCommand`, was itself a `Command` and got visited on its own by the eager command-execution path, where no strategy matched it. To fix the root cause, `AutoCdcIntoCommand` is renamed to `AutoCdcInto` and changed from a `Command` to a plain `LogicalPlan`. It only ever appears as the flow operation of a `CreateFlowCommand` (there is no standalone `AUTO CDC INTO` syntax), mirroring how its sibling flow operation `InsertIntoStatement` is a parsed statement rather than a command. A plain `LogicalPlan` (rather than `ParsedStatement`, which forces `resolved = false`) preserves the normal resolution semantics the node relies on while ensuring it is never eagerly executed or planned on its own. ### Why are the changes needed? These commands are parse-time placeholders interpreted by the pipeline submodule. Executing them directly should produce a clear unsupported-operation error, not an internal error or a generic planning failure. ### Does this introduce _any_ user-facing change? No. This syntax is not yet executable outside pipelines; this change only improves the error surfaced for unsupported direct execution. ### How was this patch tested? New `DDLSuite` cases for direct execution of `CREATE STREAMING TABLE` (no subquery), `CREATE STREAMING TABLE ... FLOW AUTO CDC`, and `CREATE FLOW ... AS AUTO CDC`. Ran `AutoCdcParserSuite` and the pipeline suites (`SqlPipelineSuite`, `AutoCdcFlowSuite`, `AutoCdcScd1SinglePipelineSuite`, `ConnectValid/InvalidPipelineSuite`) to confirm the base-class change does not affect graph registration or execution. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Opus 4.8 Co-authored-by: Isaac
… DDL tests
### What changes were proposed in this pull request?
Rename the helper source table from `src` to `cdc_src` in the two AUTO CDC
`DDLSuite` tests. `HiveDDLSuite` extends the same abstract `DDLSuite`, and the
Hive-backed `TestHive` session pre-populates a standard `src` fixture table, so
`CREATE TABLE src ...` failed with `TABLE_OR_VIEW_ALREADY_EXISTS`.
### Why are the changes needed?
The tests failed under `HiveDDLSuite` and, via the `withTable("src")` cleanup,
would have dropped Hive's shared `src` fixture. Using a unique name fixes both.
### Does this introduce _any_ user-facing change?
No, test-only.
### How was this patch tested?
Ran the rejection tests in both `HiveDDLSuite` (6/6) and
`InMemoryCatalogedDDLSuite` (6/6).
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Opus 4.8
Co-authored-by: Isaac
szehon-ho
left a comment
There was a problem hiding this comment.
A few inline comments on SqlGraphRegistrationContext. Also: CreateStreamingTableAutoCdc in v2Commands.scala still has a scaladoc saying execution support is coming in SPARK-57402 — worth updating since this PR wires up registration (that file isn't in the diff so I couldn't anchor an inline comment there).
| Table( | ||
| identifier = stIdentifier, | ||
| comment = cst.tableSpec.comment, | ||
| specifiedSchema = |
There was a problem hiding this comment.
If the user writes CREATE STREAMING TABLE target (id INT, name STRING) FLOW AUTO CDC ..., we store the column list as specifiedSchema. But AutoCdcMergeFlow always appends _cdc_metadata to the flow output schema, so GraphValidations.validateUserSpecifiedSchemas later rejects the pipeline with USER_SPECIFIED_AND_INFERRED_SCHEMA_NOT_COMPATIBLE (specified = data cols only, inferred = data cols + _cdc_metadata). The parser allows this form (AutoCdcParserSuite), but the new E2E tests avoid it by omitting the column list.
Would it be better to reject it here instead of letting registration succeed and failing at graph validation? For example:
if (cst.columns.nonEmpty) {
throw SqlGraphElementRegistrationException(
msg = "Explicit column lists are not supported for AUTO CDC streaming tables; " +
"omit the column list and let schema be inferred from the source.",
queryOrigin = queryOrigin)
}That gives a clear, immediate error for the single-statement form. The tradeoff is it doesn't cover the two-statement pattern:
CREATE STREAMING TABLE target (id INT, name STRING);
CREATE FLOW f AS AUTO CDC INTO target ...where CreateStreamingTableHandler registers the schema and a later CREATE FLOW attaches AUTO CDC — rejection would need to live in validation (or we'd need to reject column lists on all standalone CREATE STREAMING TABLE statements, which is too broad).
Longer term, treating specifiedSchema as data-columns-only for AUTO CDC targets (and appending _cdc_metadata during validation/materialization) is probably the right UX. But if we want a small fix for this PR, rejecting non-empty cst.columns here seems reasonable for the combined syntax — WDYT?
There was a problem hiding this comment.
In the existing Python API, there is no combined command - you have to create the streaming table and the flow separately. But if you create the table with a schema, then the flow schema must match it exactly. So it will fail if you don't specify the metadata column. The same will happen in SQL if you create the streaming table and the flow separately.
I am not opposed to restricting the combined syntax as you suggest, but it does not solve the problem for separate declarations.
Let me follow your suggestion for now and disallow a schema in this case. I will follow up with a separate Jira for the data-columns-only approach.
| comment = cst.tableSpec.comment, | ||
| specifiedSchema = | ||
| Option.when(cst.columns.nonEmpty)(StructType(cst.columns.map(_.toV1Column))), | ||
| partitionCols = Option(PartitionHelper.applyPartitioning(cst.partitioning, queryOrigin)), |
There was a problem hiding this comment.
AutoCdcParserSuite has a test that CLUSTER BY is honored at parse time — it lands in cst.partitioning as a ClusterByTransform. But this call routes all of partitioning through PartitionHelper, which only accepts IdentityTransform (PARTITIONED BY) and throws Invalid partitioning transform (ClusterByTransform(...)) for CLUSTER BY. So the parser test and pipeline registration disagree.
Options:
- Split
ClusterByTransformout ofpartitioningintoclusterColshere (Connect already setsclusterColsfrom clustering columns). - Reject
CLUSTER BYexplicitly in this handler with a clear error, and adjust/remove the parser test expectation if we don't support it yet.
Same pattern exists in the other SQL table handlers (CreateStreamingTableHandler, etc.), but AUTO CDC is the one with a parser test claiming CLUSTER BY works.
| case _ => | ||
| throw SqlGraphElementRegistrationException( | ||
| msg = "Unable flow type. Only INSERT INTO flows are supported.", | ||
| msg = "Unable flow type. Only INSERT INTO and AUTO CDC INTO flows are supported.", |
There was a problem hiding this comment.
Nit: "Unable flow type..." → "Unknown flow type..." (or similar).
Address review feedback: "Unable flow type." -> "Unknown flow type." in the SqlGraphRegistrationContext error for an unrecognized flow operation. Co-authored-by: Isaac
### What changes were proposed in this pull request? The parser folds a `CLUSTER BY` clause into the command's `partitioning` sequence as a `ClusterByTransform` (alongside `PARTITIONED BY` identity transforms). All four SQL pipeline dataset handlers in `SqlGraphRegistrationContext` routed `partitioning` through `PartitionHelper.applyPartitioning`, which only accepts `IdentityTransform` and threw "Invalid partitioning transform (ClusterByTransform(...))" for `CLUSTER BY`, while hard-coding `clusterCols = None`. Replace `applyPartitioning` with `splitPartitionAndClusterColumns`, which separates the parsed transforms into partition columns and cluster columns, and wire both into the registered `Table` in all four handlers (`CreateStreamingTable`, `CreateStreamingTableAsSelect`, `CreateStreamingTableAutoCdc`, `CreateMaterializedViewAsSelect`). This mirrors how the Connect/Python path already populates `clusterCols`, and lets `DatasetManager`'s existing partition/cluster mutual-exclusion check apply. Note: this drops the old `t.references.length != 1` check, which was dead code -- `IdentityTransform` always has exactly one reference. The real multipart guard (`ref.fieldNames().length != 1`) is retained. ### Why are the changes needed? Addresses review feedback on PR apache#57176: the AUTO CDC parser test asserts `CLUSTER BY` is honored, but graph registration rejected it. The same gap affected all four SQL forms, so SQL lagged behind the Connect/Python path which already supports clustering. ### Does this introduce _any_ user-facing change? Yes: `CLUSTER BY` on `CREATE STREAMING TABLE` (with or without a query or AUTO CDC flow) and `CREATE MATERIALIZED VIEW` now registers cluster columns instead of failing registration. ### How was this patch tested? New `SqlPipelineSuite` test asserts `CLUSTER BY` populates `clusterCols` (and leaves `partitionCols` empty) for all four SQL forms; confirmed it fails without the fix with the exact "Invalid partitioning transform (cluster_by(...))" error. Full `SqlPipelineSuite` (51), `AutoCdcParserSuite` (50), and ConnectValid/Invalid + AutoCdc pipeline suites pass. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Opus 4.8 Co-authored-by: Isaac
…treaming table The combined `CREATE STREAMING TABLE <name> (cols...) FLOW AUTO CDC ...` form stored the user column list as `specifiedSchema`, but AutoCdcMergeFlow always appends a reserved metadata column to the inferred schema, so the pipeline later failed at graph validation with USER_SPECIFIED_AND_INFERRED_SCHEMA_NOT_COMPATIBLE. Reject the explicit column list at registration time with a clear error instead, and let the schema be inferred from the flow. The data-columns-only UX is deferred to a follow-up. Co-authored-by: Isaac
…w graph ### What changes were proposed in this pull request? Hook up the two AUTO CDC SQL constructs introduced in SPARK-56249 to the Spark Declarative Pipelines dataflow graph in `SqlGraphRegistrationContext`: 1. `CREATE STREAMING TABLE <name> FLOW AUTO CDC FROM <source> ...`, parsed into `CreateStreamingTableAutoCdc`, now registers the streaming table and an `AutoCdcFlow` that targets it. 2. `CREATE FLOW <name> AS AUTO CDC INTO <target> FROM <source> ...`, parsed into a `CreateFlowCommand` wrapping an `AutoCdcIntoCommand`, now registers an `AutoCdcFlow` from the named flow into the target dataset. A shared `buildChangeArgs` helper converts the parse-time expressions and unresolved attributes into the `ChangeArgs` consumed by `AutoCdcFlow` (keys, sequencing, delete condition, include/exclude column selection). SQL AUTO CDC only supports SCD Type 1, matching the Connect/proto path. ### Why are the changes needed? Before this change the AUTO CDC SQL syntax parsed successfully but was rejected during graph registration, so it could not be used to define pipeline datasets or flows. ### Does this PR introduce _any_ user-facing change? Yes. AUTO CDC SQL statements now register datasets and flows in a pipeline. ### How was this patch tested? Added registration and end-to-end execution tests to `SqlPipelineSuite` covering both syntaxes, the optional clauses (APPLY AS DELETE WHEN, COLUMNS, COLUMNS * EXCEPT), and the multipart-flow-name error. Full `SqlPipelineSuite` passes (50 tests). ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Opus 4.8 Closes #57176 from anew/hookup-sql-parser-with-dataflow-graph. Authored-by: Andreas Neumann <anew@apache.org> Signed-off-by: Szehon Ho <szehon.apache@gmail.com> (cherry picked from commit c1fb0aa) Signed-off-by: Szehon Ho <szehon.apache@gmail.com>
…ation checks ### What changes were proposed in this pull request? Followup to #57176. Exclude streaming-table and flow definition plans from the generic batch streaming-source check. Add Catalyst tests covering all three definition forms and document why the check applies only to root pipeline commands while excluding materialized views. ### Why are the changes needed? `UnsupportedOperationChecker` says streaming sources are allowed within Spark Declarative Pipeline definitions, but currently rejects those definition plans before the pipelines subsystem can interpret them. ### Does this PR introduce _any_ user-facing change? No. This fixes validation consistency for pipeline-definition logical plans introduced on the unreleased master branch. ### How was this patch tested? Ran before the latest review follow-up: `./build/sbt 'catalyst/testOnly org.apache.spark.sql.catalyst.analysis.UnsupportedOperationsSuite'` All 218 tests passed. The latest follow-up adds the third AUTO CDC regression case; the suite has not been rerun locally since that addition. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Codex (GPT-5) Closes #57345 from cloud-fan/SPARK-57402-followup. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com>
…ation checks ### What changes were proposed in this pull request? Followup to #57176. Exclude streaming-table and flow definition plans from the generic batch streaming-source check. Add Catalyst tests covering all three definition forms and document why the check applies only to root pipeline commands while excluding materialized views. ### Why are the changes needed? `UnsupportedOperationChecker` says streaming sources are allowed within Spark Declarative Pipeline definitions, but currently rejects those definition plans before the pipelines subsystem can interpret them. ### Does this PR introduce _any_ user-facing change? No. This fixes validation consistency for pipeline-definition logical plans introduced on the unreleased master branch. ### How was this patch tested? Ran before the latest review follow-up: `./build/sbt 'catalyst/testOnly org.apache.spark.sql.catalyst.analysis.UnsupportedOperationsSuite'` All 218 tests passed. The latest follow-up adds the third AUTO CDC regression case; the suite has not been rerun locally since that addition. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Codex (GPT-5) Closes #57345 from cloud-fan/SPARK-57402-followup. Authored-by: Wenchen Fan <wenchen@databricks.com> Signed-off-by: Wenchen Fan <wenchen@databricks.com> (cherry picked from commit 2608b6b) Signed-off-by: Wenchen Fan <wenchen@databricks.com>
What changes were proposed in this pull request? Hook up the two AUTO CDC SQL constructs introduced in SPARK-56249 to the Spark Declarative Pipelines dataflow graph in
SqlGraphRegistrationContext:CREATE STREAMING TABLE <name> FLOW AUTO CDC FROM <source> ..., parsed intoCreateStreamingTableAutoCdc, now registers the streaming table and anAutoCdcFlowthat targets it.CREATE FLOW <name> AS AUTO CDC INTO <target> FROM <source> ..., parsed into aCreateFlowCommandwrapping anAutoCdcIntoCommand, now registers anAutoCdcFlowfrom the named flow into the target dataset.A shared
buildChangeArgshelper converts the parse-time expressions and unresolved attributes into theChangeArgsconsumed byAutoCdcFlow(keys, sequencing, delete condition, include/exclude column selection). SQL AUTO CDC only supports SCD Type 1, matching the Connect/proto path.Why are the changes needed?
Before this change the AUTO CDC SQL syntax parsed successfully but was rejected during graph registration, so it could not be used to define pipeline datasets or flows.
Does this PR introduce any user-facing change?
Yes. AUTO CDC SQL statements now register datasets and flows in a pipeline.
How was this patch tested?
Added registration and end-to-end execution tests to
SqlPipelineSuitecovering both syntaxes, the optional clauses (APPLY AS DELETE WHEN, COLUMNS, COLUMNS * EXCEPT), and the multipart-flow-name error. FullSqlPipelineSuitepasses (50 tests).Was this patch authored or co-authored using generative AI tooling?
Generated-by: Opus 4.8