SanJSp commented on code in PR #55508: URL: https://github.com/apache/spark/pull/55508#discussion_r3145941524
########## sql/core/src/test/scala/org/apache/spark/sql/connector/ResolveChangelogTablePostProcessingSuite.scala: ########## @@ -0,0 +1,970 @@ +/* + * 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.connector + +import java.util.Collections + +import org.scalatest.BeforeAndAfterEach + +import org.apache.spark.SparkRuntimeException +import org.apache.spark.sql.{AnalysisException, QueryTest, Row} +import org.apache.spark.sql.catalyst.InternalRow +import org.apache.spark.sql.connector.catalog.{ + ChangelogProperties, Column, Identifier, InMemoryChangelogCatalog} +import org.apache.spark.sql.connector.expressions.Transform +import org.apache.spark.sql.test.SharedSparkSession +import org.apache.spark.sql.types.{ + BinaryType, BooleanType, DoubleType, LongType, StringType, StructField, StructType} +import org.apache.spark.unsafe.types.UTF8String + +/** + * Tests for [[org.apache.spark.sql.catalyst.analysis.ResolveChangelogTable]] using the + * in-memory changelog catalog. These tests don't depend on Delta or any specific connector; + * they directly control what the connector "returns" by populating the in-memory changelog + * with hand-crafted change rows. + * + * Each test sets up [[ChangelogProperties]] on the catalog to enable specific post-processing + * paths (carry-over removal, update detection) and then verifies that Spark's analyzer rule + * correctly transforms the plan and produces the expected output. + */ +class ResolveChangelogTablePostProcessingSuite + extends QueryTest + with SharedSparkSession + with BeforeAndAfterEach { + + private val catalogName = "cdc_test_catalog" + private val testTableName = "events" + + override def beforeAll(): Unit = { + super.beforeAll() + spark.conf.set( + s"spark.sql.catalog.$catalogName", + classOf[InMemoryChangelogCatalog].getName) + } + + override def beforeEach(): Unit = { + super.beforeEach() + val cat = catalog + val ident = Identifier.of(Array.empty, testTableName) + if (cat.tableExists(ident)) cat.dropTable(ident) + cat.clearChangeRows(ident) + cat.setChangelogProperties(ident, ChangelogProperties()) + cat.createTable( + ident, + Array( + Column.create("id", LongType), + Column.create("name", StringType), + Column.create("row_commit_version", LongType, false)), + Array.empty[Transform], + Collections.emptyMap[String, String]()) + } + + private def catalog: InMemoryChangelogCatalog = { + spark.sessionState.catalogManager + .catalog(catalogName) + .asInstanceOf[InMemoryChangelogCatalog] + } + + private def ident = Identifier.of(Array.empty, testTableName) + + /** + * Helper to create a change row matching schema + * (id, name, row_commit_version, _change_type, _commit_version, _commit_timestamp). + * + * `rowCommitVersion` follows Delta row-tracking semantics: carry-over pairs (CoW-rewritten + * unchanged rows) share the same value on both sides; real updates carry the OLD value on + * the delete side and the NEW value on the insert side. Defaults to `commitVersion` for + * tests that don't exercise carry-over removal. + */ + private def changeRow( + id: Long, + name: String, + changeType: String, + commitVersion: Long, + rowCommitVersion: Long = -1L, + commitTimestamp: Long = 0L): InternalRow = { + val rcv = if (rowCommitVersion == -1L) commitVersion else rowCommitVersion + InternalRow( + id, + UTF8String.fromString(name), + rcv, + UTF8String.fromString(changeType), + commitVersion, + commitTimestamp) + } + + // =========================================================================== + // Carry-Over Removal + // =========================================================================== + + test("carry-over removal drops identical delete+insert pairs") { + catalog.setChangelogProperties(ident, ChangelogProperties( + containsCarryoverRows = true, + rowIdNames = Seq("id"), + rowVersionName = Some("row_commit_version"))) + + // v1: insert Alice and Bob (rcv=1 each) + // v2: real delete Alice (preimage carries old rcv=1); + // carry-over for Bob (CoW, rcv unchanged on both sides) + catalog.addChangeRows(ident, Seq( + changeRow(1L, "Alice", "insert", 1L, rowCommitVersion = 1L), + changeRow(2L, "Bob", "insert", 1L, rowCommitVersion = 1L), + changeRow(1L, "Alice", "delete", 2L, rowCommitVersion = 1L), + changeRow(2L, "Bob", "delete", 2L, rowCommitVersion = 1L), // carry-over + changeRow(2L, "Bob", "insert", 2L, rowCommitVersion = 1L))) // carry-over (same rcv) + + checkAnswer( + sql( + s"SELECT id, name, _change_type, _commit_version " + + s"FROM $catalogName.$testTableName CHANGES FROM VERSION 1 TO VERSION 2"), + Seq( + Row(1L, "Alice", "insert", 1L), + Row(2L, "Bob", "insert", 1L), + Row(1L, "Alice", "delete", 2L))) + } + + test("deduplicationMode=none keeps all carry-over rows") { + catalog.setChangelogProperties(ident, ChangelogProperties( + containsCarryoverRows = true, + rowIdNames = Seq("id"), + rowVersionName = Some("row_commit_version"))) + + catalog.addChangeRows(ident, Seq( + changeRow(1L, "Alice", "insert", 1L, rowCommitVersion = 1L), + changeRow(2L, "Bob", "delete", 2L, rowCommitVersion = 1L), + changeRow(2L, "Bob", "insert", 2L, rowCommitVersion = 1L))) + + val rows = sql( + s"SELECT id FROM $catalogName.$testTableName " + + s"CHANGES FROM VERSION 1 TO VERSION 2 WITH (deduplicationMode = 'none')") + .collect() + + assert(rows.length == 3, "Without dedup, all 3 raw rows should be returned") + } + + test("NULL rowVersion on one side is NOT silently dropped as carry-over") { + // Regression for a NULL-safety hole: min/max skip NULLs, so _min_rv = _max_rv alone + // would match a pair with one NULL and one non-null rowVersion. The _rv_cnt = 2 + // clause in the carry-over filter prevents that. + catalog.setChangelogProperties(ident, ChangelogProperties( + containsCarryoverRows = true, + rowIdNames = Seq("id"), + rowVersionName = Some("row_commit_version"))) + + catalog.addChangeRows(ident, Seq( + changeRow(1L, "Alice", "insert", 1L, rowCommitVersion = 1L), + // v2: one side has NULL rowVersion (buggy connector), the other has a real value. + InternalRow(1L, UTF8String.fromString("Alice"), null, + UTF8String.fromString("delete"), 2L, 0L), + changeRow(1L, "Alice", "insert", 2L, rowCommitVersion = 5L))) Review Comment: Done. -- 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]
