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 <tmarsh...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-HasComments: Yes
