impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "John Russell (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] Major update to Impala + Kudu page
Date Thu, 19 Jan 2017 06:26:16 GMT
John Russell has posted comments on this change.

Change subject: Major update to Impala + Kudu page
......................................................................


Patch Set 4:

(7 comments)

Addressed comments through the 'EXPLAIN' topic.

http://gerrit.cloudera.org:8080/#/c/5649/4/docs/topics/impala_alter_table.xml
File docs/topics/impala_alter_table.xml:

PS4, Line 887: Any dropped range must not contain
             :       any data values in that range.
> I don't believe this is true -- dropping a range is an efficient way to bul
Done


http://gerrit.cloudera.org:8080/#/c/5649/4/docs/topics/impala_compute_stats.xml
File docs/topics/impala_compute_stats.xml:

Line 55: <!-- Is kudu_partition_spec applicable here? -->
> nope, because afaik we don't have partition-level stats for kudu (they aren
Done


http://gerrit.cloudera.org:8080/#/c/5649/4/docs/topics/impala_create_table.xml
File docs/topics/impala_create_table.xml:

PS4, Line 107: expression
> 'expression' here makes it sound like we support computed defaults. Perhaps
Done. Changed to 'constant'. I was hoping that the default could be any expression evaluated
row-by-row at runtime, like now() or upper(different_column).


Line 122:   RANGE
> this should be RANGE (<varname>pk_col</varname> [, ...]) right?
This is how I create a table with range partitioning but no hash partitioning:

create table range_only (x int primary key)
partition by range (partition 0 <= values <= 50, partition 51 <= values <= 100)
stored as kudu

The name of the primary key column never gets mentioned in the RANGE clause. Is there an alternative
notation that would mention X in this case?


PS4, Line 125: constant
> perhaps say 'tuple' or something instead? for composite PKs you need someth
I'll say constant_or_tuple for the italicized name, and add some examples showing the parens
of the tuple notation. For DDL, I'm going to start by showing examples of the variations on
the main Kudu page, then migrate them to or reuse them on the individual statement pages.


http://gerrit.cloudera.org:8080/#/c/5649/4/docs/topics/impala_explain.xml
File docs/topics/impala_explain.xml:

PS4, Line 239: he <codeph>EXPLAIN</codeph> statement displays equivalent plan
             :       information for queries against Kudu tables as for queries
             :       against HDFS-based tables.
> Don't we need to talk about the predicates that get pushed to Kudu?
Done


http://gerrit.cloudera.org:8080/#/c/5649/4/docs/topics/impala_grant.xml
File docs/topics/impala_grant.xml:

Line 134:       Access to Kudu tables must be granted to roles as usual.
> is it worth noting here that grant/revoke from Impala has no bearing on acc
Done. (From this point on, any changes I mark as done will come in a patch set on Thursday.)


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I76dcb948dab08532fe41326b22ef78d73282db2c
Gerrit-PatchSet: 4
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: John Russell <jrussell@cloudera.com>
Gerrit-Reviewer: Ambreen Kazi <ambreen.kazi@cloudera.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogiannis@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jdcryans@apache.org>
Gerrit-Reviewer: John Russell <jrussell@cloudera.com>
Gerrit-Reviewer: Matthew Jacobs <mj@cloudera.com>
Gerrit-Reviewer: Todd Lipcon <todd@apache.org>
Gerrit-HasComments: Yes

Mime
View raw message