commit f25ebe65fcd7e2c5261889790265313327fc162c Author: Dan Burkert Date: Thu Jun 29 15:54:02 2017 -0700 HIVE-16993: ThriftHiveMetastore.create_database can fail if the locationUri is not set diff --git a/metastore/if/hive_metastore.thrift b/metastore/if/hive_metastore.thrift index 042a5d8d0b..433e73b6f8 100755 --- a/metastore/if/hive_metastore.thrift +++ b/metastore/if/hive_metastore.thrift @@ -267,7 +267,7 @@ struct GrantRevokeRoleResponse { struct Database { 1: string name, 2: string description, - 3: string locationUri, + 3: optional string locationUri, 4: map parameters, // properties associated with the database 5: optional PrincipalPrivilegeSet privileges, 6: optional string ownerName, diff --git a/metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp b/metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp index d178f10a21..c1082da3e1 100644 --- a/metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp +++ b/metastore/src/gen/thrift/gen-cpp/hive_metastore_types.cpp @@ -3745,6 +3745,7 @@ void Database::__set_description(const std::string& val) { void Database::__set_locationUri(const std::string& val) { this->locationUri = val; +__isset.locationUri = true; } void Database::__set_parameters(const std::map & val) { @@ -3885,10 +3886,11 @@ uint32_t Database::write(::apache::thrift::protocol::TProtocol* oprot) const { xfer += oprot->writeString(this->description); xfer += oprot->writeFieldEnd(); - xfer += oprot->writeFieldBegin("locationUri", ::apache::thrift::protocol::T_STRING, 3); - xfer += oprot->writeString(this->locationUri); - xfer += oprot->writeFieldEnd(); - + if (this->__isset.locationUri) { + xfer += oprot->writeFieldBegin("locationUri", ::apache::thrift::protocol::T_STRING, 3); + xfer += oprot->writeString(this->locationUri); + xfer += oprot->writeFieldEnd(); + } xfer += oprot->writeFieldBegin("parameters", ::apache::thrift::protocol::T_MAP, 4); { xfer += oprot->writeMapBegin(::apache::thrift::protocol::T_STRING, ::apache::thrift::protocol::T_STRING, static_cast(this->parameters.size())); @@ -3960,7 +3962,7 @@ void Database::printTo(std::ostream& out) const { out << "Database("; out << "name=" << to_string(name); out << ", " << "description=" << to_string(description); - out << ", " << "locationUri=" << to_string(locationUri); + out << ", " << "locationUri="; (__isset.locationUri ? (out << to_string(locationUri)) : (out << "")); out << ", " << "parameters=" << to_string(parameters); out << ", " << "privileges="; (__isset.privileges ? (out << to_string(privileges)) : (out << "")); out << ", " << "ownerName="; (__isset.ownerName ? (out << to_string(ownerName)) : (out << "")); diff --git a/metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h b/metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h index 2bba534975..f401a7f512 100644 --- a/metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h +++ b/metastore/src/gen/thrift/gen-cpp/hive_metastore_types.h @@ -1929,7 +1929,9 @@ class Database { return false; if (!(description == rhs.description)) return false; - if (!(locationUri == rhs.locationUri)) + if (__isset.locationUri != rhs.__isset.locationUri) + return false; + else if (__isset.locationUri && !(locationUri == rhs.locationUri)) return false; if (!(parameters == rhs.parameters)) return false; diff --git a/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Database.java b/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Database.java index 2769845584..277ed5c2c3 100644 --- a/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Database.java +++ b/metastore/src/gen/thrift/gen-javabean/org/apache/hadoop/hive/metastore/api/Database.java @@ -54,7 +54,7 @@ private String name; // required private String description; // required - private String locationUri; // required + private String locationUri; // optional private Map parameters; // required private PrincipalPrivilegeSet privileges; // optional private String ownerName; // optional @@ -141,7 +141,7 @@ public String getFieldName() { } // isset id assignments - private static final _Fields optionals[] = {_Fields.PRIVILEGES,_Fields.OWNER_NAME,_Fields.OWNER_TYPE}; + private static final _Fields optionals[] = {_Fields.LOCATION_URI,_Fields.PRIVILEGES,_Fields.OWNER_NAME,_Fields.OWNER_TYPE}; public static final Map<_Fields, org.apache.thrift.meta_data.FieldMetaData> metaDataMap; static { Map<_Fields, org.apache.thrift.meta_data.FieldMetaData> tmpMap = new EnumMap<_Fields, org.apache.thrift.meta_data.FieldMetaData>(_Fields.class); @@ -149,7 +149,7 @@ public String getFieldName() { new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.STRING))); tmpMap.put(_Fields.DESCRIPTION, new org.apache.thrift.meta_data.FieldMetaData("description", org.apache.thrift.TFieldRequirementType.DEFAULT, new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.STRING))); - tmpMap.put(_Fields.LOCATION_URI, new org.apache.thrift.meta_data.FieldMetaData("locationUri", org.apache.thrift.TFieldRequirementType.DEFAULT, + tmpMap.put(_Fields.LOCATION_URI, new org.apache.thrift.meta_data.FieldMetaData("locationUri", org.apache.thrift.TFieldRequirementType.OPTIONAL, new org.apache.thrift.meta_data.FieldValueMetaData(org.apache.thrift.protocol.TType.STRING))); tmpMap.put(_Fields.PARAMETERS, new org.apache.thrift.meta_data.FieldMetaData("parameters", org.apache.thrift.TFieldRequirementType.DEFAULT, new org.apache.thrift.meta_data.MapMetaData(org.apache.thrift.protocol.TType.MAP, @@ -171,13 +171,11 @@ public Database() { public Database( String name, String description, - String locationUri, Map parameters) { this(); this.name = name; this.description = description; - this.locationUri = locationUri; this.parameters = parameters; } @@ -751,14 +749,16 @@ public String toString() { sb.append(this.description); } first = false; - if (!first) sb.append(", "); - sb.append("locationUri:"); - if (this.locationUri == null) { - sb.append("null"); - } else { - sb.append(this.locationUri); + if (isSetLocationUri()) { + if (!first) sb.append(", "); + sb.append("locationUri:"); + if (this.locationUri == null) { + sb.append("null"); + } else { + sb.append(this.locationUri); + } + first = false; } - first = false; if (!first) sb.append(", "); sb.append("parameters:"); if (this.parameters == null) { @@ -936,9 +936,11 @@ public void write(org.apache.thrift.protocol.TProtocol oprot, Database struct) t oprot.writeFieldEnd(); } if (struct.locationUri != null) { - oprot.writeFieldBegin(LOCATION_URI_FIELD_DESC); - oprot.writeString(struct.locationUri); - oprot.writeFieldEnd(); + if (struct.isSetLocationUri()) { + oprot.writeFieldBegin(LOCATION_URI_FIELD_DESC); + oprot.writeString(struct.locationUri); + oprot.writeFieldEnd(); + } } if (struct.parameters != null) { oprot.writeFieldBegin(PARAMETERS_FIELD_DESC); diff --git a/metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb b/metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb index 02c5717c54..13feb840c1 100644 --- a/metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb +++ b/metastore/src/gen/thrift/gen-rb/hive_metastore_types.rb @@ -662,7 +662,7 @@ class Database FIELDS = { NAME => {:type => ::Thrift::Types::STRING, :name => 'name'}, DESCRIPTION => {:type => ::Thrift::Types::STRING, :name => 'description'}, - LOCATIONURI => {:type => ::Thrift::Types::STRING, :name => 'locationUri'}, + LOCATIONURI => {:type => ::Thrift::Types::STRING, :name => 'locationUri', :optional => true}, PARAMETERS => {:type => ::Thrift::Types::MAP, :name => 'parameters', :key => {:type => ::Thrift::Types::STRING}, :value => {:type => ::Thrift::Types::STRING}}, PRIVILEGES => {:type => ::Thrift::Types::STRUCT, :name => 'privileges', :class => ::PrincipalPrivilegeSet, :optional => true}, OWNERNAME => {:type => ::Thrift::Types::STRING, :name => 'ownerName', :optional => true}, diff --git a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java index 4938fef01c..40cfa1a9fe 100644 --- a/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java +++ b/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java @@ -632,8 +632,8 @@ private void createDefaultDB_core(RawStore ms) throws MetaException, InvalidObje try { ms.getDatabase(DEFAULT_DATABASE_NAME); } catch (NoSuchObjectException e) { - Database db = new Database(DEFAULT_DATABASE_NAME, DEFAULT_DATABASE_COMMENT, - wh.getDefaultDatabasePath(DEFAULT_DATABASE_NAME).toString(), null); + Database db = new Database(DEFAULT_DATABASE_NAME, DEFAULT_DATABASE_COMMENT, null); + db.setLocationUri(wh.getDefaultDatabasePath(DEFAULT_DATABASE_NAME).toString()); db.setOwnerName(PUBLIC); db.setOwnerType(PrincipalType.ROLE); ms.createDatabase(db); diff --git a/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStorePartitionSpecs.java b/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStorePartitionSpecs.java index 61fe7e1d91..1a3d11bbd6 100644 --- a/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStorePartitionSpecs.java +++ b/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStorePartitionSpecs.java @@ -148,7 +148,6 @@ private static void clearAndRecreateDB(HiveMetaStoreClient hmsc) throws Exceptio hmsc.createDatabase(new Database(dbName, "", // Description. - null, // Location. null // Parameters. )); } diff --git a/metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java b/metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java index b28ea73593..ea18a405a1 100644 --- a/metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java +++ b/metastore/src/test/org/apache/hadoop/hive/metastore/TestObjectStore.java @@ -180,8 +180,10 @@ public void testNotificationOps() throws InterruptedException { */ @Test public void testDatabaseOps() throws MetaException, InvalidObjectException, NoSuchObjectException { - Database db1 = new Database(DB1, "description", "locationurl", null); - Database db2 = new Database(DB2, "description", "locationurl", null); + Database db1 = new Database(DB1, "description", null); + db1.setLocationUri("locationurl"); + Database db2 = new Database(DB2, "description", null); + db2.setLocationUri("locationurl"); objectStore.createDatabase(db1); objectStore.createDatabase(db2); @@ -204,7 +206,8 @@ public void testDatabaseOps() throws MetaException, InvalidObjectException, NoSu */ @Test public void testTableOps() throws MetaException, InvalidObjectException, NoSuchObjectException, InvalidInputException { - Database db1 = new Database(DB1, "description", "locationurl", null); + Database db1 = new Database(DB1, "description", null); + db1.setLocationUri("locationurl"); objectStore.createDatabase(db1); StorageDescriptor sd1 = new StorageDescriptor(ImmutableList.of(new FieldSchema("pk_col", "double", null)), "location", null, null, false, 0, new SerDeInfo("SerDeName", "serializationLib", null), @@ -275,7 +278,8 @@ public void testTableOps() throws MetaException, InvalidObjectException, NoSuchO */ @Test public void testPartitionOps() throws MetaException, InvalidObjectException, NoSuchObjectException, InvalidInputException { - Database db1 = new Database(DB1, "description", "locationurl", null); + Database db1 = new Database(DB1, "description", null); + db1.setLocationUri("locationurl"); objectStore.createDatabase(db1); StorageDescriptor sd = new StorageDescriptor(null, "location", null, null, false, 0, new SerDeInfo("SerDeName", "serializationLib", null), null, null, null); HashMap tableParams = new HashMap(); diff --git a/metastore/src/test/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java b/metastore/src/test/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java index 1fa9447145..d38550cbbb 100644 --- a/metastore/src/test/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java +++ b/metastore/src/test/org/apache/hadoop/hive/metastore/cache/TestCachedStore.java @@ -86,7 +86,8 @@ public void testDatabaseOps() throws Exception { String dbLocation = "file:/tmp"; Map dbParams = new HashMap(); String dbOwner = "user1"; - Database db = new Database(dbName, dbDescription, dbLocation, dbParams); + Database db = new Database(dbName, dbDescription, dbParams); + db.setLocationUri(dbLocation); db.setOwnerName(dbOwner); db.setOwnerType(PrincipalType.USER); objectStore.createDatabase(db); @@ -101,7 +102,8 @@ public void testDatabaseOps() throws Exception { // Add another db via CachedStore final String dbName1 = "testDatabaseOps1"; final String dbDescription1 = "testDatabaseOps1"; - Database db1 = new Database(dbName1, dbDescription1, dbLocation, dbParams); + Database db1 = new Database(dbName1, dbDescription1, dbParams); + db.setLocationUri(dbLocation); db1.setOwnerName(dbOwner); db1.setOwnerType(PrincipalType.USER); cachedStore.createDatabase(db1); @@ -112,7 +114,8 @@ public void testDatabaseOps() throws Exception { Assert.assertEquals(db1, dbNew); // Alter the db via CachedStore (can only alter owner or parameters) - db = new Database(dbName, dbDescription, dbLocation, dbParams); + db = new Database(dbName, dbDescription, dbParams); + db.setLocationUri(dbLocation); dbOwner = "user2"; db.setOwnerName(dbOwner); db.setOwnerType(PrincipalType.USER); @@ -126,7 +129,8 @@ public void testDatabaseOps() throws Exception { // Add another db via ObjectStore final String dbName2 = "testDatabaseOps2"; final String dbDescription2 = "testDatabaseOps2"; - Database db2 = new Database(dbName2, dbDescription2, dbLocation, dbParams); + Database db2 = new Database(dbName2, dbDescription2, dbParams); + db2.setLocationUri(dbLocation); db2.setOwnerName(dbOwner); db2.setOwnerType(PrincipalType.USER); objectStore.createDatabase(db2); @@ -134,7 +138,8 @@ public void testDatabaseOps() throws Exception { // Alter db "testDatabaseOps" via ObjectStore dbOwner = "user1"; - db = new Database(dbName, dbDescription, dbLocation, dbParams); + db = new Database(dbName, dbDescription, dbParams); + db.setLocationUri(dbLocation); db.setOwnerName(dbOwner); db.setOwnerType(PrincipalType.USER); objectStore.alterDatabase(dbName, db); @@ -177,7 +182,8 @@ public void testTableOps() throws Exception { String dbLocation = "file:/tmp"; Map dbParams = new HashMap(); String dbOwner = "user1"; - Database db = new Database(dbName, dbDescription, dbLocation, dbParams); + Database db = new Database(dbName, dbDescription, dbParams); + db.setLocationUri(dbLocation); db.setOwnerName(dbOwner); db.setOwnerType(PrincipalType.USER); objectStore.createDatabase(db); @@ -281,7 +287,8 @@ public void testPartitionOps() throws Exception { String dbLocation = "file:/tmp"; Map dbParams = new HashMap(); String dbOwner = "user1"; - Database db = new Database(dbName, dbDescription, dbLocation, dbParams); + Database db = new Database(dbName, dbDescription, dbParams); + db.setLocationUri(dbLocation); db.setOwnerName(dbOwner); db.setOwnerType(PrincipalType.USER); objectStore.createDatabase(db); @@ -382,7 +389,8 @@ public void testTableColStatsOps() throws Exception { String dbLocation = "file:/tmp"; Map dbParams = new HashMap(); String dbOwner = "user1"; - Database db = new Database(dbName, dbDescription, dbLocation, dbParams); + Database db = new Database(dbName, dbDescription, dbParams); + db.setLocationUri(dbLocation); db.setOwnerName(dbOwner); db.setOwnerType(PrincipalType.USER); objectStore.createDatabase(db); @@ -681,7 +689,8 @@ public void testAggrStatsRepeatedRead() throws Exception { String tblName = "tbl"; String colName = "f1"; - Database db = new Database(dbName, null, "some_location", null); + Database db = new Database(dbName, null, null); + db.setLocationUri( "some_location"); cachedStore.createDatabase(db); List cols = new ArrayList(); diff --git a/metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseStore.java b/metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseStore.java index 4aa8c34618..52ceeab457 100644 --- a/metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseStore.java +++ b/metastore/src/test/org/apache/hadoop/hive/metastore/hbase/TestHBaseStore.java @@ -301,7 +301,8 @@ public void init() throws IOException { @Test public void createDb() throws Exception { String dbname = "mydb"; - Database db = new Database(dbname, "no description", "file:///tmp", emptyParameters); + Database db = new Database(dbname, "no description", emptyParameters); + db.setLocationUri("file:///tmp"); store.createDatabase(db); Database d = store.getDatabase(dbname); @@ -313,7 +314,8 @@ public void createDb() throws Exception { @Test public void alterDb() throws Exception { String dbname = "mydb"; - Database db = new Database(dbname, "no description", "file:///tmp", emptyParameters); + Database db = new Database(dbname, "no description", emptyParameters); + db.setLocationUri("file:///tmp"); store.createDatabase(db); db.setDescription("a description"); store.alterDatabase(dbname, db); @@ -327,7 +329,8 @@ public void alterDb() throws Exception { @Test public void dropDb() throws Exception { String dbname = "anotherdb"; - Database db = new Database(dbname, "no description", "file:///tmp", emptyParameters); + Database db = new Database(dbname, "no description", emptyParameters); + db.setLocationUri("file:///tmp"); store.createDatabase(db); Database d = store.getDatabase(dbname);