impala-dev mailing list archives

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

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


Patch Set 1:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/3942/1/fe/src/main/java/com/cloudera/impala/analysis/PartitionSet.java
File fe/src/main/java/com/cloudera/impala/analysis/PartitionSet.java:

Line 132:   // Transform <COL> = NULL into IsNull expr; <String COL> = '' into
IsNull expr and
> Hmm, should I fix that together?
I'm ok with doing the fix separately if that's what you prefer. My thinking was that a lot
of code here will go away for the fix.


http://gerrit.cloudera.org:8080/#/c/3942/1/fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java
File fe/src/main/java/com/cloudera/impala/service/CatalogOpExecutor.java:

Line 1834:         } catch (NoSuchObjectException e) {
> Hive does this by making sure every partition expr should at least match on
Thanks for the info! After thinking this through with Greg, we've concluded this would be
the best course of action with respect to IF EXISTS:

* The Hive behavior seems bizarre and somewhat arbitrary, so let's not adopt that.

* IF EXISTS seems generally not very useful, in particular, since we now return the number
of dropped partitions. Ideally, we should deprecate IF EXISTS, but let's defer that until
a later time and preserve compatibility as much as possible.

As a result, let's do the following:

* Existing cases with fully-specified partitions obey IF EXISTS just as before.

For example, consider a table partitioned by year and month.

ALTER TABLE DROP PARTITION (year=2009, month=1) should throw an error if the partition does
not exist. It would not throw an error when IF EXISTS is specified.

The requirement is that all partition columns are mentioned with <SlotRef>=<Literal>
just like in the old PartitionSpec.

This ensures that we won't break any existing scripts that may rely on the existing behavior
of IF EXISTS.

* For the new cases where we drop partitions based on "arbirtary" exprs, we should just ignore
IF EXISTS.

For example, ALTER TABLE DROP PARTITION (year = 2009) would always succeed, even if no partitions
were dropped. The same applies to ALTER TABLE DROP PARTITION (month < 10) and so on.

Does this make sense to you? We maintain compatibility and the behavior for the new cases
seems sane. Eventually, we should deprecate IF EXISTS imo.


-- 
To view, visit http://gerrit.cloudera.org:8080/3942
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2c9162fcf9d227b8daf4c2e761d57bab4e26408f
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Amos Bird <amosbird@gmail.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Amos Bird <amosbird@gmail.com>
Gerrit-HasComments: Yes

Mime
View raw message