Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/20429 )
Change subject: IMPALA-12409: Don't allow EXTERNAL Iceberg tables to point another Iceberg table in Hive catalog ...................................................................... Patch Set 2: (5 comments) http://gerrit.cloudera.org:8080/#/c/20429/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/20429/2//COMMIT_MSG@10 PS2, Line 10: Hive Catalog I might miss something but why is this patch restricted to Hive catalog? I managed to achieve the same behaviour in Hadoop catalog too. http://gerrit.cloudera.org:8080/#/c/20429/2/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java File fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java: http://gerrit.cloudera.org:8080/#/c/20429/2/fe/src/main/java/org/apache/impala/analysis/CreateTableStmt.java@778 PS2, Line 778: if (isExternalWithNoPurge() && IcebergUtil.isHiveCatalog(getTblProperties())) { So we are allowed to create such an external table if we provide 'external.table.purge'='true' too? Isn't it still an issue, even bigger because dropping such an external table could also drop the files from the other non-external table? http://gerrit.cloudera.org:8080/#/c/20429/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test File testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test: http://gerrit.cloudera.org:8080/#/c/20429/2/testdata/workloads/functional-query/queries/QueryTest/iceberg-negative.test@103 PS2, Line 103: functioanal nit: I love the name functioanal :D http://gerrit.cloudera.org:8080/#/c/20429/2/tests/query_test/test_iceberg.py File tests/query_test/test_iceberg.py: http://gerrit.cloudera.org:8080/#/c/20429/2/tests/query_test/test_iceberg.py@1096 PS2, Line 1096: self.run_stmt_in_hive Just curious: Is there a similar working going on Hive side to reject setting such properties? If yes, then later on pulling in a new Hive version to Impala would make these tests fail, that is fine we should just be aware of this. http://gerrit.cloudera.org:8080/#/c/20429/2/tests/query_test/test_iceberg.py@1103 PS2, Line 1103: self.run_stmt_in_hive("""alter table {0} set tblproperties We have already altered the 'iceberg.table_identifier' of this table so refresh would give us the expected error message anyway. How can we differentiate that the error message now comes because we've set 'iceberg.mr.table.identifier' and not because we've set 'iceberg.table_identifier'? -- To view, visit http://gerrit.cloudera.org:8080/20429 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ifb0d7f0e7ec40fba356bd58b43f68d070432de71 Gerrit-Change-Number: 20429 Gerrit-PatchSet: 2 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Tamas Mate <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Thu, 31 Aug 2023 08:29:27 +0000 Gerrit-HasComments: Yes
