github-actions[bot] commented on code in PR #65281:
URL: https://github.com/apache/doris/pull/65281#discussion_r3533972352
##########
regression-test/suites/external_table_p0/iceberg/test_iceberg_deletion_vector.groovy:
##########
@@ -74,9 +371,200 @@ suite("test_iceberg_deletion_vector", "p0,external") {
qt_q19 """ SELECT count(value) FROM dv_test_1w where id%2 = 1; """
qt_q20 """ SELECT count(id) FROM dv_test_1w where id%3 = 1; """
qt_q21 """ SELECT count(ts) FROM dv_test_1w where id%3 != 1; """
+ qt_q22 """ SELECT * FROM dv_test ORDER BY data DESC LIMIT 3; """
+ qt_q23 """ SELECT * FROM dv_test_v2 ORDER BY data DESC LIMIT 3; """
+ qt_q24 """ SELECT batch, count(*), sum(id) FROM dv_test GROUP BY batch
ORDER BY batch; """
+ qt_q25 """ SELECT batch, count(*), sum(id) FROM dv_test_v2 GROUP BY batch
ORDER BY batch; """
+ qt_q26 """ SELECT batch, count(*), sum(id) FROM dv_test_orc GROUP BY batch
ORDER BY batch; """
+ qt_q27 """ SELECT id, data FROM dv_test WHERE id BETWEEN 2 AND 10 ORDER BY
id LIMIT 2; """
+ qt_q28 """ SELECT id, data FROM dv_test_v2 WHERE id BETWEEN 2 AND 10 ORDER
BY id LIMIT 2; """
+ qt_q29 """ SELECT id, data FROM dv_test_orc WHERE id BETWEEN 2 AND 10
ORDER BY id LIMIT 2; """
+ qt_q30 """ SELECT count(*) FROM (SELECT * FROM dv_test_1w WHERE id % 7 = 0
ORDER BY id LIMIT 100) t; """
+ qt_q31 """ SELECT count(*) FROM (SELECT * FROM dv_test_1w WHERE id % 11 =
0 ORDER BY id LIMIT 100) t; """
+
+ // Metadata matrix verifies the physical delete-file content types that
back the logical
+ // scenario checks above.
+ qt_delete_type_matrix """
+ SELECT 'dv_equality' scenario, content, file_format, count(*) files,
sum(record_count) records
+ FROM `dv_delete_matrix_equality_and_dv\$files`
+ GROUP BY content, file_format
+ UNION ALL
+ SELECT 'dv_only' scenario, content, file_format, count(*) files,
sum(record_count) records
+ FROM `dv_test\$files`
+ GROUP BY content, file_format
+ UNION ALL
+ SELECT 'dv_position' scenario, content, file_format, count(*) files,
sum(record_count) records
+ FROM `dv_delete_matrix_position_and_dv\$files`
+ GROUP BY content, file_format
+ UNION ALL
+ SELECT 'equality_only' scenario, content, file_format, count(*) files,
sum(record_count) records
+ FROM `dv_delete_matrix_equality_only\$files`
+ GROUP BY content, file_format
+ UNION ALL
+ SELECT 'no_delete' scenario, content, file_format, count(*) files,
sum(record_count) records
+ FROM `dv_test_no_delete\$files`
+ GROUP BY content, file_format
+ UNION ALL
+ SELECT 'position_only' scenario, content, file_format, count(*) files,
sum(record_count) records
+ FROM `dv_delete_matrix_position_only\$files`
+ GROUP BY content, file_format
+ ORDER BY scenario, content, file_format;
+ """
+ // Iceberg v3 semantics: when a data file has a DV, old position deletes
for that file must be
+ // ignored, while equality deletes still filter matching rows.
+ qt_position_delete_ignored_with_dv """
+ SELECT id, name
+ FROM dv_delete_matrix_position_and_dv
+ WHERE id IN (1, 2, 5, 10, 15)
+ ORDER BY id, name
+ LIMIT 20;
+ """
+ qt_equality_delete_active_with_dv """
+ SELECT id, batch, data
+ FROM dv_delete_matrix_equality_and_dv
+ ORDER BY id;
+ """
+
+ def normalizeExternalRows = { String output ->
+ return output.readLines()
+ .collect { it.trim() }
+ .findAll { it ==~ /^-?[0-9].*/ && it.contains("\t") }
+ .join("\n")
+ }
+ String expectedRows = ["2\t1\tb", "4\t1\td", "6\t1\tf", "8\t1\th",
"12\t2\tl"].join("\n")
+ String sparkRows = normalizeExternalRows(runSparkSql(
+ "use format_v3; select id, batch, data from
dv_delete_matrix_equality_and_dv order by id;",
+ 300
+ ))
+ assertEquals(expectedRows, sparkRows)
+ String javaRows = normalizeExternalRows(runInSparkContainer(
+ "java -cp \"/tmp:/opt/spark/jars/*\" ReadIcebergRows " +
+ "format_v3 dv_delete_matrix_equality_and_dv id batch data",
+ 300
+ ))
+ assertEquals(expectedRows, javaRows)
+
+ String trinoContainerName = findRequiredDockerContainer(
Review Comment:
This makes the Iceberg DV suite require a Trino container even though the
normal Iceberg external environment does not provide one.
`findRequiredDockerContainer("Trino", "icebergTrinoContainer", ...)` asserts
when no matching container exists, but
`docker/thirdparties/docker-compose/iceberg/iceberg.yaml.tpl` only starts
Spark, REST, MinIO, Postgres, and mc, and the external regression config
enables Iceberg tests without setting `icebergTrinoContainer`. The only Trino
images in this repo are the two Kerberos services, so an Iceberg-only run finds
none, while a full external environment can find multiple unrelated Trino
containers and hit the other assertion. The previous code skipped the Trino
cross-check when no Trino container was running; with this change the suite can
fail before any Doris DV assertion runs. Please either keep the Trino
cross-check optional when no Trino container/config is present, or add a
dedicated Trino service/config for the Iceberg environment before mak
ing this mandatory.
##########
regression-test/suites/external_table_p0/iceberg/test_iceberg_deletion_vector.groovy:
##########
@@ -74,9 +371,200 @@ suite("test_iceberg_deletion_vector", "p0,external") {
qt_q19 """ SELECT count(value) FROM dv_test_1w where id%2 = 1; """
qt_q20 """ SELECT count(id) FROM dv_test_1w where id%3 = 1; """
qt_q21 """ SELECT count(ts) FROM dv_test_1w where id%3 != 1; """
+ qt_q22 """ SELECT * FROM dv_test ORDER BY data DESC LIMIT 3; """
+ qt_q23 """ SELECT * FROM dv_test_v2 ORDER BY data DESC LIMIT 3; """
+ qt_q24 """ SELECT batch, count(*), sum(id) FROM dv_test GROUP BY batch
ORDER BY batch; """
+ qt_q25 """ SELECT batch, count(*), sum(id) FROM dv_test_v2 GROUP BY batch
ORDER BY batch; """
+ qt_q26 """ SELECT batch, count(*), sum(id) FROM dv_test_orc GROUP BY batch
ORDER BY batch; """
+ qt_q27 """ SELECT id, data FROM dv_test WHERE id BETWEEN 2 AND 10 ORDER BY
id LIMIT 2; """
+ qt_q28 """ SELECT id, data FROM dv_test_v2 WHERE id BETWEEN 2 AND 10 ORDER
BY id LIMIT 2; """
+ qt_q29 """ SELECT id, data FROM dv_test_orc WHERE id BETWEEN 2 AND 10
ORDER BY id LIMIT 2; """
+ qt_q30 """ SELECT count(*) FROM (SELECT * FROM dv_test_1w WHERE id % 7 = 0
ORDER BY id LIMIT 100) t; """
+ qt_q31 """ SELECT count(*) FROM (SELECT * FROM dv_test_1w WHERE id % 11 =
0 ORDER BY id LIMIT 100) t; """
+
+ // Metadata matrix verifies the physical delete-file content types that
back the logical
+ // scenario checks above.
+ qt_delete_type_matrix """
+ SELECT 'dv_equality' scenario, content, file_format, count(*) files,
sum(record_count) records
+ FROM `dv_delete_matrix_equality_and_dv\$files`
+ GROUP BY content, file_format
+ UNION ALL
+ SELECT 'dv_only' scenario, content, file_format, count(*) files,
sum(record_count) records
+ FROM `dv_test\$files`
+ GROUP BY content, file_format
+ UNION ALL
+ SELECT 'dv_position' scenario, content, file_format, count(*) files,
sum(record_count) records
+ FROM `dv_delete_matrix_position_and_dv\$files`
+ GROUP BY content, file_format
+ UNION ALL
+ SELECT 'equality_only' scenario, content, file_format, count(*) files,
sum(record_count) records
+ FROM `dv_delete_matrix_equality_only\$files`
+ GROUP BY content, file_format
+ UNION ALL
+ SELECT 'no_delete' scenario, content, file_format, count(*) files,
sum(record_count) records
+ FROM `dv_test_no_delete\$files`
+ GROUP BY content, file_format
+ UNION ALL
+ SELECT 'position_only' scenario, content, file_format, count(*) files,
sum(record_count) records
+ FROM `dv_delete_matrix_position_only\$files`
+ GROUP BY content, file_format
+ ORDER BY scenario, content, file_format;
+ """
+ // Iceberg v3 semantics: when a data file has a DV, old position deletes
for that file must be
+ // ignored, while equality deletes still filter matching rows.
+ qt_position_delete_ignored_with_dv """
Review Comment:
This new scenario documents the right Iceberg v3 rule, but the format_v2
path still implements the opposite for the same split shape.
`IcebergTableReader::prepare_split()` lets
`TableReader::_parse_delete_predicates()` load the DV into `_delete_rows`, then
`_init_delete_predicates()` copies those rows and still calls
`_init_position_delete_rows(position_delete_files)`, so v2 applies the union of
the DV and any position-delete files. The classic `IcebergReaderMixin` path
skips position deletes when a DV is present, and the Iceberg spec's
scan-planning rule says position delete files apply only when there is no DV
for the data file: https://iceberg.apache.org/spec/#scan-planning. Because
`enable_file_scanner_v2` is on by default for supported Iceberg Parquet splits,
please align the v2 reader with this test's semantics and update the v2 unit
that currently expects
`IcebergTableReaderMergesDeletionVectorAndPositionDeleteFiles`.
--
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]
