github-actions[bot] commented on code in PR #68128:
URL: https://github.com/apache/doris/pull/68128#discussion_r4081062164
##########
regression-test/data/external_table_p0/iceberg/write/test_iceberg_write_stats2.out:
##########
@@ -7,7 +7,7 @@ true 11 111 1.1 1.1 1111 1234.5678
1234.567890 123456789012345678.123456789012 a
0 PARQUET 2 {1:2, 2:2, 3:2, 4:2, 5:2, 6:2, 7:2, 8:2, 9:2, 10:2,
11:2, 12:2} {1:0, 2:0, 3:0, 4:0, 5:0, 6:0, 7:0, 8:0, 9:0, 10:0, 11:0, 12:0}
{1:0x00, 2:0x0B000000, 3:0x6F00000000000000, 4:0xCDCC8C3F,
5:0x9A9999999999F13F, 6:0x00000457, 7:0x00BC614E, 8:0x00000000499602D2,
9:0x000000018EE90FF6C373E0393713FA14, 10:0x616161, 11:0x9E4B0000,
12:0x005CE70F33F10500} {1:0x01, 2:0x16000000, 3:0xDE00000000000000,
4:0xCDCC0C40, 5:0x9A99999999990140, 6:0x000008AE, 7:0x05397FB1,
8:0x000000020A75E124, 9:0x0000000C7748819DFFB62505316873CB, 10:0x626262,
11:0x434C0000, 12:0x40D91FA833FE0500}
-- !sql_2 --
-{"bigint_col":{"column_size":74, "value_count":2, "null_value_count":0,
"nan_value_count":null, "lower_bound":111, "upper_bound":222},
"boolean_col":{"column_size":33, "value_count":2, "null_value_count":0,
"nan_value_count":null, "lower_bound":0, "upper_bound":1},
"date_col":{"column_size":66, "value_count":2, "null_value_count":0,
"nan_value_count":null, "lower_bound":"2023-01-01",
"upper_bound":"2023-06-15"}, "datetime_col1":{"column_size":74,
"value_count":2, "null_value_count":0, "nan_value_count":null,
"lower_bound":"2023-01-01 20:34:56.000000", "upper_bound":"2023-06-16
07:45:01.000000"}, "decimal_col1":{"column_size":66, "value_count":2,
"null_value_count":0, "nan_value_count":null, "lower_bound":1111,
"upper_bound":2222}, "decimal_col2":{"column_size":66, "value_count":2,
"null_value_count":0, "nan_value_count":null, "lower_bound":1234.5678,
"upper_bound":8765.4321}, "decimal_col3":{"column_size":74, "value_count":2,
"null_value_count":0, "nan_value_count":null, "lower_boun
d":1234.567890, "upper_bound":8765.432100}, "decimal_col4":{"column_size":90,
"value_count":2, "null_value_count":0, "nan_value_count":null,
"lower_bound":123456789012345678.123456789012,
"upper_bound":987654321098765432.987654321099}, "double_col":{"column_size":74,
"value_count":2, "null_value_count":0, "nan_value_count":null,
"lower_bound":1.1, "upper_bound":2.2}, "float_col":{"column_size":66,
"value_count":2, "null_value_count":0, "nan_value_count":null,
"lower_bound":1.1, "upper_bound":2.2}, "int_col":{"column_size":66,
"value_count":2, "null_value_count":0, "nan_value_count":null,
"lower_bound":11, "upper_bound":22}, "string_col":{"column_size":72,
"value_count":2, "null_value_count":0, "nan_value_count":null,
"lower_bound":"aaa", "upper_bound":"bbb"}}
+{"bigint_col":{"column_size":74, "value_count":2, "null_value_count":0,
"nan_value_count":null, "lower_bound":111, "upper_bound":222},
"boolean_col":{"column_size":33, "value_count":2, "null_value_count":0,
"nan_value_count":null, "lower_bound":0, "upper_bound":1},
"date_col":{"column_size":66, "value_count":2, "null_value_count":0,
"nan_value_count":null, "lower_bound":"2023-01-01",
"upper_bound":"2023-06-15"}, "datetime_col1":{"column_size":74,
"value_count":2, "null_value_count":0, "nan_value_count":null,
"lower_bound":"2023-01-01 20:34:56.000000", "upper_bound":"2023-06-16
07:45:01.000000"}, "decimal_col1":{"column_size":66, "value_count":2,
"null_value_count":0, "nan_value_count":null, "lower_bound":1111,
"upper_bound":2222}, "decimal_col2":{"column_size":66, "value_count":2,
"null_value_count":0, "nan_value_count":null, "lower_bound":1234.5678,
"upper_bound":8765.4321}, "decimal_col3":{"column_size":74, "value_count":2,
"null_value_count":0, "nan_value_count":null, "lower_boun
d":1234.567890, "upper_bound":8765.432100}, "decimal_col4":{"column_size":90,
"value_count":2, "null_value_count":0, "nan_value_count":null,
"lower_bound":123456789012345678.123456789012,
"upper_bound":987654321098765432.987654321099}, "double_col":{"column_size":74,
"value_count":2, "null_value_count":0, "nan_value_count":0, "lower_bound":1.1,
"upper_bound":2.2}, "float_col":{"column_size":66, "value_count":2,
"null_value_count":0, "nan_value_count":0, "lower_bound":1.1,
"upper_bound":2.2}, "int_col":{"column_size":66, "value_count":2,
"null_value_count":0, "nan_value_count":null, "lower_bound":11,
"upper_bound":22}, "string_col":{"column_size":72, "value_count":2,
"null_value_count":0, "nan_value_count":null, "lower_bound":"aaa",
"upper_bound":"bbb"}}
Review Comment:
[P2] Regenerate this golden with the regression runner
This output changes the asserted BE-to-Iceberg manifest result from `null`
to `0`, but the PR's validation record says the file was edited without running
the case because no BE build was available. Doris requires regression `.out`
files to be generated by `run-regression-test.sh` (and this file carries the
generated-file warning), so a plausible hand edit is not validated evidence
that the new writer/Thrift/manifest path produces this exact result. Please run
the prescribed Iceberg regression and commit the runner-generated output.
##########
gensrc/thrift/DataSinks.thrift:
##########
@@ -491,6 +491,12 @@ struct TIcebergTableSink {
17: optional TIcebergWriteType write_type = TIcebergWriteType.INSERT;
// Unset keeps collection enabled for rolling upgrades with older FEs.
18: optional bool collect_column_stats;
+ // Iceberg field ids of the FLOAT/DOUBLE fields whose NaN count would
survive the table's metrics policy
+ // (effective mode != none). Counting a NaN is an extra pass over the data
-- unlike the other statistics,
+ // which the parquet footer already carries -- so BE must not pay it for a
field FE would then drop.
+ // Unset or empty means count nothing: an older FE does not read
nan_value_counts back, so counting for it
+ // would be pure waste, and a table whose float fields are all
metrics-disabled has nothing to report.
+ 19: optional list<i32> nan_count_field_ids;
Review Comment:
[P2] Carry NaN-count field IDs through UPDATE/MERGE
This field only reaches the direct `TIcebergTableSink` path. UPDATE and SQL
MERGE are planned as `TIcebergMergeSink`; that struct has
`collect_column_stats` but no `nan_count_field_ids`, and
`VIcebergMergeSink::_build_inner_sinks` therefore constructs its inner table
sink without the list. Those replacement data files register no columns in
`VIcebergParquetWriter`, emit no `nan_value_counts`, and remain conservatively
unprunable even when NaN-free, so the pruning restoration does not cover all
Doris-written files. Please add the parallel merge-sink field, populate/copy it
using the merge schema, and cover an UPDATE/MERGE-written file.
--
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]
