svn commit: r1654909 - in /hive/trunk: common/src/java/org/apache/hadoop/hive/conf/ itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/ metastore/src/java/org/apache/hadoop/hive/metastore/

[kh...@apache.org]

Author: khorgath
Date: Mon Jan 26 23:20:01 2015
New Revision: 1654909

URL: http://svn.apache.org/r1654909
Log:
HIVE-8485 : HMS on Oracle incompatibility (Sushanth Sowmyan, reviewed by Sergey Shelukhin, Chaoyu Tang)

Added:
    hive/trunk/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreUtils.java
Modified:
    hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
    hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
    hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
    hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java

Modified: hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java
URL: http://svn.apache.org/viewvc/hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java?rev=1654909&r1=1654908&r2=1654909&view=diff
==============================================================================
--- hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java (original)
+++ hive/trunk/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java Mon Jan 26 23:20:01 2015
@@ -159,6 +159,7 @@ public class HiveConf extends Configurat
       HiveConf.ConfVars.HMSHANDLERINTERVAL,
       HiveConf.ConfVars.HMSHANDLERFORCERELOADCONF,
       HiveConf.ConfVars.METASTORE_PARTITION_NAME_WHITELIST_PATTERN,
+      HiveConf.ConfVars.METASTORE_ORM_RETRIEVE_MAPNULLS_AS_EMPTY_STRINGS,
       HiveConf.ConfVars.METASTORE_DISALLOW_INCOMPATIBLE_COL_TYPE_CHANGES,
       HiveConf.ConfVars.USERS_IN_ADMIN_ROLE,
       HiveConf.ConfVars.HIVE_AUTHORIZATION_MANAGER,
@@ -545,6 +546,11 @@ public class HiveConf extends Configurat
         "select query has incorrect syntax or something similar inside a transaction, the\n" +
         "entire transaction will fail and fall-back to DataNucleus will not be possible. You\n" +
         "should disable the usage of direct SQL inside transactions if that happens in your case."),
+    METASTORE_ORM_RETRIEVE_MAPNULLS_AS_EMPTY_STRINGS("hive.metastore.orm.retrieveMapNullsAsEmptyStrings",false,
+        "Thrift does not support nulls in maps, so any nulls present in maps retrieved from ORM must " +
+        "either be pruned or converted to empty strings. Some backing dbs such as Oracle persist empty strings " +
+        "as nulls, so we should set this parameter if we wish to reverse that behaviour. For others, " +
+        "pruning is the correct behaviour"),
     METASTORE_DISALLOW_INCOMPATIBLE_COL_TYPE_CHANGES(
         "hive.metastore.disallow.incompatible.col.type.changes", false,
         "If true (default is false), ALTER TABLE operations which change the type of a\n" +

Added: hive/trunk/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreUtils.java
URL: http://svn.apache.org/viewvc/hive/trunk/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreUtils.java?rev=1654909&view=auto
==============================================================================
--- hive/trunk/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreUtils.java (added)
+++ hive/trunk/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreUtils.java Mon Jan 26 23:20:01 2015
@@ -0,0 +1,62 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.metastore;
+
+import junit.framework.TestCase;
+
+import java.util.HashMap;
+import java.util.Map;
+
+public class TestMetaStoreUtils extends TestCase {
+
+  public void testTrimMapNullsXform() throws Exception {
+    Map<String,String> m = new HashMap<String,String>();
+    m.put("akey","aval");
+    m.put("blank","");
+    m.put("null",null);
+
+    Map<String,String> xformed = MetaStoreUtils.trimMapNulls(m,true);
+    assertEquals(3,xformed.size());
+    assert(xformed.containsKey("akey"));
+    assert(xformed.containsKey("blank"));
+    assert(xformed.containsKey("null"));
+    assertEquals("aval",xformed.get("akey"));
+    assertEquals("",xformed.get("blank"));
+    assertEquals("",xformed.get("null"));
+  }
+
+  public void testTrimMapNullsPrune() throws Exception {
+    Map<String,String> m = new HashMap<String,String>();
+    m.put("akey","aval");
+    m.put("blank","");
+    m.put("null",null);
+
+    Map<String,String> pruned = MetaStoreUtils.trimMapNulls(m,false);
+    assertEquals(2,pruned.size());
+    assert(pruned.containsKey("akey"));
+    assert(pruned.containsKey("blank"));
+    assert(!pruned.containsKey("null"));
+    assertEquals("aval",pruned.get("akey"));
+    assertEquals("",pruned.get("blank"));
+    assert(!pruned.containsValue(null));
+  }
+
+
+
+}

Modified: hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java
URL: http://svn.apache.org/viewvc/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java?rev=1654909&r1=1654908&r2=1654909&view=diff
==============================================================================
--- hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java (original)
+++ hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java Mon Jan 26 23:20:01 2015
@@ -108,6 +108,7 @@ class MetaStoreDirectSql {
    */
   private final DB dbType;
   private final int batchSize;
+  private final boolean convertMapNullsToEmptyStrings;
 
   /**
    * Whether direct SQL can be used with the current datastore backing {@link #pm}.
@@ -123,6 +124,9 @@ class MetaStoreDirectSql {
     }
     this.batchSize = batchSize;
 
+    convertMapNullsToEmptyStrings =
+        HiveConf.getBoolVar(conf, ConfVars.METASTORE_ORM_RETRIEVE_MAPNULLS_AS_EMPTY_STRINGS);
+
     this.isCompatibleDatastore = ensureDbInit() && runTestQuery();
     if (isCompatibleDatastore) {
       LOG.info("Using direct SQL, underlying DB is " + dbType);
@@ -298,7 +302,7 @@ class MetaStoreDirectSql {
       String type = extractSqlString(dbline[5]);
       db.setOwnerType(
           (null == type || type.trim().isEmpty()) ? null : PrincipalType.valueOf(type));
-      db.setParameters(dbParams);
+      db.setParameters(MetaStoreUtils.trimMapNulls(dbParams,convertMapNullsToEmptyStrings));
       if (LOG.isDebugEnabled()){
         LOG.debug("getDatabase: directsql returning db " + db.getName()
             + " locn["+db.getLocationUri()  +"] desc [" +db.getDescription()

Modified: hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
URL: http://svn.apache.org/viewvc/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java?rev=1654909&r1=1654908&r2=1654909&view=diff
==============================================================================
--- hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java (original)
+++ hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java Mon Jan 26 23:20:01 2015
@@ -37,6 +37,8 @@ import java.util.Set;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
+import com.google.common.base.Predicates;
+import com.google.common.collect.Maps;
 import org.apache.commons.lang.StringUtils;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -76,6 +78,8 @@ import org.apache.hadoop.hive.shims.Shim
 import org.apache.hadoop.hive.thrift.HadoopThriftAuthBridge;
 import org.apache.hadoop.util.ReflectionUtils;
 
+import javax.annotation.Nullable;
+
 public class MetaStoreUtils {
 
   protected static final Log LOG = LogFactory.getLog("hive.log");
@@ -1609,4 +1613,44 @@ public class MetaStoreUtils {
     }
     return new String[] {names[0], names[1]};
   }
+
+  /**
+   * Helper function to transform Nulls to empty strings.
+   */
+  private static final com.google.common.base.Function<String,String> transFormNullsToEmptyString
+      = new com.google.common.base.Function<String, String>() {
+    @Override
+    public java.lang.String apply(@Nullable java.lang.String string) {
+      if (string == null){
+        return "";
+      } else {
+        return string;
+      }
+    }
+  };
+
+  /**
+   * We have aneed to sanity-check the map before conversion from persisted objects to
+   * metadata thrift objects because null values in maps will cause a NPE if we send
+   * across thrift. Pruning is appropriate for most cases except for databases such as
+   * Oracle where Empty strings are stored as nulls, in which case we need to handle that.
+   * See HIVE-8485 for motivations for this.
+   */
+  public static Map<String,String> trimMapNulls(
+      Map<String,String> dnMap, boolean retrieveMapNullsAsEmptyStrings){
+    if (dnMap == null){
+      return null;
+    }
+    // Must be deterministic order map - see HIVE-8707
+    //   => we use Maps.newLinkedHashMap instead of Maps.newHashMap
+    if (retrieveMapNullsAsEmptyStrings) {
+      // convert any nulls present in map values to empty strings - this is done in the case
+      // of backing dbs like oracle which persist empty strings as nulls.
+      return Maps.newLinkedHashMap(Maps.transformValues(dnMap, transFormNullsToEmptyString));
+    } else {
+      // prune any nulls present in map values - this is the typical case.
+      return Maps.newLinkedHashMap(Maps.filterValues(dnMap, Predicates.notNull()));
+    }
+  }
+
 }

Modified: hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java
URL: http://svn.apache.org/viewvc/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java?rev=1654909&r1=1654908&r2=1654909&view=diff
==============================================================================
--- hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java (original)
+++ hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java Mon Jan 26 23:20:01 2015
@@ -148,8 +148,6 @@ import org.apache.thrift.TException;
 import org.datanucleus.store.rdbms.exceptions.MissingTableException;
 
 import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
-
 
 /**
  * This class is the interface between the application logic and the database
@@ -583,7 +581,7 @@ public class ObjectStore implements RawS
     db.setName(mdb.getName());
     db.setDescription(mdb.getDescription());
     db.setLocationUri(mdb.getLocationUri());
-    db.setParameters(mdb.getParameters());
+    db.setParameters(convertMap(mdb.getParameters()));
     db.setOwnerName(mdb.getOwnerName());
     String type = mdb.getOwnerType();
     db.setOwnerType((null == type || type.trim().isEmpty()) ? null : PrincipalType.valueOf(type));
@@ -1030,9 +1028,9 @@ public class ObjectStore implements RawS
   }
 
   /** Makes shallow copy of a map to avoid DataNucleus mucking with our objects. */
-  private <K, V> Map<K, V> convertMap(Map<K, V> dnMap) {
-    // Must be deterministic order map - see HIVE-8707
-    return (dnMap == null) ? null : Maps.newLinkedHashMap(dnMap);
+  private Map<String, String> convertMap(Map<String, String> dnMap) {
+    return MetaStoreUtils.trimMapNulls(dnMap,
+        HiveConf.getBoolVar(getConf(), ConfVars.METASTORE_ORM_RETRIEVE_MAPNULLS_AS_EMPTY_STRINGS));
   }
 
   private Table convertToTable(MTable mtbl) throws MetaException {