This is an automated email from the ASF dual-hosted git repository. stigahuang pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/impala.git
commit 13e99384ae8e6b86f3a1aedc61dff34fd25b0a84 Author: Eyizoha <[email protected]> AuthorDate: Thu Nov 2 09:59:58 2023 +0800 IMPALA-11776: Use hive.metastore.table.owner for create iceberg table IMPALA-11429 made the creation of an Iceberg table happen in 2 steps. The first step creates the table, however, with wrong owner and the second step is an ALTER TABLE to set the correct table owner. Since Iceberg 1.1.0 there is now a way to provide a table owner via a table property so we can make the create table operation to take one step again. https://github.com/apache/iceberg/pull/5763 https://github.com/apache/iceberg/pull/6154 This patch implements this behavior, when creating an Iceberg table through HiveCatalog, specifying HMS_TABLE_OWNER as hive.metastore.table.owner in properties can do it in one step. Testing: - Use the existing test test_iceberg.py test_table_owner. Change-Id: I56ef7929449105af571d1fb9cb585d9b0733a39d Reviewed-on: http://gerrit.cloudera.org:8080/20646 Reviewed-by: Impala Public Jenkins <[email protected]> Tested-by: Impala Public Jenkins <[email protected]> --- .../apache/impala/service/CatalogOpExecutor.java | 35 +--------------------- .../impala/service/IcebergCatalogOpExecutor.java | 8 ++++- tests/query_test/test_observability.py | 1 - 3 files changed, 8 insertions(+), 36 deletions(-) diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java index b958c352c..170e1d94f 100644 --- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java +++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java @@ -219,7 +219,6 @@ import org.apache.impala.thrift.TIcebergCatalog; import org.apache.impala.thrift.TImpalaTableType; import org.apache.impala.thrift.TIcebergPartitionSpec; import org.apache.impala.thrift.TKuduPartitionParam; -import org.apache.impala.thrift.TOwnerType; import org.apache.impala.thrift.TPartitionDef; import org.apache.impala.thrift.TPartitionKeyValue; import org.apache.impala.thrift.TPartitionStats; @@ -388,8 +387,6 @@ public class CatalogOpExecutor { private static final String LOADED_ICEBERG_TABLE = "Loaded iceberg table"; private static final String SENT_CATALOG_FOR_SYNC_DDL = "Sent catalog update for sync_ddl"; - private static final String SET_ICEBERG_TABLE_OWNER = - "Set Iceberg table owner in Metastore"; private final CatalogServiceCatalog catalog_; private final AuthorizationConfig authzConfig_; @@ -3873,7 +3870,7 @@ public class CatalogOpExecutor { } String tableLoc = IcebergCatalogOpExecutor.createTable(catalog, IcebergUtil.getIcebergTableIdentifier(newTable), location, columns, - partitionSpec, tableProperties).location(); + partitionSpec, newTable.getOwner(), tableProperties).location(); newTable.getSd().setLocation(tableLoc); catalogTimeline.markEvent(CREATED_ICEBERG_TABLE + catalog.name()); } else { @@ -3942,24 +3939,6 @@ public class CatalogOpExecutor { LOG.debug("Created an iceberg table {} in catalog with create event Id {} ", newTbl.getFullName(), createEventId); addTableToCatalogUpdate(newTbl, wantMinimalResult, response.result); - - try { - // Currently we create Iceberg tables using the Iceberg API, however, table owner - // is hardcoded to be the user running the Iceberg process. In our case it's the - // user running Catalogd and not the user running the create table DDL. Hence, an - // extra "ALTER TABLE SET OWNER" step is required. - setIcebergTableOwnerAfterCreateTable(newTable.getDbName(), - newTable.getTableName(), newTable.getOwner()); - catalogTimeline.markEvent(SET_ICEBERG_TABLE_OWNER); - LOG.debug("Table owner has been changed to " + newTable.getOwner()); - } catch (Exception e) { - LOG.warn("Failed to set table owner after creating " + - "Iceberg table but the table {} has been created successfully. Reason: {}", - newTable.toString(), e); - addSummary(response, "Table has been created. However, unable to change table " + - "owner to " + newTable.getOwner()); - return true; - } } catch (Exception e) { if (e instanceof AlreadyExistsException && ifNotExists) { addSummary(response, "Table already exists."); @@ -3975,18 +3954,6 @@ public class CatalogOpExecutor { return true; } - private void setIcebergTableOwnerAfterCreateTable(String dbName, String tableName, - String newOwner) throws ImpalaException { - TAlterTableOrViewSetOwnerParams setOwnerParams = new TAlterTableOrViewSetOwnerParams( - TOwnerType.USER, newOwner); - TAlterTableParams alterTableParams = new TAlterTableParams( - TAlterTableType.SET_OWNER, new TTableName(dbName, tableName)); - alterTableParams.setSet_owner_params(setOwnerParams); - TDdlExecResponse dummyResponse = new TDdlExecResponse(); - dummyResponse.result = new TCatalogUpdateResult(); - - alterTable(alterTableParams, null, true, dummyResponse); - } /** * Creates a new table in the metastore based on the definition of an existing table. * No data is copied as part of this process, it is a metadata only operation. If the diff --git a/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java index 76f6737b8..786e5ff2a 100644 --- a/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java +++ b/fe/src/main/java/org/apache/impala/service/IcebergCatalogOpExecutor.java @@ -45,6 +45,7 @@ import org.apache.iceberg.UpdateSchema; import org.apache.iceberg.catalog.TableIdentifier; import org.apache.iceberg.exceptions.ValidationException; import org.apache.iceberg.expressions.Expressions; +import org.apache.iceberg.hive.HiveCatalog; import org.apache.impala.analysis.IcebergPartitionSpec; import org.apache.impala.catalog.FeIcebergTable; import org.apache.impala.catalog.IcebergTable; @@ -52,6 +53,7 @@ import org.apache.impala.catalog.TableLoadingException; import org.apache.impala.catalog.TableNotFoundException; import org.apache.impala.catalog.events.MetastoreEvents.MetastoreEventPropertyKey; import org.apache.impala.catalog.iceberg.IcebergCatalog; +import org.apache.impala.catalog.iceberg.IcebergHiveCatalog; import org.apache.impala.common.ImpalaRuntimeException; import org.apache.impala.fb.FbIcebergColumnStats; import org.apache.impala.fb.FbIcebergDataFile; @@ -83,11 +85,15 @@ public class IcebergCatalogOpExecutor { */ public static Table createTable(TIcebergCatalog catalog, TableIdentifier identifier, String location, List<TColumn> columns, TIcebergPartitionSpec partitionSpec, - Map<String, String> tableProperties) throws ImpalaRuntimeException { + String owner, Map<String, String> tableProperties) throws ImpalaRuntimeException { // Each table id increase from zero Schema schema = createIcebergSchema(columns); PartitionSpec spec = IcebergUtil.createIcebergPartition(schema, partitionSpec); IcebergCatalog icebergCatalog = IcebergUtil.getIcebergCatalog(catalog, location); + if (icebergCatalog instanceof IcebergHiveCatalog) { + // Put table owner to table properties for HiveCatalog. + tableProperties.put(HiveCatalog.HMS_TABLE_OWNER, owner); + } Table iceTable = icebergCatalog.createTable(identifier, schema, spec, location, tableProperties); LOG.info("Create iceberg table successful."); diff --git a/tests/query_test/test_observability.py b/tests/query_test/test_observability.py index 9ea951aa5..67f5ca282 100644 --- a/tests/query_test/test_observability.py +++ b/tests/query_test/test_observability.py @@ -513,7 +513,6 @@ class TestObservability(ImpalaTestSuite): "Created table using Iceberg Catalog HIVE_CATALOG", "Fetched event batch from Metastore", "Created table in catalog cache", - "Set Iceberg table owner in Metastore", "DDL finished", ])
