Abhishek Chennaka has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21119 )

Change subject: [blog] blogpost about auto-incrementing column in Kudu
......................................................................


Patch Set 2:

(19 comments)

http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md
File _posts/2024-03-07-introducing-auto-incrementing-column.md:

PS2:
> Please remove trailing spaces in every line of this file.
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@20
PS2, Line 20: declared non-unique
> partially defined
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@21
PS2, Line 21: The
            : system populates the counter upon every INSERT operation
> It would be great to mention this is done at the server side, not at the cl
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@22
PS2, Line 22: partition local
> partition-local
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@29
PS2, Line 29: to the next highest available counter value
> Do you mind deciphering 'next highest available'?  What is the availability
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@35
PS2, Line 35: auto_incrementing_counter
> This is a bit confusing to me: is that "auto_incrementing_id" or "auto_incr
Sorry, this was a typo.


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@46
PS2, Line 46: If the column is really needed
> If the auto-incrementing column's data is needed, ...
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@79
PS2, Line 79: primary key
> partial primary key ?
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@87
PS2, Line 87: for column id
> nit: in the 'id' column
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@104
PS2, Line 104: fetch
> to fetch
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@106
PS2, Line 106: SELECT *, auto_incrementing_id
> I was under impression that Impala would introduce a special session-level
No, there is no such configuration option.


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@107
PS2, Line 107: SELECT *, auto_incrementing_id
> Just to make sure it understand this correctly: will the queries below work
Yes, they do. The logic is as follows:
Unless there is an explicit predicate on "auto_incrementing_id" skip it.


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@120
PS2, Line 120: Update and Delete a row
> nit: Updating and deleting rows
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@125
PS2, Line 125: default> DELETE FROM demo_table where id=2;
             : Query: DELETE FROM demo_table where id=2;
             : Modified 1 row(s), 0 row error(s) in 1.40s
> Do you mind updating this example (or adding an extra one with UPDATE or DE
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@150
PS2, Line 150: Unlike in Impala scanning the table all the table data including 
the auto incrementing column is
> Unlike in Impala, scanning ...
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@150
PS2, Line 150: the table all the table data including the auto incrementing 
column is
             : returned when the entire table is scanned and there is no need 
to explicitly request the
             : auto-incrementing column.
> Consider rewriting this part to be more readable.
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@155
PS2, Line 155: the auto_incrementing column value
> a value for the auto-incrementing column  ?
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@158
PS2, Line 158: #### Examples
> Break this into two separate sentences.
Done


http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@166
PS2, Line 166: existing
> Why adding 'existing'?  Is this something about backwards compatibility?  I
Done



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

Gerrit-Project: kudu
Gerrit-Branch: gh-pages
Gerrit-MessageType: comment
Gerrit-Change-Id: I39f34eea6877a8e050ba2e187ff71555256bf797
Gerrit-Change-Number: 21119
Gerrit-PatchSet: 2
Gerrit-Owner: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Attila Bukor <[email protected]>
Gerrit-Reviewer: Marton Greber <[email protected]>
Gerrit-Reviewer: Wenzhe Zhou <[email protected]>
Gerrit-Comment-Date: Fri, 05 Apr 2024 19:58:02 +0000
Gerrit-HasComments: Yes

Reply via email to