impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Matthew Jacobs (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-3725 Support Kudu UPSERT in Impala
Date Mon, 29 Aug 2016 20:25:58 GMT
Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3725 Support Kudu UPSERT in Impala
......................................................................


Patch Set 2:

(7 comments)

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

PS1, Line 689: !isUpsert_
flip the conditions to avoid using a negative, just easier to read


http://gerrit.cloudera.org:8080/#/c/4047/2/fe/src/main/java/com/cloudera/impala/analysis/InsertStmt.java
File fe/src/main/java/com/cloudera/impala/analysis/InsertStmt.java:

PS2, Line 445: ars
typo


PS2, Line 447:    * - All columns if this is an UPSERT.
As discussed, if Kudu leaves non-referenced columns as-is, then let's remove this restriction.
I think PK columns will still be required.


PS2, Line 689: if (!isUpsert_) {
             :       strBuilder.append("INSERT ");
             :     } else {
             :       strBuilder.append("UPSERT ");
             :     }
Flip the logic; easier to read when, all else being equal, there are fewer negatives.


http://gerrit.cloudera.org:8080/#/c/4047/2/testdata/workloads/functional-planner/queries/PlannerTest/insert.test
File testdata/workloads/functional-planner/queries/PlannerTest/insert.test:

PS2, Line 524: ---- SCANRANGELOCATIONS
             : NODE 0:
             :   HDFS SPLIT hdfs://localhost:20500/test-warehouse/alltypes/year=2009/month=5/090501.txt
0:20853
             : ---- DISTRIBUTEDPLAN
             : UPSERT INTO KUDU [functional_kudu.testtbl]
             : |
             : 00:SCAN HDFS [functional.alltypes]
             :    partitions=1/24 files=1 size=20.36KB
not needed, remove


PS2, Line 541: ---- SCANRANGELOCATIONS
             : 
             : ---- DISTRIBUTEDPLAN
             : UPSERT INTO KUDU [functional_kudu.testtbl]
             : |
             : 00:UNION
             :    constant-operands=2
same


PS2, Line 560: ---- SCANRANGELOCATIONS
             : NODE 0:
             :   HDFS SPLIT hdfs://localhost:20500/test-warehouse/alltypes/year=2009/month=5/090501.txt
0:20853
             : ---- DISTRIBUTEDPLAN
             : UPSERT INTO KUDU [functional_kudu.testtbl]
             : |
             : 01:EXCHANGE [UNPARTITIONED]
             : |  limit: 10
             : |
             : 00:SCAN HDFS [functional.alltypes]
             :    partitions=1/24 files=1 size=20.36KB
             :    limit: 10
same


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I8df5cea36b642e267f85ff6b163f3dd96b8386e9
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarshall@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message