Mailing-List: contact reviews-help@impala.incubator.apache.org; run by ezmlm
Precedence: bulk
Message-Id: <201611022230.uA2MUWlp014434@ip-10-146-233-104.ec2.internal>
Date: Wed, 2 Nov 2016 22:30:32 +0000
From: "Matthew Jacobs (Code Review)" <gerrit@cloudera.org>
To: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>, impala-cr@cloudera.com,
        reviews@impala.incubator.apache.org
CC: Alex Behm <alex.behm@cloudera.com>
Reply-To: mj@cloudera.com
Subject: =?UTF-8?Q?=5BImpala-ASF-CR=5D_IMPALA-3724=3A_Support_Kudu_non-covering_range_partitions=0A?=
In-Reply-To: <gerrit.1477520580000.I6799c01a37003f0f4c068d911a13e3f060110a06@gerrit.cloudera.org>
References: <gerrit.1477520580000.I6799c01a37003f0f4c068d911a13e3f060110a06@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: Wed, 02 Nov 2016 22:30:44 -0000

Matthew Jacobs has posted comments on this change.

Change subject: IMPALA-3724: Support Kudu non-covering range partitions
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/4856/3/testdata/workloads/functional-query/queries/QueryTest/kudu_create.test
File testdata/workloads/functional-query/queries/QueryTest/kudu_create.test:

PS3, Line 82: create table tab (a int primary key) distributed by range (a) (partition value = false)
            : stored as kudu
            : ---- CATCH
            : ImpalaRuntimeException: Expected 'int32' literal for column 'a' got 'BOOLEAN'
> Well the checks in the analysis allow this case because boolean is implicit
hm... I would have imagined that the constant evaluation during analysis (e.g. that would evaluate 1+1) returns the expected type?


http://gerrit.cloudera.org:8080/#/c/4856/3/testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test
File testdata/workloads/functional-query/queries/QueryTest/kudu_crud.test:

> Matt this is a not a valid partition syntax. You can't have multiple-column
oh, that's a relief, I didn't realize we had simplified what we're supporting.

The comment about covering all supported key types still holds though, we should have tables somewhere that are created with the various types we support and make sure the plumbing all goes through to Kudu. I think it's fine if there are already create table statements somewhere else that exercise those types w/ range partitions, but I think we should have something that creates range partitions w/ TINYINT, SMALLINT, BIGINT.


Having something that exercises the combination hash + range partitioning is also still relevant.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I6799c01a37003f0f4c068d911a13e3f060110a06
Gerrit-PatchSet: 3
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Alex Behm <alex.behm@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-HasComments: Yes