From 96f2831ae44c9178a5d09dcf8de30ff62854c08c Mon Sep 17 00:00:00 2001 From: Alexander Kolbasov Date: Wed, 15 Aug 2018 16:13:31 -0700 Subject: [PATCH 1/1] HIVE-19253: HMS ignores tableType property for external tables --- .../apache/hadoop/hive/metastore/ObjectStore.java | 19 ++---- .../hive/metastore/utils/MetaStoreUtils.java | 9 ++- .../hadoop/hive/metastore/TestObjectStore.java | 75 ++++++++++++++++++++++ 3 files changed, 89 insertions(+), 14 deletions(-) diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java index 8e2f94eb69..f1fa36e380 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/ObjectStore.java @@ -1911,7 +1911,7 @@ private Table convertToTable(MTable mtbl) throws MetaException { // for backwards compatibility with old metastore persistence if (mtbl.getViewOriginalText() != null) { tableType = TableType.VIRTUAL_VIEW.toString(); - } else if (Boolean.parseBoolean(mtbl.getParameters().get("EXTERNAL"))) { + } else if (Boolean.parseBoolean(mtbl.getParameters().get(MetaStoreUtils.EXTERNAL_TABLE_PROP))) { tableType = TableType.EXTERNAL_TABLE.toString(); } else { tableType = TableType.MANAGED_TABLE.toString(); @@ -1954,19 +1954,12 @@ private MTable convertToMTable(Table tbl) throws InvalidObjectException, DatabaseName.getQualified(catName, tbl.getDbName()) + " doesn't exist."); } - // If the table has property EXTERNAL set, update table type - // accordingly + // If the table type is set as MANAGED, but the table has property + // EXTERNAL, change the table type to EXTERNAL as well. String tableType = tbl.getTableType(); - boolean isExternal = Boolean.parseBoolean(tbl.getParameters().get("EXTERNAL")); - if (TableType.MANAGED_TABLE.toString().equals(tableType)) { - if (isExternal) { - tableType = TableType.EXTERNAL_TABLE.toString(); - } - } - if (TableType.EXTERNAL_TABLE.toString().equals(tableType)) { - if (!isExternal) { - tableType = TableType.MANAGED_TABLE.toString(); - } + if (TableType.MANAGED_TABLE.name().equals(tableType) && + Boolean.parseBoolean(tbl.getParameters().get(MetaStoreUtils.EXTERNAL_TABLE_PROP))) { + tableType = TableType.EXTERNAL_TABLE.name(); } PrincipalType ownerPrincipalType = tbl.getOwnerType(); diff --git a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java index 5233bee592..39d1169e6a 100644 --- a/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java +++ b/standalone-metastore/metastore-server/src/main/java/org/apache/hadoop/hive/metastore/utils/MetaStoreUtils.java @@ -100,6 +100,9 @@ protected DateFormat initialValue() { public static final String EXTERNAL_TABLE_PURGE = "external.table.purge"; + /** Table property marking table as external. */ + public static final String EXTERNAL_TABLE_PROP = "EXTERNAL"; + // Right now we only support one special character '/'. // More special characters can be added accordingly in the future. // NOTE: @@ -108,6 +111,7 @@ protected DateFormat initialValue() { // HIVE_SUPPORT_SPECICAL_CHARACTERS_IN_TABLE_NAMES in HiveConf as well. private static final char[] specialCharactersInTableNames = new char[] { '/' }; + /** * Catches exceptions that can't be handled and bundles them to MetaException * @@ -210,6 +214,9 @@ public static boolean isExternalTable(Table table) { if (table == null) { return false; } + if (table.getTableType().equals(TableType.EXTERNAL_TABLE.name())) { + return true; + } Map params = table.getParameters(); if (params == null) { return false; @@ -238,7 +245,7 @@ public static boolean isExternalTablePurge(Table table) { } public static boolean isExternal(Map tableParams){ - return isPropertyTrue(tableParams, "EXTERNAL"); + return isPropertyTrue(tableParams, EXTERNAL_TABLE_PROP); } public static boolean isPropertyTrue(Map tableParams, String prop) { diff --git a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java index b74c3048fa..6ed557deb6 100644 --- a/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java +++ b/standalone-metastore/metastore-server/src/test/java/org/apache/hadoop/hive/metastore/TestObjectStore.java @@ -65,6 +65,8 @@ import org.apache.hadoop.hive.metastore.metrics.MetricsConstants; import org.apache.hadoop.hive.metastore.model.MNotificationLog; import org.apache.hadoop.hive.metastore.model.MNotificationNextId; +import org.apache.hadoop.hive.metastore.model.MTable; +import org.apache.hadoop.hive.metastore.utils.MetaStoreUtils; import org.junit.Assert; import org.junit.Assume; import org.junit.Before; @@ -86,6 +88,7 @@ import java.util.HashMap; import java.util.HashSet; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.BrokenBarrierException; import java.util.concurrent.CyclicBarrier; @@ -811,6 +814,64 @@ public void testNotificationOps() throws InterruptedException, MetaException { Assert.assertEquals(0, eventResponse.getEventsSize()); } + /** + * Verify that table type is set correctly based on input table properties. + * Two things are verified: + *
    + *
  1. When EXTERNAL property is set to true, table type should be external
  2. + *
  3. When table type is set to external it should remain external + *
+ * @throws Exception + */ + @Test + public void testExternalTable() throws Exception { + Database db1 = new DatabaseBuilder() + .setName(DB1) + .setDescription("description") + .setLocation("locationurl") + .build(conf); + objectStore.createDatabase(db1); + + List tables = new ArrayList<>(4); + Map expectedValues = new HashMap<>(); + + int i = 1; + // Case 1: EXTERNAL = true, tableType == MANAGED_TABLE + // The result should be external table + Table tbl1 = buildTable(conf, db1, "t" + i++, true, null); + tables.add(tbl1); + expectedValues.put(tbl1.getTableName(), true); + // Case 2: EXTERNAL = false, tableType == EXTERNAL_TABLE + // The result should be external table + Table tbl2 = buildTable(conf, db1, "t" + i++, false, TableType.EXTERNAL_TABLE.name()); + tables.add(tbl2); + expectedValues.put(tbl2.getTableName(), true); + // Case 3: EXTERNAL = false, tableType == EXTERNAL_TABLE + // The result should be external table + Table tbl3 = buildTable(conf, db1, "t" + i++, true, TableType.EXTERNAL_TABLE.name()); + tables.add(tbl3); + expectedValues.put(tbl3.getTableName(), true); + // Case 4: EXTERNAL = false, tableType == MANAGED_TABLE + // The result should be managed table + Table tbl4 = buildTable(conf, db1, "t" + i++, false, null); + tables.add(tbl4); + expectedValues.put(tbl4.getTableName(), false); + + for (Table t: tables) { + objectStore.createTable(t); + + Table tOut = objectStore.getTable(db1.getCatalogName(), DB1, t.getTableName()); + Assert.assertEquals("Table " + t.getTableName() + "has invalid external state", + expectedValues.get(t.getTableName()), MetaStoreUtils.isExternalTable(tOut)); + } + + for (Table t: tables) { + objectStore.dropTable(db1.getCatalogName(), DB1, t.getTableName()); + } + + objectStore.dropDatabase(db1.getCatalogName(), DB1); + } + /** * Test metastore configuration property METASTORE_MAX_EVENT_RESPONSE */ @@ -918,5 +979,19 @@ private void createTestCatalog(String catName) throws MetaException { .build(); objectStore.createCatalog(cat); } + + private Table buildTable(Configuration conf, Database db, String name, boolean external_prop, + String tableType) throws MetaException { + Table tbl = new TableBuilder() + .inDb(db) + .setTableName(name) + .addCol("col1", ColumnType.STRING_TYPE_NAME) + .addTableParam(MetaStoreUtils.EXTERNAL_TABLE_PROP, String.valueOf(external_prop)) + .build(conf); + if (tableType != null) { + tbl.setTableType(tableType); + } + return tbl; + } } -- 2.16.3