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] Updates to DML statements for Impala + Kudu
Date Fri, 27 Jan 2017 20:05:36 GMT
John Russell has posted comments on this change.

Change subject: Updates to DML statements for Impala + Kudu
......................................................................


Patch Set 5:

(11 comments)

Addressed all MJ's comments.

http://gerrit.cloudera.org:8080/#/c/5646/4/docs/topics/impala_delete.xml
File docs/topics/impala_delete.xml:

PS4, Line 79:     <p>
            :       The conditions in the <codeph>WHERE</codeph> clause can refer
to
            :       any combination of primary key columns or other columns. Referring to
            :       pr
> maybe worth mentioning that predicates on the PK will be faster- this still
Done


PS4, Line 89: 
> The
Done


PS4, Line 88: >
            : 
            :     <p>
> seems to duplicate the stmt 2 above
Done


PS4, Line 93:       intuitively expect:
            :     </p>
            :     <ul>
            :       <li>
            :         <p>
            :           If some rows cannot be deleted because their
            :           some primary key columns are not found, due to their being deleted
            :           by a concurrent <codeph>DELETE</codeph> operation,
            :           the statement succeeds but returns a warning.
            :         </p>
            :       </li>
            :       <l
> these could be combined I think and made more clear, e.g.
Done


PS4, Line 108:           or <codeph>UPSERT</codeph> statements running concurrently
on the same table.
> This is not true, we show it in the shell and in the profile (not *DBC/HS2)
Ah I was seeing "zero rows" in a downlevel impala-shell.


PS4, Line 140: -- delete 0 or 1 rows.)
             : DELETE FROM kudu_table WHERE c1 = 100;
> maybe worth mentioning this one would be fastest assuming year, month, day 
Let's save that for when we beef up the "Impala + Kudu Performance" section. Always risky
to get too much into performance in these syntax sections.


http://gerrit.cloudera.org:8080/#/c/5646/4/docs/topics/impala_update.xml
File docs/topics/impala_update.xml:

PS4, Line 62:       The conditions in the <codeph>WHERE</codeph> clause are the
same ones allowed
            :       for the <codeph>SELECT</codeph> statement.
> same comment as in delete case about predicates on PKs will be faster.
Done


PS4, Line 77: 
            :     <p>
            :       Because Kudu currently does not enforce strong consistency during concurrent
DML operations,
            :       be aware that the results after this statement finishes might be different
than you
            :       intuitively expect:
            :     </p>
            :     <ul>
            :       <li>
            :         <p>
> same comment about combining these as in DELETE
Done


PS4, Line 85: 
> this should be updated
Done


http://gerrit.cloudera.org:8080/#/c/5646/4/docs/topics/impala_upsert.xml
File docs/topics/impala_upsert.xml:

PS4, Line 41: <indexterm audience="hidden">UPSERT statement</indexterm>
            :       Acts as a combination of the <codeph>INSERT</codeph>
            :       and <codeph>UPDATE</codeph> statements.
> not sure if we should state this in docs
It is pretty well-known for people familiar with UPSERT (i.e. implied by the name), and for
people who don't know it's a useful bit of conceptual info. I'll leave it in.


PS4, Line 78: 
> this ends up formatted oddly in the pdf, maybe next line or out of the code
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I60512b7957fb53d86d3123a4f1d46fbb355f4665
Gerrit-PatchSet: 5
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