impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Alex Behm (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-1654: Partition expr in DDL operations.
Date Thu, 01 Sep 2016 04:57:31 GMT
Alex Behm has posted comments on this change.

Change subject: IMPALA-1654: Partition expr in DDL operations.

Patch Set 3:

File fe/src/main/java/com/cloudera/impala/analysis/

Line 86:             if (num == 3) break;
make 3 a static constant with a descriptive name

Line 91:               Joiner.on(", ").join(partitionNames), num<partitions.size() ? "..."
: "."));
style: space before and after "<"
File fe/src/main/java/com/cloudera/impala/analysis/

Line 94:     boolean ignore = false;
The old scenario requires that we have exactly one equality predicate for all partition columns,
and I don't think this logic captures that.

For example, if you have a table partitioned by a, b, c, then we want to obey IF EXISTS only
if there is exactly one <SlotRef>=<Literal> predicate for each partition column.

The current code will obey IF EXISTS in a cases like
ALTER TABLE DROP PARTITION (a=1, b=2) but it should not.

You may find Predicate.isSingleColumnPredicate() useful.

It would be nice to have this logic in a separate function with comment describing the scenarios
and the desire for backwards compatibility.

Also please add a test case like the one I mentioned above to make sure we do not obey IF
EXISTS then.

Line 99:         if (!(bp.getChild(0) instanceof SlotRef ||
The sides could be reversed. I'm not sure if we allowed that previously, but I think we should
treat those cases the same now.
File fe/src/test/java/com/cloudera/impala/analysis/

Line 170:         "Partition exprs cannot contain non-partition column int_col.");
we typically add a colon before the offending expr like this:

Partition exprs cannot contain non-partition column: int_col

Line 209:     // Drop partition using predicates
add comment that IF EXISTS is ignored here

Line 212:     AnalysisError("alter table functional.alltypes drop " +
should not be an error

Line 499:                   "year=2009/month=11 ... Partition expr in set location statements
" +
let's reverse the sentences in the error message, i.e. Partition expr in set ... should come

To view, visit
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c9162fcf9d227b8daf4c2e761d57bab4e26408f
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Amos Bird <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Amos Bird <>
Gerrit-HasComments: Yes

View raw message