Alexey Serbin 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: (20 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. 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 http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@21 PS2, Line 21: This is The idea behind the auto-incrementing column is to have a monotonically increasing counter. 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 client side ("system" sounds a bit vague, at least to me). 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 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 criteria in this context? 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_incrementing_counter"? Or it's named differently in various places? 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, ... 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 ? 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 http://gerrit.cloudera.org:8080/#/c/21119/2/_posts/2024-03-07-introducing-auto-incrementing-column.md@104 PS2, Line 104: fetch to fetch 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 config option to do so, no? 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 as expected as well SELECT auto_incrementing_id FROM demo_table; SELECT auto_incrementing_id, * FROM demo_table; 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 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 DELETE statement) using id = 1 predicate to see a report on updating multiple rows at once? 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 ... 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. 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 ? 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. 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? If so, maybe be more explicit about that? -- 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: 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: Tue, 19 Mar 2024 22:15:10 +0000 Gerrit-HasComments: Yes
