Thomas Tauber-Marshall has posted comments on this change. Change subject: IMPALA-3725 Support Kudu UPSERT in Impala ......................................................................
Patch Set 10: (8 comments) http://gerrit.cloudera.org:8080/#/c/4047/9/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java: Line 121: // If this is an INSERT, will contain one Expr for all non-partition columns of the > ... will contain one Expr for all non-partition columns of the target table Done Line 124: // If this is an UPSERT, will contain one Expr per column mentioned in the query and > will contain one Expr per mentioned column Done Line 128: > As discussed, please clarify the "if (value == NULL)" part in the BE kudu-t Done Line 131: // Only used for UPSERT, set in prepareExpressions(). > mentionedUpsertColumns_? Done Line 664: ArrayList<Column> columns = table_.getColumnsInHiveOrder(); > ++col Done Line 670: resultExprs_.add(selectListExprs.get(i)); > single line Done Line 754: return TableSink.create(table_, isUpsert_ ? TableSink.Op.UPSERT : TableSink.Op.INSERT, > easier to add a Preconditions.checkState(isUpsert_ || referencedColumns_.is Done http://gerrit.cloudera.org:8080/#/c/4047/9/testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test File testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test: Line 307: select false as valb, 'he' as name, id from tdata where id < 2 > let's also add an upsert test to see what happens when a PK column is NULL It results in the query getting aborted, which as far as I can tell is not something that our query test framework can currently handle. -- 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: 10 Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-Owner: Thomas Tauber-Marshall <tmarsh...@cloudera.com> Gerrit-Reviewer: Alex Behm <alex.b...@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