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 <jruss...@cloudera.com> Gerrit-Reviewer: Ambreen Kazi <ambreen.k...@cloudera.com> Gerrit-Reviewer: Dimitris Tsirogiannis <dtsirogian...@cloudera.com> Gerrit-Reviewer: Jean-Daniel Cryans <jdcry...@apache.org> Gerrit-Reviewer: John Russell <jruss...@cloudera.com> Gerrit-Reviewer: Matthew Jacobs <m...@cloudera.com> Gerrit-Reviewer: Todd Lipcon <t...@apache.org> Gerrit-HasComments: Yes
