Author: namit
Date: Wed Feb 20 12:56:38 2013
New Revision: 1448138
URL: http://svn.apache.org/r1448138
Log:
HIVE-4027 Thrift alter_table api doesnt validate column type
(Gang Tim Liu via namit)
Modified:
hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/MetaStoreUtils.java
hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
Modified:
hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
URL:
http://svn.apache.org/viewvc/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java?rev=1448138&r1=1448137&r2=1448138&view=diff
==============================================================================
---
hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
(original)
+++
hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveAlterHandler.java
Wed Feb 20 12:56:38 2013
@@ -64,7 +64,7 @@ public class HiveAlterHandler implements
}
if (!MetaStoreUtils.validateName(newt.getTableName())
- || !MetaStoreUtils.validateColNames(newt.getSd().getCols())) {
+ || !MetaStoreUtils.validateTblColumns(newt.getSd().getCols())) {
throw new InvalidOperationException(newt.getTableName()
+ " is not a valid object name");
}
Modified:
hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
URL:
http://svn.apache.org/viewvc/hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java?rev=1448138&r1=1448137&r2=1448138&view=diff
==============================================================================
---
hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
(original)
+++
hive/trunk/metastore/src/java/org/apache/hadoop/hive/metastore/HiveMetaStore.java
Wed Feb 20 12:56:38 2013
@@ -995,9 +995,9 @@ public class HiveMetaStore extends Thrif
InvalidObjectException, NoSuchObjectException {
if (!MetaStoreUtils.validateName(tbl.getTableName())
- || !MetaStoreUtils.validateColNames(tbl.getSd().getCols())
+ || !MetaStoreUtils.validateTblColumns(tbl.getSd().getCols())
|| (tbl.getPartitionKeys() != null && !MetaStoreUtils
- .validateColNames(tbl.getPartitionKeys()))
+ .validateTblColumns(tbl.getPartitionKeys()))
|| !MetaStoreUtils.validateSkewedColNames(
(null == tbl.getSd().getSkewedInfo()) ?
null : tbl.getSd().getSkewedInfo().getSkewedColNames())
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=1448138&r1=1448137&r2=1448138&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
Wed Feb 20 12:56:38 2013
@@ -27,11 +27,13 @@ import java.net.ServerSocket;
import java.net.Socket;
import java.util.ArrayList;
import java.util.HashMap;
+import java.util.HashSet;
import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Map.Entry;
import java.util.Properties;
+import java.util.Set;
import java.util.regex.Matcher;
import java.util.regex.Pattern;
@@ -326,11 +328,38 @@ public class MetaStoreUtils {
return false;
}
- static public boolean validateColNames(List<FieldSchema> cols) {
+ static public boolean validateTblColumns(List<FieldSchema> cols) {
for (FieldSchema fieldSchema : cols) {
if (!validateName(fieldSchema.getName())) {
return false;
}
+ if (!validateColumnType(fieldSchema.getType())) {
+ return false;
+ }
+ }
+ return true;
+ }
+
+ /**
+ * validate column type
+ *
+ * if it is predefined, yes. otherwise no
+ * @param name
+ * @return
+ */
+ static public boolean validateColumnType(String type) {
+ int last = 0;
+ boolean lastAlphaDigit = Character.isLetterOrDigit(type.charAt(last));
+ for (int i = 1; i <= type.length(); i++) {
+ if (i == type.length()
+ || Character.isLetterOrDigit(type.charAt(i)) != lastAlphaDigit) {
+ String token = type.substring(last, i);
+ last = i;
+ if (!hiveThriftTypeMap.contains(token)) {
+ return false;
+ }
+ break;
+ }
}
return true;
}
@@ -419,6 +448,15 @@ public class MetaStoreUtils {
org.apache.hadoop.hive.serde.serdeConstants.DECIMAL_TYPE_NAME,
"decimal");
}
+ static Set<String> hiveThriftTypeMap; //for validation
+ static {
+ hiveThriftTypeMap = new HashSet<String>();
+
hiveThriftTypeMap.addAll(org.apache.hadoop.hive.serde.serdeConstants.PrimitiveTypes);
+
hiveThriftTypeMap.addAll(org.apache.hadoop.hive.serde.serdeConstants.CollectionTypes);
+
hiveThriftTypeMap.add(org.apache.hadoop.hive.serde.serdeConstants.UNION_TYPE_NAME);
+
hiveThriftTypeMap.add(org.apache.hadoop.hive.serde.serdeConstants.STRUCT_TYPE_NAME);
+ }
+
/**
* Convert type to ThriftType. We do that by tokenizing the type and convert
* each token.
Modified:
hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
URL:
http://svn.apache.org/viewvc/hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java?rev=1448138&r1=1448137&r2=1448138&view=diff
==============================================================================
---
hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
(original)
+++
hive/trunk/metastore/src/test/org/apache/hadoop/hive/metastore/TestHiveMetaStore.java
Wed Feb 20 12:56:38 2013
@@ -854,8 +854,8 @@ public abstract class TestHiveMetaStore
public void testDatabaseLocationWithPermissionProblems() throws Exception {
- // Note: The following test will fail if you are running this test as
root. Setting
- // permission to '0' on the database folder will not preclude root from
being able
+ // Note: The following test will fail if you are running this test as
root. Setting
+ // permission to '0' on the database folder will not preclude root from
being able
// to create the necessary files.
if (System.getProperty("user.name").equals("root")) {
@@ -1482,6 +1482,24 @@ public abstract class TestHiveMetaStore
assertTrue("Able to create table with invalid name: " + invTblName,
false);
}
+
+ // create an invalid table which has wrong column type
+ ArrayList<FieldSchema> invColsInvType = new ArrayList<FieldSchema>(2);
+ invColsInvType.add(new FieldSchema("name",
serdeConstants.STRING_TYPE_NAME, ""));
+ invColsInvType.add(new FieldSchema("income", "xyz", ""));
+ tbl.setTableName(tblName);
+ tbl.getSd().setCols(invColsInvType);
+ boolean failChecker = false;
+ try {
+ client.createTable(tbl);
+ } catch (InvalidObjectException ex) {
+ failChecker = true;
+ }
+ if (!failChecker) {
+ assertTrue("Able to create table with invalid column type: " +
invTblName,
+ false);
+ }
+
ArrayList<FieldSchema> cols = new ArrayList<FieldSchema>(2);
cols.add(new FieldSchema("name", serdeConstants.STRING_TYPE_NAME, ""));
cols.add(new FieldSchema("income", serdeConstants.INT_TYPE_NAME, ""));
@@ -1561,6 +1579,17 @@ public abstract class TestHiveMetaStore
assertEquals("alter table didn't move data correct location", tbl3
.getSd().getLocation(), tbl2.getSd().getLocation());
}
+
+ // alter table with invalid column type
+ tbl_pk.getSd().setCols(invColsInvType);
+ failed = false;
+ try {
+ client.alter_table(dbName, tbl2.getTableName(), tbl_pk);
+ } catch (InvalidOperationException ex) {
+ failed = true;
+ }
+ assertTrue("Should not have succeeded in altering column", failed);
+
} catch (Exception e) {
System.err.println(StringUtils.stringifyException(e));
System.err.println("testSimpleTable() failed.");
@@ -1757,7 +1786,7 @@ public abstract class TestHiveMetaStore
} catch (TException e) {
e.printStackTrace();
assert (false);
- }
+ }
assert (threwException);
}
@@ -2106,7 +2135,7 @@ public abstract class TestHiveMetaStore
* at least works correctly.
*/
public void testSynchronized() throws Exception {
- int currentNumberOfDbs = client.getAllDatabases().size();
+ int currentNumberOfDbs = client.getAllDatabases().size();
IMetaStoreClient synchronizedClient =
HiveMetaStoreClient.newSynchronizedClient(client);
