Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/19690 )
Change subject: UDU-1945 Add C++ example for non-unique PK ...................................................................... Patch Set 2: (28 comments) http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/getting_started.cc File examples/cpp/getting_started.cc: PS1: > Once this file changed, you need to update README.adoc in the corresponding I just wanted to rename this file, to indicate the the example.cc is the general getting started, and others (like this non_unique_primary_key.cc) would be feature specific. I will keep the original example.cc filename, to avoid unnecessary headaches for now, and later I can do the renaming, if you find it also reasonable. Thanks for mentioning this issue! http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc File examples/cpp/non_unique_primary_key.cc: http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@80 PS1, Line 80: ey column for ha > Could we get column name from input schema? Actually this is a good point, as I can also add explanation about the ordering of the key cols: to mention that auto-incrementing column is always the last, so we can use schema.Column(0). Thanks for pointing this out! http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@98 PS1, Line 98: st > indent in two spaces Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@102 PS1, Line 102: > nit: incrementing Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@109 PS1, Line 109: } : : S > remove these three lines Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@118 PS1, Line 118: for (int i = 0; i < predicates.size(); i++) { > add space after 'for', add a space before { Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@128 PS1, Line 128: one do > Remind me if I am wrong but we currently have the default implementation on You are correct. I just wanted to highlight that this is the default behaviour, and if someone wants to change this, they can introduce a projection. Changed to commend to reflect this. http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@140 PS1, Line 140: KuduScanner scanner(table.get()); > add space after 'for', add a space before { Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@158 PS1, Line 158: KU > nit: indent in 4 spaces Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@165 PS1, Line 165: KU > nit: indent in 4 spaces Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@172 PS1, Line 172: Status s = session->Flush(); : KUDU_RETURN_NOT_OK(s); : r > remove Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@182 PS1, Line 182: = 0; > nit here and above: delete Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@184 PS1, Line 184: } > add space after 'for', add a space before { Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@201 PS1, Line 201: > nit: indent in 4 spaces Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@208 PS1, Line 208: > nit: indent in 4 spaces Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@214 PS1, Line 214: } : : > remove Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@257 PS1, Line 257: ", Kud > why up case? I just wanted to highlight, that it is used in predicate. But it is not really necessary, changed all of these to lower case. http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@259 PS1, Line 259: KU > nit: indent in 4 spaces Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@275 PS1, Line 275: // > nit: indent in 4 spaces Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@278 PS1, Line 278: > nit: indent in 4 spaces Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@283 PS1, Line 283: > nit: indent in 4 spaces Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@290 PS1, Line 290: // Update a row. > warning: unused variable 'int_val_EQUALS' [clang-diagnostic-unused-variable Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@299 PS1, Line 299: in > nit: indent in 4 spaces Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@319 PS1, Line 319: D > nit: indent in 4 spaces Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@331 PS1, Line 331: KU > nit: indent in 4 spaces Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@334 PS1, Line 334: KU > nit: indent in 4 spaces Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@339 PS1, Line 339: in > nit: indent in 4 spaces Done http://gerrit.cloudera.org:8080/#/c/19690/1/examples/cpp/non_unique_primary_key.cc@371 PS1, Line 371: DU > nit: indent in 4 spaces Done -- To view, visit http://gerrit.cloudera.org:8080/19690 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I8c6be6bece56788dc858468d1fcccff6955836ec Gerrit-Change-Number: 19690 Gerrit-PatchSet: 2 Gerrit-Owner: Marton Greber <[email protected]> Gerrit-Reviewer: Abhishek Chennaka <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Marton Greber <[email protected]> Gerrit-Reviewer: Tidy Bot (241) Gerrit-Reviewer: Wenzhe Zhou <[email protected]> Gerrit-Comment-Date: Wed, 05 Apr 2023 10:48:34 +0000 Gerrit-HasComments: Yes
