impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Thomas Tauber-Marshall (Code Review)" <>
Subject [Impala-ASF-CR] IMPALA-3725 Support Kudu UPSERT in Impala
Date Thu, 20 Oct 2016 22:16:38 GMT
Thomas Tauber-Marshall has posted comments on this change.

Change subject: IMPALA-3725 Support Kudu UPSERT in Impala

Patch Set 8:

File fe/src/main/java/org/apache/impala/analysis/

Line 689:       throw new AnalysisException("UPSERT does not currently support any plan hints.");
> make this a warning instead like we do for unrecognized hints
File fe/src/test/java/org/apache/impala/analysis/

Line 3456:   public void TestUpsert() {
> Let's move this into a new file. This file is alread
File fe/src/test/java/org/apache/impala/analysis/

Line 1653:       ParsesOk(String.format("upsert into %s t [shuffle] select a from src", optTbl));
> move into TestPlanHints()
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
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
To unsubscribe, visit

Gerrit-MessageType: comment
Gerrit-Change-Id: I8df5cea36b642e267f85ff6b163f3dd96b8386e9
Gerrit-PatchSet: 8
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Thomas Tauber-Marshall <>
Gerrit-Reviewer: Alex Behm <>
Gerrit-Reviewer: Dimitris Tsirogiannis <>
Gerrit-Reviewer: Matthew Jacobs <>
Gerrit-Reviewer: Thomas Tauber-Marshall <>
Gerrit-HasComments: Yes

View raw message