impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Tauber-Marshall (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-5294: Kudu INSERT partitioning fails with constants
Date Wed, 10 May 2017 01:59:21 GMT
Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-5294: Kudu INSERT partitioning fails with constants
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6828/1/fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java
File fe/src/main/java/org/apache/impala/analysis/KuduPartitionExpr.java:

Line 58:   }
> why can this not be derived from the target table?
Done


http://gerrit.cloudera.org:8080/#/c/6828/1/fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java
File fe/src/main/java/org/apache/impala/planner/DistributedPlanner.java:

Line 251:           (KuduTable) insertStmt.getTargetTable(),
> i don't like this solution, because that ignores whatever else might have b
The KuduPartitionExpr doesn't just need a certain number of exprs, it needs the exact actual
exprs since it uses them to generate the values. Even for a constant, we have to know what
the actual constant is in the BE to send to Kudu.

Currently, nothing else is being done to partitionExprs. Its possible we could add something
in the future, but I'm not sure what we would do that would ever apply to the Kudu case, since
ultimately we're sending the values to Kudu to determine the partition.

Another possible solution would be to add a 'table instanceof KuduTable' check around the
removeConstants() call above, but then we lose the property that if all of the partition exprs
are constants we fall back to UNPARTITIONED, since obviously its pointless to send a bunch
of identical constant rows to Kudu to get the same partition value repeatedly.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I12cbb319f9a5c47fdbfee347b47650186b27f8f9
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message