You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@hive.apache.org by ek...@apache.org on 2017/12/21 04:23:25 UTC

hive git commit: HIVE-18317 - Improve error messages in TransactionalValidationListerner (Eugene Koifman, reviewed by Jason Dere)

Repository: hive
Updated Branches:
  refs/heads/master 9dc02efae -> 21e18dea4


HIVE-18317 - Improve error messages in TransactionalValidationListerner (Eugene Koifman, reviewed by Jason Dere)


Project: http://git-wip-us.apache.org/repos/asf/hive/repo
Commit: http://git-wip-us.apache.org/repos/asf/hive/commit/21e18dea
Tree: http://git-wip-us.apache.org/repos/asf/hive/tree/21e18dea
Diff: http://git-wip-us.apache.org/repos/asf/hive/diff/21e18dea

Branch: refs/heads/master
Commit: 21e18dea4fb3e6e9f9c70282eabdc04a0be569b2
Parents: 9dc02ef
Author: Eugene Koifman <ek...@hortonworks.com>
Authored: Wed Dec 20 20:18:31 2017 -0800
Committer: Eugene Koifman <ek...@hortonworks.com>
Committed: Wed Dec 20 20:18:31 2017 -0800

----------------------------------------------------------------------
 .../hive/metastore/TestHiveMetaStore.java       | 13 ++++----
 .../clientnegative/create_not_acid.q.out        |  2 +-
 .../TransactionalValidationListener.java        | 31 +++++++++++++-------
 3 files changed, 27 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hive/blob/21e18dea/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
----------------------------------------------------------------------
diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
index f344c47..0aa1d4e 100644
--- a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
+++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
@@ -2999,7 +2999,7 @@ public abstract class TestHiveMetaStore extends TestCase {
       Table t = createTable(dbName, tblName, owner, params, null, sd, 0);
       Assert.assertTrue("Expected exception", false);
     } catch (MetaException e) {
-      Assert.assertEquals("'transactional' property of TBLPROPERTIES may only have value 'true'", e.getMessage());
+      Assert.assertEquals("'transactional' property of TBLPROPERTIES may only have value 'true': acidDb.acidTable", e.getMessage());
     }
 
     // Fail - "transactional" property is set to an invalid value
@@ -3009,7 +3009,7 @@ public abstract class TestHiveMetaStore extends TestCase {
       Table t = createTable(dbName, tblName, owner, params, null, sd, 0);
       Assert.assertTrue("Expected exception", false);
     } catch (MetaException e) {
-      Assert.assertEquals("'transactional' property of TBLPROPERTIES may only have value 'true'", e.getMessage());
+      Assert.assertEquals("'transactional' property of TBLPROPERTIES may only have value 'true': acidDb.acidTable", e.getMessage());
     }
 
     // Fail - "transactional" is set to true, but the table is not bucketed
@@ -3019,7 +3019,7 @@ public abstract class TestHiveMetaStore extends TestCase {
       Table t = createTable(dbName, tblName, owner, params, null, sd, 0);
       Assert.assertTrue("Expected exception", false);
     } catch (MetaException e) {
-      Assert.assertEquals("The table must be stored using an ACID compliant format (such as ORC)", e.getMessage());
+      Assert.assertEquals("The table must be stored using an ACID compliant format (such as ORC): acidDb.acidTable", e.getMessage());
     }
 
     // Fail - "transactional" is set to true, and the table is bucketed, but doesn't use ORC
@@ -3032,7 +3032,7 @@ public abstract class TestHiveMetaStore extends TestCase {
       Table t = createTable(dbName, tblName, owner, params, null, sd, 0);
       Assert.assertTrue("Expected exception", false);
     } catch (MetaException e) {
-      Assert.assertEquals("The table must be stored using an ACID compliant format (such as ORC)", e.getMessage());
+      Assert.assertEquals("The table must be stored using an ACID compliant format (such as ORC): acidDb.acidTable", e.getMessage());
     }
 
     // Succeed - "transactional" is set to true, and the table is bucketed, and uses ORC
@@ -3052,12 +3052,11 @@ public abstract class TestHiveMetaStore extends TestCase {
     try {
       params.clear();
       params.put("transactional", "false");
-      t = new Table();
       t.setParameters(params);
       client.alter_table(dbName, tblName, t);
       Assert.assertTrue("Expected exception", false);
     } catch (MetaException e) {
-      Assert.assertEquals("TBLPROPERTIES with 'transactional'='true' cannot be unset", e.getMessage());
+      Assert.assertEquals("TBLPROPERTIES with 'transactional'='true' cannot be unset: aciddb.acidtable", e.getMessage());
     }
 
     // Fail - trying to set "transactional" to "true" but doesn't satisfy bucketing and Input/OutputFormat requirement
@@ -3072,7 +3071,7 @@ public abstract class TestHiveMetaStore extends TestCase {
       client.alter_table(dbName, tblName, t);
       Assert.assertTrue("Expected exception", false);
     } catch (MetaException e) {
-      Assert.assertEquals("The table must be stored using an ACID compliant format (such as ORC)", e.getMessage());
+      Assert.assertEquals("The table must be stored using an ACID compliant format (such as ORC): aciddb.acidtable1", e.getMessage());
     }
 
     // Succeed - trying to set "transactional" to "true", and satisfies bucketing and Input/OutputFormat requirement

http://git-wip-us.apache.org/repos/asf/hive/blob/21e18dea/ql/src/test/results/clientnegative/create_not_acid.q.out
----------------------------------------------------------------------
diff --git a/ql/src/test/results/clientnegative/create_not_acid.q.out b/ql/src/test/results/clientnegative/create_not_acid.q.out
index 4e775e5..e5aad61 100644
--- a/ql/src/test/results/clientnegative/create_not_acid.q.out
+++ b/ql/src/test/results/clientnegative/create_not_acid.q.out
@@ -2,4 +2,4 @@ PREHOOK: query: create table acid_notbucketed(a int, b varchar(128)) TBLPROPERTI
 PREHOOK: type: CREATETABLE
 PREHOOK: Output: database:default
 PREHOOK: Output: default@acid_notbucketed
-FAILED: Execution Error, return code 1 from org.apache.hadoop.hive.ql.exec.DDLTask. MetaException(message:The table must be stored using an ACID compliant format (such as ORC))
+FAILED: Execution Error, return code 1 from org.apache.hadoop.hive.ql.exec.DDLTask. MetaException(message:The table must be stored using an ACID compliant format (such as ORC): default.acid_notbucketed)

http://git-wip-us.apache.org/repos/asf/hive/blob/21e18dea/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
----------------------------------------------------------------------
diff --git a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
index c9ee688..df14752 100644
--- a/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
+++ b/standalone-metastore/src/main/java/org/apache/hadoop/hive/metastore/TransactionalValidationListener.java
@@ -138,7 +138,8 @@ public final class TransactionalValidationListener extends MetaStorePreEventList
       if (!conformToAcid(newTable)) {
         // INSERT_ONLY tables don't have to conform to ACID requirement like ORC or bucketing
         if (transactionalPropertiesValue == null || !"insert_only".equalsIgnoreCase(transactionalPropertiesValue)) {
-          throw new MetaException("The table must be stored using an ACID compliant format (such as ORC)");
+          throw new MetaException("The table must be stored using an ACID compliant format (such as ORC): "
+          + Warehouse.getQualifiedName(newTable));
         }
       }
 
@@ -161,7 +162,8 @@ public final class TransactionalValidationListener extends MetaStorePreEventList
     if (!hasValidTransactionalValue && !MetaStoreUtils.isInsertOnlyTableParam(oldTable.getParameters())) {
       // if here, there is attempt to set transactional to something other than 'true'
       // and NOT the same value it was before
-      throw new MetaException("TBLPROPERTIES with 'transactional'='true' cannot be unset");
+      throw new MetaException("TBLPROPERTIES with 'transactional'='true' cannot be unset: "
+          + Warehouse.getQualifiedName(newTable));
     }
 
     if (isTransactionalPropertiesPresent) {
@@ -230,7 +232,8 @@ public final class TransactionalValidationListener extends MetaStorePreEventList
     if ("false".equalsIgnoreCase(transactional)) {
       // just drop transactional=false.  For backward compatibility in case someone has scripts
       // with transactional=false
-      LOG.info("'transactional'='false' is no longer a valid property and will be ignored");
+      LOG.info("'transactional'='false' is no longer a valid property and will be ignored: " +
+        Warehouse.getQualifiedName(newTable));
       return;
     }
 
@@ -238,7 +241,8 @@ public final class TransactionalValidationListener extends MetaStorePreEventList
       if (!conformToAcid(newTable)) {
         // INSERT_ONLY tables don't have to conform to ACID requirement like ORC or bucketing
         if (transactionalProperties == null || !"insert_only".equalsIgnoreCase(transactionalProperties)) {
-          throw new MetaException("The table must be stored using an ACID compliant format (such as ORC)");
+          throw new MetaException("The table must be stored using an ACID compliant format (such as ORC): "
+              + Warehouse.getQualifiedName(newTable));
         }
       }
 
@@ -257,7 +261,8 @@ public final class TransactionalValidationListener extends MetaStorePreEventList
       return;
     }
     // transactional is found, but the value is not in expected range
-    throw new MetaException("'transactional' property of TBLPROPERTIES may only have value 'true'");
+    throw new MetaException("'transactional' property of TBLPROPERTIES may only have value 'true': "
+        + Warehouse.getQualifiedName(newTable));
   }
 
   /**
@@ -274,11 +279,13 @@ public final class TransactionalValidationListener extends MetaStorePreEventList
    * Check that InputFormatClass/OutputFormatClass should implement
    * AcidInputFormat/AcidOutputFormat
    */
-  private boolean conformToAcid(Table table) throws MetaException {
+  public static boolean conformToAcid(Table table) throws MetaException {
     StorageDescriptor sd = table.getSd();
     try {
-      Class inputFormatClass = Class.forName(sd.getInputFormat());
-      Class outputFormatClass = Class.forName(sd.getOutputFormat());
+      Class inputFormatClass = sd.getInputFormat() == null ? null :
+          Class.forName(sd.getInputFormat());
+      Class outputFormatClass = sd.getOutputFormat() == null ? null :
+          Class.forName(sd.getOutputFormat());
 
       if (inputFormatClass == null || outputFormatClass == null ||
           !Class.forName("org.apache.hadoop.hive.ql.io.AcidInputFormat").isAssignableFrom(inputFormatClass) ||
@@ -286,7 +293,9 @@ public final class TransactionalValidationListener extends MetaStorePreEventList
         return false;
       }
     } catch (ClassNotFoundException e) {
-      throw new MetaException("Invalid input/output format for table");
+      LOG.warn("Could not verify InputFormat=" + sd.getInputFormat() + " or OutputFormat=" +
+          sd.getOutputFormat() + "  for " + Warehouse.getQualifiedName(table));
+      return false;
     }
 
     return true;
@@ -311,8 +320,8 @@ public final class TransactionalValidationListener extends MetaStorePreEventList
           parameters.remove(key);
           String validationError = validateTransactionalProperties(tableTransactionalProperties);
           if (validationError != null) {
-            throw new MetaException("Invalid transactional properties specified for the "
-                + "table with the error " + validationError);
+            throw new MetaException("Invalid transactional properties specified for "
+                + Warehouse.getQualifiedName(table) + " with the error " + validationError);
           }
           break;
         }