github-actions[bot] commented on code in PR #65281:
URL: https://github.com/apache/doris/pull/65281#discussion_r3533750828
##########
regression-test/suites/external_table_p0/iceberg/test_iceberg_deletion_vector.groovy:
##########
@@ -42,30 +42,283 @@ suite("test_iceberg_deletion_vector", "p0,external") {
sql """switch ${catalog_name};"""
sql """ use format_v3;"""
+ sql """ set file_split_size=1;"""
Review Comment:
This session variable should be restored in a `finally`. Right now the suite
sets `file_split_size=1` before all of the Spark setup, Java/Spark/optional
Trino checks, query assertions, and profile assertions, but only unsets it at
the normal end of the file. Any failure before that final line leaves the same
regression session in tiny-split mode, which can make later diagnostics or
subsequent work in the runner misleading. Please wrap the body after this setup
in `try/finally` and unset `file_split_size` in the `finally`, like the Paimon
DV suite does.
##########
be/src/format/table/iceberg_delete_file_reader_helper.cpp:
##########
@@ -17,6 +17,7 @@
#include "format/table/iceberg_delete_file_reader_helper.h"
+#include <fmt/format.h>
Review Comment:
This key is still ambiguous because both path fields are inserted unescaped
before the numeric suffix. For example, `data_file_path = "s3://b/a"` with
`delete_file.path = "x#y"` produces the same key as `data_file_path =
"s3://b/a#x"` with `delete_file.path = "y"` when offset and size match:
`delete_dv_s3://b/a#x#y#1#2`. Since Iceberg paths are object paths and may
contain `#`, the scan-node cache can reuse another data file's deletion-vector
rows and filter the wrong positions. Please make the string components
unambiguous, e.g. length-prefix or binary-encode each path before appending
offset and size, and add a collision test with `#` in both paths.
--
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]
