Arnab Karmakar has posted comments on this change. ( http://gerrit.cloudera.org:8080/24088 )
Change subject: IMPALA-14589: Add support for Iceberg V3 default values ...................................................................... Patch Set 3: (9 comments) Thanks for the comments! http://gerrit.cloudera.org:8080/#/c/24088/1//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/24088/1//COMMIT_MSG@9 PS1, Line 9: > nit: please use the 72-char limit in the body of the commit message Done http://gerrit.cloudera.org:8080/#/c/24088/1//COMMIT_MSG@27 PS1, Line 27: - If present, the value is materialized in the template tuple before > OTOH, to be able to test other data types with default values, it would mak As discussed, we can add DDL in the subsequent patch. We can add more test cases in that same PR. http://gerrit.cloudera.org:8080/#/c/24088/1/be/src/exec/file-metadata-utils.cc File be/src/exec/file-metadata-utils.cc: http://gerrit.cloudera.org:8080/#/c/24088/1/be/src/exec/file-metadata-utils.cc@176 PS1, Line 176: *slot > It should be VLOG_FILE or VLOG_ROW. Same goes for L184. Done http://gerrit.cloudera.org:8080/#/c/24088/1/be/src/exec/parquet/hdfs-parquet-scanner.cc File be/src/exec/parquet/hdfs-parquet-scanner.cc: http://gerrit.cloudera.org:8080/#/c/24088/1/be/src/exec/parquet/hdfs-parquet-scanner.cc@649 PS1, Line 649: // We are selecting a column that is not in the file. If it doesn't have a default : // value, we would set its slot to NULL during the scan, so any predicate would : // evaluate to false. Return early. NULL comparisons cannot happen here, since : // predicates with NULL literals are filtered in the frontend. : if (!file_metadata_utils_.HasIcebergDefaultValue(slot_desc)) { : *skip_row_group = true; : break; : } : // If the column has a default value, we can't evaluate statistics for it since : / > This could be moved to FileMetadataUtils::NeedDataInFile(). I tried doing this but faced errors because: 1. Theres a problem in the 4th callsite of NeedDataInFile() in the file as we're calling ResolvePath() after NeedDataInFile(). 2. For data files where values dont exist for column with a default val, it'll return false and we'll skip to the next slot which is ok. 3. But for data files where values do exist for column j, no column reader will be created and the actual values wont be read by the scanners and we'll only get the default val. http://gerrit.cloudera.org:8080/#/c/24088/1/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java: http://gerrit.cloudera.org:8080/#/c/24088/1/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@998 PS1, Line 998: } > Can you please file a Jira ticket that it should be done for the MERGE-stat Done. Filed IMPALA-14833 http://gerrit.cloudera.org:8080/#/c/24088/1/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java@1008 PS1, Line 1008: > You could add: "Please specify value for column '{}' explicitly." Done http://gerrit.cloudera.org:8080/#/c/24088/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-v3-default-values.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-v3-default-values.test: http://gerrit.cloudera.org:8080/#/c/24088/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-v3-default-values.test@136 PS1, Line 136: > You could change the write-default in iceberg_v3_default_value/metadata/v4. Done http://gerrit.cloudera.org:8080/#/c/24088/1/testdata/workloads/functional-query/queries/QueryTest/iceberg-v3-default-values.test@163 PS1, Line 163: # Time travel — first snapshot: schema had only i > Please add tests for INSERT-overwrite as well. Done http://gerrit.cloudera.org:8080/#/c/24088/1/tests/query_test/test_iceberg.py File tests/query_test/test_iceberg.py: http://gerrit.cloudera.org:8080/#/c/24088/1/tests/query_test/test_iceberg.py@2341 PS1, Line 2341: > Can we have tests for ORC/Avro as well? Done -- To view, visit http://gerrit.cloudera.org:8080/24088 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I9f1be994a336b30b17b17819091417d777a39be9 Gerrit-Change-Number: 24088 Gerrit-PatchSet: 3 Gerrit-Owner: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Sun, 29 Mar 2026 18:25:38 +0000 Gerrit-HasComments: Yes
