Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/19114 )
Change subject: IMPALA-11607: Support for TPC-H and TPC-DS test datasets stored as Iceberg tables ...................................................................... Patch Set 10: (2 comments) Some small enhancement requests, otherwise LGTM! http://gerrit.cloudera.org:8080/#/c/19114/10/testdata/bin/generate-schema-statements.py File testdata/bin/generate-schema-statements.py: http://gerrit.cloudera.org:8080/#/c/19114/10/testdata/bin/generate-schema-statements.py@351 PS10, Line 351: col.startswith maybe use col.lower().startswith(("primary key", "foreign key")) That will also work for "PrImAry kEy" and "fOreIGN Key" And maybe we should use strip() as well. http://gerrit.cloudera.org:8080/#/c/19114/10/testdata/bin/generate-schema-statements.py@353 PS10, Line 353: # Omit PRIMARY KEY declaration e.g 'col_i INT PRIMARY KEY,' : col = col.replace(" primary key", "").replace(" PRIMARY KEY", "") : # Iceberg tables do not support TINYINT and SMALLINT : col = col.replace(" tinyint", " int").replace(" TINYINT", " INT") : col = col.replace(" smallint", " int").replace(" SMALLINT", " INT") Maybe use case-insensitive regex replace here as well. -- To view, visit http://gerrit.cloudera.org:8080/19114 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ic8bbc5209e7e649f67a48144a2844b35d9f9c7f1 Gerrit-Change-Number: 19114 Gerrit-PatchSet: 10 Gerrit-Owner: Anonymous Coward <lipeng...@sensorsdata.cn> Gerrit-Reviewer: Anonymous Coward <lipeng...@sensorsdata.cn> Gerrit-Reviewer: Gergely Fürnstáhl <gfurnst...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tamas Mate <tma...@apache.org> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Fri, 21 Oct 2022 14:07:49 +0000 Gerrit-HasComments: Yes