szehon-ho commented on code in PR #57584:
URL: https://github.com/apache/spark/pull/57584#discussion_r3668142785
##########
sql/pipelines/src/test/scala/org/apache/spark/sql/pipelines/graph/AutoCdcOutOfOrderConvergenceSuite.scala:
##########
@@ -173,22 +185,25 @@ class AutoCdcScd1OutOfOrderConvergenceSuite
val shuffledEventStream = rand.shuffle(sortedEventStream)
withClue(
- s"\nseed=$seed (rerun with -D$seedSystemProperty=$seed to reproduce)\n" +
+ s"\nscdType=${scdType.label} seed=$seed " +
+ s"(rerun with -D$seedSystemProperty=$seed to reproduce)\n" +
Review Comment:
The property this message tells the developer to set is still
`spark.sql.test.autocdc.scd1OutOfOrderConvergenceSeed` (line 80), so a failing
SCD2 run prints an SCD1-named knob. Worth renaming along with the suite.
Separately, the comment on line 193 justifies the scd-type table-name suffix
as keeping the SCD1 and SCD2 tests from colliding within a run, but the mixin's
`afterEach` calls `SharedTablesInMemoryRowLevelOperationTableCatalog.reset()`,
so the tables cannot collide. The suffix is harmless; the stated reason just is
not the real one.
##########
sql/pipelines/src/test/scala/org/apache/spark/sql/pipelines/graph/AutoCdcScd2SinglePipelineSuite.scala:
##########
@@ -0,0 +1,188 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.pipelines.graph
+
+import org.apache.spark.sql.Row
+import org.apache.spark.sql.execution.streaming.runtime.MemoryStream
+import org.apache.spark.sql.functions
+import org.apache.spark.sql.pipelines.autocdc.{
+ AutoCdcReservedNames,
+ ColumnSelection,
+ ScdType,
+ UnqualifiedColumnName
+}
+import org.apache.spark.sql.pipelines.utils.{ExecutionTest,
TestGraphRegistrationContext}
+import org.apache.spark.sql.test.SharedSparkSession
+
+/**
+ * End-to-end smoke tests for AutoCDC SCD Type 2 flows running within a single
pipeline: one
+ * [[DataflowGraph]] / [[TestPipelineUpdateContext]] executes an SCD2 AutoCDC
flow through the
+ * [[Scd2MergeStreamingWrite]] streaming write, and both the target table and
the auxiliary
+ * table contents are asserted at the end.
+ *
+ * This exercises the full wiring landed for SCD2: the flow planner routing an
SCD2
+ * [[AutoCdcMergeFlow]] to [[Scd2MergeStreamingWrite]], the auxiliary-table
materialization, and
+ * the [[org.apache.spark.sql.pipelines.autocdc.Scd2ForeachBatchHandler]]
reconciliation.
+ */
+class AutoCdcScd2SinglePipelineSuite
+ extends ExecutionTest
+ with SharedSparkSession
+ with AutoCdcGraphExecutionTestMixin {
+
+ /** The SCD2 target's `_cdc_metadata` struct value for a given
recordStartAt. */
+ private def scd2Meta(recordStartAt: Long): Row = Row(recordStartAt)
+
+ /**
+ * DDL for an SCD2 target table with user columns `(id, name, version)` plus
the framework
+ * columns `__START_AT` / `__END_AT` (sequencing type BIGINT) and the SCD2
`_cdc_metadata`
+ * struct. `version` is the sequencing column and, unless excluded via a
column selection, is
+ * retained as an ordinary user column in the target.
+ */
+ private def createScd2Target(table: String): Unit = {
+ spark.sql(
+ s"CREATE TABLE $table (" +
+ s"id INT NOT NULL, name STRING, version BIGINT NOT NULL,
$scd2MetadataDdl)"
+ )
+ }
+
+ test("SCD2: an upsert lands an open current record in an empty target
table") {
+ val session = spark
+ import session.implicits._
+ createScd2Target(s"$catalog.$namespace.target")
+
+ val stream = MemoryStream[(Int, String, Long)]
+ stream.addData((1, "alice", 10L))
+
+ val ctx = new TestGraphRegistrationContext(spark) {
+ registerTable("target", catalog = Some(catalog), database =
Some(namespace))
+ registerFlow(autoCdcFlow(
+ name = "auto_cdc_flow",
+ target = "target",
+ query = dfFlowFunc(stream.toDF().toDF("id", "name", "version")),
+ keys = Seq("id"),
+ sequencing = functions.col("version"),
+ scdType = ScdType.Type2
+ ))
+ }
+
+ runPipeline(ctx)
+
+ // A single event opens a current record: START_AT = the event sequence,
END_AT = null.
+ checkAnswer(
+ spark.table(s"$catalog.$namespace.target"),
+ Seq(Row(1, "alice", 10L, 10L, null, scd2Meta(10L)))
+ )
+ }
+
+ test("SCD2: an update to a key closes the prior record and opens a new one")
{
+ val session = spark
+ import session.implicits._
+ createScd2Target(s"$catalog.$namespace.target")
+
+ val stream = MemoryStream[(Int, String, Long)]
+ stream.addData((1, "alice", 10L), (1, "alicia", 20L))
+
+ val ctx = new TestGraphRegistrationContext(spark) {
+ registerTable("target", catalog = Some(catalog), database =
Some(namespace))
+ registerFlow(autoCdcFlow(
+ name = "auto_cdc_flow",
+ target = "target",
+ query = dfFlowFunc(stream.toDF().toDF("id", "name", "version")),
+ keys = Seq("id"),
+ sequencing = functions.col("version"),
+ scdType = ScdType.Type2
+ ))
+ }
+
+ runPipeline(ctx)
+
+ // The first value is closed at the second event's sequence; the second
value is open.
+ checkAnswer(
+ spark.table(s"$catalog.$namespace.target"),
+ Seq(
+ Row(1, "alice", 10L, 10L, 20L, scd2Meta(10L)),
+ Row(1, "alicia", 20L, 20L, null, scd2Meta(20L))
+ )
+ )
+ }
+
+ test("SCD2: a delete closes the current record with no open record
remaining") {
+ val session = spark
+ import session.implicits._
+ // Target omits `is_delete`: the source carries it as a control column
driving the delete
+ // condition, and it is excluded from the target projection.
+ createScd2Target(s"$catalog.$namespace.target")
+
+ val stream = MemoryStream[(Int, String, Long, Boolean)]
+ stream.addData((1, "alice", 10L, false), (1, null, 20L, true))
+
+ val ctx = new TestGraphRegistrationContext(spark) {
+ registerTable("target", catalog = Some(catalog), database =
Some(namespace))
+ registerFlow(autoCdcFlow(
+ name = "auto_cdc_flow",
+ target = "target",
+ query = dfFlowFunc(stream.toDF().toDF("id", "name", "version",
"is_delete")),
+ keys = Seq("id"),
+ sequencing = functions.col("version"),
+ columnSelection = Some(
+
ColumnSelection.ExcludeColumns(Seq(UnqualifiedColumnName("is_delete")))
+ ),
+ deleteCondition = Some(functions.col("is_delete")),
+ scdType = ScdType.Type2
+ ))
+ }
+
+ runPipeline(ctx)
+
+ // The delete closes the open record at the delete's sequence; nothing
remains open.
+ checkAnswer(
+ spark.table(s"$catalog.$namespace.target"),
+ Seq(Row(1, "alice", 10L, 10L, 20L, scd2Meta(10L)))
+ )
+ }
+
+ test("SCD2: the auxiliary table is materialized for the target") {
+ val session = spark
+ import session.implicits._
+ createScd2Target(s"$catalog.$namespace.target")
+
+ val stream = MemoryStream[(Int, String, Long)]
+ stream.addData((1, "alice", 10L))
+
+ val ctx = new TestGraphRegistrationContext(spark) {
+ registerTable("target", catalog = Some(catalog), database =
Some(namespace))
+ registerFlow(autoCdcFlow(
+ name = "auto_cdc_flow",
+ target = "target",
+ query = dfFlowFunc(stream.toDF().toDF("id", "name", "version")),
+ keys = Seq("id"),
+ sequencing = functions.col("version"),
+ scdType = ScdType.Type2
+ ))
+ }
+
+ runPipeline(ctx)
+
+ // The SCD2 auxiliary table exists and carries the aux-only
deleted-by-batch-id marker column
+ // in addition to the full target row schema.
+ val auxColumns =
spark.table(auxTableNameFor("target")).schema.fieldNames.toSet
+ assert(auxColumns.contains(AutoCdcReservedNames.cdcMetadataColName))
+ assert(auxColumns.contains("__START_AT"))
+ assert(auxColumns.contains("__END_AT"))
Review Comment:
The comment says the aux table "carries the aux-only deleted-by-batch-id
marker column", but no assertion checks for it. Relatedly, the class scaladoc
says "both the target table and the auxiliary table contents are asserted at
the end", whereas what is asserted here is the schema rather than the contents.
`Scd2BatchProcessor.deletedByBatchIdColName` is `private[pipelines]` and
`AutoCdcScd2AuxiliaryTableSpecSuite` already uses it from this package, so this
could assert the full materialized schema (the target fields plus the marker
appended) and then match its own comment. The distinct value of this test over
the spec suite is proving the table really is created by a live run, and a
full-schema assertion keeps that while making it stronger.
Same thought for the two literals: `Scd2BatchProcessor.startAtColName` /
`endAtColName` are reachable here, and those two are precisely the reserved
columns that do not carry `AutoCdcReservedNames.prefix`, so a rename would slip
past a string literal.
--
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]
