diff --git a/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java b/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java index 667d88381..5f47d9d27 100644 --- a/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java +++ b/fe/src/compat-hive-2/java/org/apache/impala/compat/MetastoreShim.java @@ -453,7 +453,7 @@ public class MetastoreShim { } /** - * Return the default table path. + * Return the default table path for a new table. * * Hive-3 doesn't allow managed table to be non transactional after HIVE-22158. * Creating a non transactional managed table will finally result in an external table @@ -461,8 +461,8 @@ public class MetastoreShim { * EXTERNAL, the location will be under "metastore.warehouse.external.dir" (HIVE-19837, * introduces in hive-2.7, not in hive-2.1.x-cdh6.x yet). */ - public static String getNonAcidTablePath(Database db, String tableName) + public static String getPathForNewTable(Database db, Table tbl) throws MetaException { - return new Path(db.getLocationUri(), tableName).toString(); + return new Path(db.getLocationUri(), tbl.getTableName().toLowerCase()).toString(); } } diff --git a/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java b/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java index abdf32164..29fe34f87 100644 --- a/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java +++ b/fe/src/compat-hive-3/java/org/apache/impala/compat/MetastoreShim.java @@ -90,6 +90,7 @@ import org.apache.impala.service.Frontend; import org.apache.impala.service.MetadataOp; import org.apache.impala.thrift.TMetadataOpRequest; import org.apache.impala.thrift.TResultSet; +import org.apache.impala.util.AcidUtils; import org.apache.impala.util.AcidUtils.TblTransaction; import org.apache.log4j.Logger; import org.apache.thrift.TException; @@ -932,7 +933,7 @@ public class MetastoreShim { } /** - * Return the default table path. + * Return the default table path for a new table. * * Hive-3 doesn't allow managed table to be non transactional after HIVE-22158. * Creating a non transactional managed table will finally result in an external table @@ -940,12 +941,19 @@ public class MetastoreShim { * EXTERNAL, the location will be under "metastore.warehouse.external.dir" (HIVE-19837, * introduces in hive-2.7, not in hive-2.1.x-cdh6.x yet). */ - public static String getNonAcidTablePath(Database db, String tableName) + public static String getPathForNewTable(Database db, Table tbl) throws MetaException { Warehouse wh = new Warehouse(new HiveConf()); - // Non ACID managed tables are all translated to external tables by HMS's default - // transformer (HIVE-22158). + // Non transactional tables are all translated to external tables by HMS's default + // transformer (HIVE-22158). Note that external tables can't be transactional. + // So the request and result of the default transformer is: + // non transactional managed table => external table + // non transactional external table => external table + // transactional managed table => managed table + // transactional external table (not allowed) + boolean isExternal = !AcidUtils.isTransactionalTable(tbl.getParameters()); // TODO(IMPALA-9088): deal with customized transformer in HMS. - return wh.getDefaultTablePath(db, tableName, /*isExternal*/ true).toString(); + return wh.getDefaultTablePath(db, tbl.getTableName().toLowerCase(), isExternal) + .toString(); } } diff --git a/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java b/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java index e0a632f32..2c4e44224 100644 --- a/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java +++ b/fe/src/main/java/org/apache/impala/analysis/CreateTableAsSelectStmt.java @@ -214,8 +214,8 @@ public class CreateTableAsSelectStmt extends StatementBase { // Set a valid location of this table using the same rules as the metastore, unless // the user specified a path. if (msTbl.getSd().getLocation() == null || msTbl.getSd().getLocation().isEmpty()) { - msTbl.getSd().setLocation(MetastoreShim.getNonAcidTablePath( - db.getMetaStoreDb(), msTbl.getTableName().toLowerCase())); + msTbl.getSd().setLocation( + MetastoreShim.getPathForNewTable(db.getMetaStoreDb(), msTbl)); } FeTable tmpTable = null; diff --git a/tests/custom_cluster/test_custom_hive_configs.py b/tests/custom_cluster/test_custom_hive_configs.py index 74186bdbc..513c86a19 100644 --- a/tests/custom_cluster/test_custom_hive_configs.py +++ b/tests/custom_cluster/test_custom_hive_configs.py @@ -15,11 +15,12 @@ # specific language governing permissions and limitations # under the License. -from tests.common.custom_cluster_test_suite import CustomClusterTestSuite - import pytest from os import getenv +from tests.common.custom_cluster_test_suite import CustomClusterTestSuite +from tests.common.skip import SkipIfHive2 + HIVE_SITE_EXT_DIR = getenv('IMPALA_HOME') + '/fe/src/test/resources/hive-site-ext' @@ -33,6 +34,7 @@ class TestCustomHiveConfigs(CustomClusterTestSuite): super(TestCustomHiveConfigs, cls).setup_class() # TODO: Remove the xfail marker after bumping CDP_BUILD_NUMBER to contain HIVE-22158 + @SkipIfHive2.acid @pytest.mark.xfail(run=True, reason="May fail on Hive3 versions without HIVE-22158") @pytest.mark.execute_serially @CustomClusterTestSuite.with_args(hive_conf_dir=HIVE_SITE_EXT_DIR) @@ -42,25 +44,37 @@ class TestCustomHiveConfigs(CustomClusterTestSuite): 'metastore.warehouse.external.dir' is different from 'metastore.warehouse.dir' in Hive. """ - self.execute_query_expect_success( - self.client, 'create table %s.ctas_tbl as select 1, 2, "name"' % - unique_database) - res = self.execute_query_expect_success( - self.client, 'select * from %s.ctas_tbl' % unique_database) - assert '1\t2\tname' == res.get_data() + # Test creating non-ACID managed table by CTAS. The HMS transformer will translate it + # into an external table. But we should still be able to read/write it correctly. + self.__check_query_results( + unique_database + '.ctas_tbl', '1\t2\tname', + 'create table %s as select 1, 2, "name"') - self.execute_query_expect_success( - self.client, 'create external table %s.ctas_ext_tbl as select 1, 2, "name"' % - unique_database) + # Test creating non-ACID external table by CTAS. + self.__check_query_results( + unique_database + '.ctas_ext_tbl', '1\t2\tname', + 'create external table %s as select 1, 2, "name"') # Set "external.table.purge"="true" so we can clean files of the external table # finally. self.execute_query_expect_success( self.client, 'alter table %s.ctas_ext_tbl set tblproperties' '("external.table.purge"="true")' % unique_database) - res = self.execute_query_expect_success( - self.client, 'select * from %s.ctas_ext_tbl' % unique_database) - assert '1\t2\tname' == res.get_data() - # Explicitly drop the database with CASCADE to clean files of the external table - self.execute_query_expect_success( - self.client, 'drop database if exists cascade' + unique_database) + # Test creating insert-only ACID managed table by CTAS. + self.__check_query_results( + unique_database + '.insertonly_acid_ctas', '1\t2\tname', + 'create table %s ' + 'tblproperties("transactional"="true", "transactional_properties"="insert_only") ' + 'as select 1, 2, "name"') + + # Test creating insert-only ACID external table by CTAS. Should not be allowed. + self.execute_query_expect_failure( + self.client, + 'create external table %s.insertonly_acid_ext_ctas ' + 'tblproperties("transactional"="true", "transactional_properties"="insert_only") ' + 'as select 1, 2, "name"' % unique_database) + + def __check_query_results(self, table, expected_results, query_format): + self.execute_query_expect_success(self.client, query_format % table) + res = self.execute_query_expect_success(self.client, "select * from " + table) + assert expected_results == res.get_data()