Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-3725 Support Kudu UPSERT in Impala ......................................................................
Patch Set 8: (6 comments) http://gerrit.cloudera.org:8080/#/c/4047/8/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java: Line 689: throw new AnalysisException("UPSERT does not currently support any plan hints."); > make this a warning instead like we do for unrecognized hints Done http://gerrit.cloudera.org:8080/#/c/4047/8/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java: Line 3456: public void TestUpsert() { > Let's move this into a new AnalyzeUpsertStmt.java file. This file is alread Done http://gerrit.cloudera.org:8080/#/c/4047/8/fe/src/test/java/org/apache/impala/analysis/ParserTest.java File fe/src/test/java/org/apache/impala/analysis/ParserTest.java: Line 1653: ParsesOk(String.format("upsert into %s t [shuffle] select a from src", optTbl)); > move into TestPlanHints() Done http://gerrit.cloudera.org:8080/#/c/4047/8/testdata/workloads/functional-planner/queries/PlannerTest/kudu-upsert.test File testdata/workloads/functional-planner/queries/PlannerTest/kudu-upsert.test: Line 40: runtime filters: RF000 -> a.string_col > add DISTRIBUTEDPLAN here also just to make sure we can generate one Done http://gerrit.cloudera.org:8080/#/c/4047/8/testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test File testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test: Line 289: upsert into table tdata (id, valf) values (2, 10), (120, 20), (0, 0) > throw in a few NULLs somewhere Great catch! This actually didn't work as written. The latest version fixes it, but the fix is unfortunately somewhat complicated. Line 306: upsert into table tdata (valb, name, id) > add an upsert without a query stmt I assume that you're talking about how the query statement is optional for the parser when the permutation is present, for consistency with insert? If so, its not actually possible to successfully run such a query (at least at the moment), since either you list some columns in the permutation, in which case you will get the error that the number of columns in the permutation don't match the number of columns in the query stmt (which is 0), or else you don't list columns in the permutation, in which case you will get the error that you must list all of the primary key columns, and we already have parser/analyzer tests for those situations. -- 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: 8 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall <[email protected]> Gerrit-Reviewer: Alex Behm <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Matthew Jacobs <[email protected]> Gerrit-Reviewer: Thomas Tauber-Marshall <[email protected]> Gerrit-HasComments: Yes
