diff --git a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java index dbd2a0f..48834a9 100644 --- a/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java +++ b/common/src/java/org/apache/hadoop/hive/conf/HiveConf.java @@ -150,6 +150,7 @@ 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, @@ -522,6 +523,11 @@ "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" + diff --git a/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreUtils.java b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreUtils.java new file mode 100644 index 0000000..c4dd97e --- /dev/null +++ b/itests/hive-unit/src/test/java/org/apache/hadoop/hive/metastore/TestMetaStoreUtils.java @@ -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 m = new HashMap(); + m.put("akey","aval"); + m.put("blank",""); + m.put("null",null); + + Map 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 m = new HashMap(); + m.put("akey","aval"); + m.put("blank",""); + m.put("null",null); + + Map 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)); + } + + + +} diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java b/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java index 564ac8b..d022283 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreDirectSql.java @@ -108,6 +108,7 @@ */ 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,10 @@ public MetaStoreDirectSql(PersistenceManager pm, Configuration conf) { } this.batchSize = batchSize; + convertMapNullsToEmptyStrings = + HiveConf.getVar(conf, ConfVars.METASTORE_ORM_RETRIEVE_MAPNULLS_AS_EMPTY_STRINGS) + .equalsIgnoreCase("true"); + this.isCompatibleDatastore = ensureDbInit() && runTestQuery(); if (isCompatibleDatastore) { LOG.info("Using direct SQL, underlying DB is " + dbType); @@ -298,7 +303,7 @@ public Database getDatabase(String dbName) throws MetaException{ 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() diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java b/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java index 2db2658..612f927 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java @@ -37,6 +37,8 @@ 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.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 static int getArchivingLevel(Partition part) throws MetaException { } return new String[] {names[0], names[1]}; } + + /** + * Helper function to transform Nulls to empty strings. + */ + private static final com.google.common.base.Function transFormNullsToEmptyString + = new com.google.common.base.Function() { + @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 trimMapNulls( + Map 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())); + } + } + } diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java b/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java index b2dca74..c621904 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java @@ -142,8 +142,6 @@ 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 @@ -577,7 +575,7 @@ public Database getJDODatabase(String name) throws NoSuchObjectException { 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)); @@ -1024,9 +1022,10 @@ private MTable getMTable(String db, String table) { } /** Makes shallow copy of a map to avoid DataNucleus mucking with our objects. */ - private Map convertMap(Map dnMap) { - // Must be deterministic order map - see HIVE-8707 - return (dnMap == null) ? null : Maps.newLinkedHashMap(dnMap); + private Map convertMap(Map dnMap) { + return MetaStoreUtils.trimMapNulls(dnMap, + HiveConf.getVar(getConf(), ConfVars.METASTORE_ORM_RETRIEVE_MAPNULLS_AS_EMPTY_STRINGS) + .equalsIgnoreCase("true")); } private Table convertToTable(MTable mtbl) throws MetaException {