IMPALA-4170: Fix identifier quoting in COMPUTE INCREMENTAL STATS. The SQL statements generated from COMPUTE INCREMENTAL STATS did not properly quote identifiers when incrementally updating the stats for newly added partitions.
Our existing tests did not catch this case because the code paths for doing the initial stats computation and the incremental stats computation are different, in particular, the code for generating the SQL statements. Change-Id: I63adcc45dc964ce769107bf4139fc4566937bb96 Reviewed-on: http://gerrit.cloudera.org:8080/4479 Reviewed-by: Henry Robinson <he...@cloudera.com> Tested-by: Internal Jenkins Project: http://git-wip-us.apache.org/repos/asf/incubator-impala/repo Commit: http://git-wip-us.apache.org/repos/asf/incubator-impala/commit/3aa43516 Tree: http://git-wip-us.apache.org/repos/asf/incubator-impala/tree/3aa43516 Diff: http://git-wip-us.apache.org/repos/asf/incubator-impala/diff/3aa43516 Branch: refs/heads/master Commit: 3aa43516252fe4e75176ab62cb6ce1f04938a96e Parents: 19de09a Author: Alex Behm <alex.b...@cloudera.com> Authored: Tue Sep 20 10:19:29 2016 -0700 Committer: Internal Jenkins <cloudera-hud...@gerrit.cloudera.org> Committed: Wed Sep 21 01:24:53 2016 +0000 ---------------------------------------------------------------------- .../cloudera/impala/catalog/HdfsPartition.java | 27 ++++++------- .../QueryTest/compute-stats-incremental.test | 40 +++++++++++++++----- 2 files changed, 42 insertions(+), 25 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/3aa43516/fe/src/main/java/com/cloudera/impala/catalog/HdfsPartition.java ---------------------------------------------------------------------- diff --git a/fe/src/main/java/com/cloudera/impala/catalog/HdfsPartition.java b/fe/src/main/java/com/cloudera/impala/catalog/HdfsPartition.java index 5deb74f..f408468 100644 --- a/fe/src/main/java/com/cloudera/impala/catalog/HdfsPartition.java +++ b/fe/src/main/java/com/cloudera/impala/catalog/HdfsPartition.java @@ -44,11 +44,11 @@ import com.cloudera.impala.thrift.THdfsCompression; import com.cloudera.impala.thrift.THdfsFileBlock; import com.cloudera.impala.thrift.THdfsFileDesc; import com.cloudera.impala.thrift.THdfsPartition; -import com.cloudera.impala.thrift.THdfsPartitionLocation; import com.cloudera.impala.thrift.TNetworkAddress; import com.cloudera.impala.thrift.TPartitionStats; import com.cloudera.impala.thrift.TTableStats; import com.cloudera.impala.util.HdfsCachingUtil; +import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Joiner; import com.google.common.base.Objects; import com.google.common.base.Preconditions; @@ -57,7 +57,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Lists; import com.google.common.collect.Maps; -import com.google.common.annotations.VisibleForTesting; /** * Query-relevant information for one table partition. Partitions are comparable @@ -337,25 +336,23 @@ public class HdfsPartition implements Comparable<HdfsPartition> { /** * Utility method which returns a string of conjuncts of equality exprs to exactly * select this partition (e.g. ((month=2009) AND (year=2012)). - * TODO: Remove this when TODO elsewhere in this file to save and expose the list of - * TPartitionKeyValues has been resolved. + * TODO: Remove this when the TODO elsewhere in this file to save and expose the + * list of TPartitionKeyValues has been resolved. */ public String getConjunctSql() { - List<String> partitionCols = Lists.newArrayList(); - for (int i = 0; i < getTable().getNumClusteringCols(); ++i) { - partitionCols.add(ToSqlUtils.getIdentSql(getTable().getColumns().get(i).getName())); + List<String> partColSql = Lists.newArrayList(); + for (Column partCol: getTable().getClusteringColumns()) { + partColSql.add(ToSqlUtils.getIdentSql(partCol.getName())); } List<String> conjuncts = Lists.newArrayList(); - for (int i = 0; i < partitionCols.size(); ++i) { - LiteralExpr expr = getPartitionValues().get(i); - String sql = expr.toSql(); - if (expr instanceof NullLiteral || sql.isEmpty()) { - conjuncts.add(ToSqlUtils.getIdentSql(partitionCols.get(i)) - + " IS NULL"); + for (int i = 0; i < partColSql.size(); ++i) { + LiteralExpr partVal = getPartitionValues().get(i); + String partValSql = partVal.toSql(); + if (partVal instanceof NullLiteral || partValSql.isEmpty()) { + conjuncts.add(partColSql.get(i) + " IS NULL"); } else { - conjuncts.add(ToSqlUtils.getIdentSql(partitionCols.get(i)) - + "=" + sql); + conjuncts.add(partColSql.get(i) + "=" + partValSql); } } return "(" + Joiner.on(" AND " ).join(conjuncts) + ")"; http://git-wip-us.apache.org/repos/asf/incubator-impala/blob/3aa43516/testdata/workloads/functional-query/queries/QueryTest/compute-stats-incremental.test ---------------------------------------------------------------------- diff --git a/testdata/workloads/functional-query/queries/QueryTest/compute-stats-incremental.test b/testdata/workloads/functional-query/queries/QueryTest/compute-stats-incremental.test index 382737e..5f98ee7 100644 --- a/testdata/workloads/functional-query/queries/QueryTest/compute-stats-incremental.test +++ b/testdata/workloads/functional-query/queries/QueryTest/compute-stats-incremental.test @@ -324,27 +324,47 @@ STRING, BIGINT, BIGINT, STRING, STRING, STRING, STRING, STRING, STRING ==== ---- QUERY # Check that incremental stats queries handle partitions with keyword names -create table incremental_keyword_part_key(col int) partitioned by -(`date` int); -insert into incremental_keyword_part_key partition(`date`=1) values(2); +create table incremental_keyword_part_key(col int) +partitioned by (`date` int, `table` string); +insert into incremental_keyword_part_key +partition(`date`=1, `table`='a') values(2); compute incremental stats incremental_keyword_part_key -partition(`date`=1); +partition(`date`=1, `table`='a'); show table stats incremental_keyword_part_key; ---- RESULTS -'1',1,1,'2B','NOT CACHED','NOT CACHED','TEXT','true',regex:.* -'Total',1,1,'2B','0B','','','','' +'1','a',1,1,'2B','NOT CACHED','NOT CACHED','TEXT','true',regex:.* +'Total','',1,1,'2B','0B','','','','' ---- TYPES -STRING, BIGINT, BIGINT, STRING, STRING, STRING, STRING, STRING, STRING +STRING, STRING, BIGINT, BIGINT, STRING, STRING, STRING, STRING, STRING, STRING +==== +---- QUERY +# IMPALA-4170: Check that incremental stats queries handles +# new partitions with keyword names. +insert into incremental_keyword_part_key +partition(`date`=2, `table`='b') values(3); +insert into incremental_keyword_part_key +partition(`date`=NULL, `table`='') values(4); +compute incremental stats incremental_keyword_part_key; +show table stats incremental_keyword_part_key; +---- RESULTS +'NULL','NULL',1,1,'2B','NOT CACHED','NOT CACHED','TEXT','true',regex:.* +'1','a',1,1,'2B','NOT CACHED','NOT CACHED','TEXT','true',regex:.* +'2','b',1,1,'2B','NOT CACHED','NOT CACHED','TEXT','true',regex:.* +'Total','',3,3,'6B','0B','','','','' +---- TYPES +STRING, STRING, BIGINT, BIGINT, STRING, STRING, STRING, STRING, STRING, STRING ==== ---- QUERY drop stats incremental_keyword_part_key; compute incremental stats incremental_keyword_part_key; show table stats incremental_keyword_part_key; ---- RESULTS -'1',1,1,'2B','NOT CACHED','NOT CACHED','TEXT','true',regex:.* -'Total',1,1,'2B','0B','','','','' +'NULL','NULL',1,1,'2B','NOT CACHED','NOT CACHED','TEXT','true',regex:.* +'1','a',1,1,'2B','NOT CACHED','NOT CACHED','TEXT','true',regex:.* +'2','b',1,1,'2B','NOT CACHED','NOT CACHED','TEXT','true',regex:.* +'Total','',3,3,'6B','0B','','','','' ---- TYPES -STRING, BIGINT, BIGINT, STRING, STRING, STRING, STRING, STRING, STRING +STRING, STRING, BIGINT, BIGINT, STRING, STRING, STRING, STRING, STRING, STRING ==== ---- QUERY create table incremental_string_part_value(col int) partitioned by
