Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201610200553.u9K5r7L2006804@ip-10-146-233-104.ec2.internal>
Date: Thu, 20 Oct 2016 05:53:07 +0000
From: "Alex Behm (Code Review)" <gerrit@cloudera.org>
To: Lars Volker <lv@cloudera.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
CC: Marcel Kornacker <marcel@cloudera.com>
Reply-To: alex.behm@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-2521=3A_Add_clustered_hint_to_insert_statements=0A?=
In-Reply-To: <gerrit.1476752309000.I412153bd8435d792bd61dea268d7a3b884048f14@gerrit.cloudera.org>
References: <gerrit.1476752309000.I412153bd8435d792bd61dea268d7a3b884048f14@gerrit.cloudera.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Content-Disposition: inline
User-Agent: Gerrit/2.12.2
archived-at: Thu, 20 Oct 2016 05:53:51 -0000

Alex Behm has posted comments on this change.

Change subject: IMPALA-2521: Add clustered hint to insert statements
......................................................................


Patch Set 5:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/4745/5/fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
File fe/src/main/java/org/apache/impala/analysis/InsertStmt.java:

Line 132:   private ArrayList<Expr> kuduKeyExprs_ = Lists.newArrayList();
how about primaryKeyExprs_?


Line 517:    * expressions. Result exprs for key columns of kudu tables are stored in kuduKeyExprs_.
Mention that as point 4


Line 601:       // Store exprs for kudu key columns.
Kudu (it's a proper noun)


Line 602:       if (matchFound && table_ instanceof KuduTable) {
You also need to substitute the kuduExprs_ in InsertStmt.substituteResultExprs().

You can try something like this (and add a test):

INSERT INTO kudu_table /*+ clustered */
SELECT * FROM (SELECT c1, c2, k1, k2) FROM src_table


Line 604:         for (String kuduKeyColumnName: kuduTable.getKuduKeyColumnNames()) {
add helper KuduTable.isPrimaryKeyColumn()


http://gerrit.cloudera.org:8080/#/c/4745/5/fe/src/main/java/org/apache/impala/planner/Planner.java
File fe/src/main/java/org/apache/impala/planner/Planner.java:

Line 499:    * (key columns for kudu tables), so that partitions can be written sequentially in the
Kudu tables


http://gerrit.cloudera.org:8080/#/c/4745/5/testdata/workloads/functional-planner/queries/PlannerTest/kudu.test
File testdata/workloads/functional-planner/queries/PlannerTest/kudu.test:

Line 248: insert into table functional_kudu.alltypes /*+ clustered,shuffle */
shuffle/noshuffle don't do anything for Kudu tables, so I don't think this is necessary

they might do something in the future


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I412153bd8435d792bd61dea268d7a3b884048f14
Gerrit-PatchSet: 5
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Lars Volker <lv@cloudera.com>
Gerrit-Reviewer: Marcel Kornacker <marcel@cloudera.com>
Gerrit-HasComments: Yes