Author: navis
Date: Fri Jul 11 02:15:37 2014
New Revision: 1609613
URL: http://svn.apache.org/r1609613
Log:
HIVE-3392 : Hive unnecessarily validates table SerDes when dropping a table
(Navis reviewed by Jason Dere)
Modified:
hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Partition.java
hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java
hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java
hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/CreateTableDesc.java
Modified: hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
URL:
http://svn.apache.org/viewvc/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java?rev=1609613&r1=1609612&r2=1609613&view=diff
==============================================================================
--- hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java
(original)
+++ hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/exec/DDLTask.java Fri Jul
11 02:15:37 2014
@@ -3753,8 +3753,7 @@ public class DDLTask extends Task<DDLWor
tbl.getTTable().getSd().getSerdeInfo().getParameters().putAll(
alterTbl.getProps());
}
- if
(!conf.getStringCollection(ConfVars.SERDESUSINGMETASTOREFORSCHEMA.varname)
- .contains(serdeName)) {
+ if (!Table.hasMetastoreBasedSchema(conf, serdeName)) {
tbl.setFields(Hive.getFieldsFromDeserializer(tbl.getTableName(), tbl.
getDeserializer()));
}
Modified: hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
URL:
http://svn.apache.org/viewvc/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java?rev=1609613&r1=1609612&r2=1609613&view=diff
==============================================================================
--- hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java
(original)
+++ hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Hive.java Fri Jul
11 02:15:37 2014
@@ -58,7 +58,6 @@ import org.apache.hadoop.hive.conf.HiveC
import org.apache.hadoop.hive.metastore.HiveMetaException;
import org.apache.hadoop.hive.metastore.HiveMetaHook;
import org.apache.hadoop.hive.metastore.HiveMetaHookLoader;
-import org.apache.hadoop.hive.metastore.HiveMetaStoreClient;
import org.apache.hadoop.hive.metastore.IMetaStoreClient;
import org.apache.hadoop.hive.metastore.MetaStoreUtils;
import org.apache.hadoop.hive.metastore.RetryingMetaStoreClient;
@@ -213,7 +212,6 @@ public class Hive {
/**
* Hive
*
- * @param argFsRoot
* @param c
*
*/
@@ -402,6 +400,7 @@ public class Hive {
if (newTbl.getParameters() != null) {
newTbl.getParameters().remove(hive_metastoreConstants.DDL_TIME);
}
+ newTbl.checkValidity();
getMSC().alter_table(names[0], names[1], newTbl.getTTable());
} catch (MetaException e) {
throw new HiveException("Unable to alter table.", e);
@@ -469,6 +468,7 @@ public class Hive {
if (newPart.getParameters() != null) {
newPart.getParameters().remove(hive_metastoreConstants.DDL_TIME);
}
+ newPart.checkValidity();
getMSC().alter_partition(dbName, tblName, newPart.getTPartition());
} catch (MetaException e) {
@@ -1018,10 +1018,7 @@ public class Hive {
}
}
- Table table = new Table(tTable);
-
- table.checkValidity();
- return table;
+ return new Table(tTable);
}
/**
Modified:
hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Partition.java
URL:
http://svn.apache.org/viewvc/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Partition.java?rev=1609613&r1=1609612&r2=1609613&view=diff
==============================================================================
--- hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Partition.java
(original)
+++ hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Partition.java
Fri Jul 11 02:15:37 2014
@@ -33,7 +33,6 @@ import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hive.common.JavaUtils;
-import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
import org.apache.hadoop.hive.metastore.MetaStoreUtils;
import org.apache.hadoop.hive.metastore.ProtectMode;
import org.apache.hadoop.hive.metastore.Warehouse;
@@ -46,7 +45,6 @@ import org.apache.hadoop.hive.ql.io.Hive
import org.apache.hadoop.hive.ql.io.HiveOutputFormat;
import org.apache.hadoop.hive.ql.io.HiveSequenceFileOutputFormat;
import org.apache.hadoop.hive.serde2.Deserializer;
-import org.apache.hadoop.hive.serde2.SerDeUtils;
import org.apache.hadoop.mapred.InputFormat;
import org.apache.thrift.TException;
import org.apache.thrift.protocol.TBinaryProtocol;
@@ -504,8 +502,7 @@ public class Partition implements Serial
public List<FieldSchema> getCols() {
try {
- if
(Hive.get().getConf().getStringCollection(ConfVars.SERDESUSINGMETASTOREFORSCHEMA.varname)
- .contains(tPartition.getSd().getSerdeInfo().getSerializationLib())) {
+ if (Table.hasMetastoreBasedSchema(Hive.get().getConf(),
tPartition.getSd())) {
return tPartition.getSd().getCols();
}
return Hive.getFieldsFromDeserializer(table.getTableName(),
getDeserializer());
@@ -644,4 +641,10 @@ public class Partition implements Serial
public Map<List<String>, String> getSkewedColValueLocationMaps() {
return tPartition.getSd().getSkewedInfo().getSkewedColValueLocationMaps();
}
+
+ public void checkValidity() throws HiveException {
+ if (!tPartition.getSd().equals(table.getSd())) {
+ Table.validateColumns(getCols(), table.getPartCols());
+ }
+ }
}
Modified: hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java
URL:
http://svn.apache.org/viewvc/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java?rev=1609613&r1=1609612&r2=1609613&view=diff
==============================================================================
--- hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java
(original)
+++ hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/metadata/Table.java Fri
Jul 11 02:15:37 2014
@@ -23,18 +23,19 @@ import java.io.Serializable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
-import java.util.Iterator;
import java.util.LinkedHashMap;
import java.util.List;
import java.util.Map;
import java.util.Properties;
+import org.apache.commons.lang3.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.hadoop.fs.FileStatus;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hive.common.JavaUtils;
+import org.apache.hadoop.hive.conf.HiveConf;
import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
import org.apache.hadoop.hive.metastore.MetaStoreUtils;
import org.apache.hadoop.hive.metastore.ProtectMode;
@@ -58,7 +59,6 @@ import org.apache.hadoop.hive.serde.serd
import org.apache.hadoop.hive.serde2.Deserializer;
import org.apache.hadoop.hive.serde2.MetadataTypedColumnsetSerDe;
import org.apache.hadoop.hive.serde2.SerDeException;
-import org.apache.hadoop.hive.serde2.SerDeUtils;
import org.apache.hadoop.hive.serde2.objectinspector.StructField;
import org.apache.hadoop.hive.serde2.objectinspector.StructObjectInspector;
import org.apache.hadoop.io.Writable;
@@ -137,7 +137,7 @@ public class Table implements Serializab
}
/**
- * Initialize an emtpy table.
+ * Initialize an empty table.
*/
public static org.apache.hadoop.hive.metastore.api.Table
getEmptyTable(String databaseName, String tableName) {
@@ -210,37 +210,11 @@ public class Table implements Serializab
assert(getViewExpandedText() == null);
}
- Iterator<FieldSchema> iterCols = getCols().iterator();
- List<String> colNames = new ArrayList<String>();
- while (iterCols.hasNext()) {
- String colName = iterCols.next().getName();
- if (!MetaStoreUtils.validateColumnName(colName)) {
- throw new HiveException("Invalid column name '" + colName
- + "' in the table definition");
- }
- Iterator<String> iter = colNames.iterator();
- while (iter.hasNext()) {
- String oldColName = iter.next();
- if (colName.equalsIgnoreCase(oldColName)) {
- throw new HiveException("Duplicate column name " + colName
- + " in the table definition.");
- }
- }
- colNames.add(colName.toLowerCase());
- }
+ validateColumns(getCols(), getPartCols());
+ }
- if (getPartCols() != null) {
- // there is no overlap between columns and partitioning columns
- Iterator<FieldSchema> partColsIter = getPartCols().iterator();
- while (partColsIter.hasNext()) {
- String partCol = partColsIter.next().getName();
- if (colNames.contains(partCol.toLowerCase())) {
- throw new HiveException("Partition column name " + partCol
- + " conflicts with table columns.");
- }
- }
- }
- return;
+ public StorageDescriptor getSd() {
+ return tTable.getSd();
}
public void setInputFormatClass(Class<? extends InputFormat>
inputFormatClass) {
@@ -623,15 +597,15 @@ public class Table implements Serializab
public List<FieldSchema> getCols() {
+ String serializationLib = getSerializationLib();
try {
- if (null == getSerializationLib() ||
Hive.get().getConf().getStringCollection(
-
ConfVars.SERDESUSINGMETASTOREFORSCHEMA.varname).contains(getSerializationLib()))
{
+ if (hasMetastoreBasedSchema(Hive.get().getConf(), serializationLib)) {
return tTable.getSd().getCols();
} else {
return Hive.getFieldsFromDeserializer(getTableName(),
getDeserializer());
}
} catch (HiveException e) {
- LOG.error("Unable to get field from serde: " + getSerializationLib(), e);
+ LOG.error("Unable to get field from serde: " + serializationLib, e);
}
return new ArrayList<FieldSchema>();
}
@@ -1000,4 +974,44 @@ public class Table implements Serializab
public boolean isTemporary() {
return tTable.isTemporary();
}
+
+ public static boolean hasMetastoreBasedSchema(HiveConf conf,
StorageDescriptor serde) {
+ return hasMetastoreBasedSchema(conf,
serde.getSerdeInfo().getSerializationLib());
+ }
+
+ public static boolean hasMetastoreBasedSchema(HiveConf conf, String
serdeLib) {
+ return StringUtils.isEmpty(serdeLib) ||
+
conf.getStringCollection(ConfVars.SERDESUSINGMETASTOREFORSCHEMA.varname).contains(serdeLib);
+ }
+
+ public static void validateColumns(List<FieldSchema> columns,
List<FieldSchema> partCols)
+ throws HiveException {
+ List<String> colNames = new ArrayList<String>();
+ for (FieldSchema partCol: columns) {
+ String colName = normalize(partCol.getName());
+ if (colNames.contains(colName)) {
+ throw new HiveException("Duplicate column name " + colName
+ + " in the table definition.");
+ }
+ colNames.add(colName);
+ }
+ if (partCols != null) {
+ // there is no overlap between columns and partitioning columns
+ for (FieldSchema partCol: partCols) {
+ String colName = normalize(partCol.getName());
+ if (colNames.contains(colName)) {
+ throw new HiveException("Partition column name " + colName
+ + " conflicts with table columns.");
+ }
+ }
+ }
+ }
+
+ private static String normalize(String colName) throws HiveException {
+ if (!MetaStoreUtils.validateColumnName(colName)) {
+ throw new HiveException("Invalid column name '" + colName
+ + "' in the table definition");
+ }
+ return colName.toLowerCase();
+ }
};
Modified:
hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java
URL:
http://svn.apache.org/viewvc/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java?rev=1609613&r1=1609612&r2=1609613&view=diff
==============================================================================
---
hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java
(original)
+++
hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/parse/BaseSemanticAnalyzer.java
Fri Jul 11 02:15:37 2014
@@ -920,7 +920,7 @@ public abstract class BaseSemanticAnalyz
/**
* Sets the table access information.
*
- * @param taInfo The TableAccessInfo structure that is set in the
optimization phase.
+ * @param tableAccessInfo The TableAccessInfo structure that is set in the
optimization phase.
*/
public void setTableAccessInfo(TableAccessInfo tableAccessInfo) {
this.tableAccessInfo = tableAccessInfo;
@@ -1313,7 +1313,7 @@ public abstract class BaseSemanticAnalyz
return getTable(currentDb, tblName, throwException);
}
- // qnName : possibly contains database name (dot seperated)
+ // qnName : possibly contains database name (dot separated)
protected Table getTableWithQN(String qnName, boolean throwException) throws
SemanticException {
int dot = qnName.indexOf('.');
if (dot < 0) {
Modified:
hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/CreateTableDesc.java
URL:
http://svn.apache.org/viewvc/hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/CreateTableDesc.java?rev=1609613&r1=1609612&r2=1609613&view=diff
==============================================================================
--- hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/CreateTableDesc.java
(original)
+++ hive/trunk/ql/src/java/org/apache/hadoop/hive/ql/plan/CreateTableDesc.java
Fri Jul 11 02:15:37 2014
@@ -24,22 +24,20 @@ import java.util.Iterator;
import java.util.List;
import java.util.Map;
-import org.apache.commons.lang.StringUtils;
import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;
import org.apache.hadoop.hive.common.JavaUtils;
import org.apache.hadoop.hive.conf.HiveConf;
-import org.apache.hadoop.hive.conf.HiveConf.ConfVars;
import org.apache.hadoop.hive.metastore.api.FieldSchema;
import org.apache.hadoop.hive.metastore.api.Order;
import org.apache.hadoop.hive.ql.ErrorMsg;
import org.apache.hadoop.hive.ql.exec.Utilities;
import org.apache.hadoop.hive.ql.io.HiveFileFormatUtils;
import org.apache.hadoop.hive.ql.io.HiveOutputFormat;
+import org.apache.hadoop.hive.ql.metadata.Table;
import org.apache.hadoop.hive.ql.parse.BaseSemanticAnalyzer;
import org.apache.hadoop.hive.ql.parse.ParseUtils;
import org.apache.hadoop.hive.ql.parse.SemanticException;
-import org.apache.hadoop.hive.serde2.SerDeUtils;
import org.apache.hadoop.hive.serde2.typeinfo.TypeInfo;
import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
@@ -412,8 +410,7 @@ public class CreateTableDesc extends DDL
if ((this.getCols() == null) || (this.getCols().size() == 0)) {
// for now make sure that serde exists
- if (StringUtils.isEmpty(this.getSerName())
- ||
conf.getStringCollection(ConfVars.SERDESUSINGMETASTOREFORSCHEMA.varname).contains(this.getSerName()))
{
+ if (Table.hasMetastoreBasedSchema(conf, getSerName())) {
throw new SemanticException(ErrorMsg.INVALID_TBL_DDL_SERDE.getMsg());
}
return;
