Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/16939 )
Change subject: IMPALA-10432: INSERT INTO Iceberg tables with partition transforms ...................................................................... Patch Set 5: (6 comments) Thanks for the comments, Gabor! http://gerrit.cloudera.org:8080/#/c/16939/5/testdata/workloads/functional-query/queries/QueryTest/iceberg-partition-transform-insert.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-partition-transform-insert.test: http://gerrit.cloudera.org:8080/#/c/16939/5/testdata/workloads/functional-query/queries/QueryTest/iceberg-partition-transform-insert.test@365 PS5, Line 365: t_year=1969 > I checked the Iceberg spec and it says that year transform should return an Here we have the human-readable partition path. We convert the partition path to partition data in IcebergUtil. http://gerrit.cloudera.org:8080/#/c/16939/5/testdata/workloads/functional-query/queries/QueryTest/iceberg-partition-transform-insert.test@487 PS5, Line 487: t_month=1970-01 > Just for my understanding: According to Iceberg spec month transform should See above answer. http://gerrit.cloudera.org:8080/#/c/16939/5/testdata/workloads/functional-query/queries/QueryTest/iceberg-partition-transform-insert.test@691 PS5, Line 691: create table hour_transform(t timestamp) > Do we have a negative test where HOUR partition transform is used on a date This patch only adds write support for the partition transforms, it doesn't have effect on what we allow in a CREATE TABLE statement. I don't know if we've written such tests earlier, though I suspect we'd get an exception from Iceberg in this case. http://gerrit.cloudera.org:8080/#/c/16939/5/testdata/workloads/functional-query/queries/QueryTest/iceberg-partition-transform-insert.test@723 PS5, Line 723: 1969-12-31-22 > Similar comment I made above about Iceberg spec: "as hours from 1970-01-01 See answer above. http://gerrit.cloudera.org:8080/#/c/16939/5/testdata/workloads/functional-query/queries/QueryTest/iceberg-partition-transform-insert.test@869 PS5, Line 869: ==== > I miss some tests where null values are inserted into partition columns. Do I've created IMPALA-10446 for interop testing with Hive. I think NULL value testing would make sense there. http://gerrit.cloudera.org:8080/#/c/16939/5/testdata/workloads/functional-query/queries/QueryTest/iceberg-partition-transform-insert.test@870 PS5, Line 870: > Well just throwing out ideas but would it make sense to have a partitioned We use the Iceberg APIs for partition pruning, so the above tests also check whether the partition values are correct. I'm planning to add Iceberg to our test environment in the future. So we can write interop tests between Impala and Hive. I've just opened IMPALA-10446 for that. -- To view, visit http://gerrit.cloudera.org:8080/16939 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3edf02048cea78703837b248c55219c22d512b78 Gerrit-Change-Number: 16939 Gerrit-PatchSet: 5 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: wangsheng <[email protected]> Gerrit-Comment-Date: Mon, 18 Jan 2021 18:02:41 +0000 Gerrit-HasComments: Yes
