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 <[email protected]> Gerrit-Reviewer: Marcel Kornacker <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-HasComments: Yes
