Yuqi Du has posted comments on this change. ( http://gerrit.cloudera.org:8080/20061 )
Change subject: [server] KUDU-1945 Auto_incrementing column Upsert support ...................................................................... Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/20061/1/src/kudu/common/row_operations.cc File src/kudu/common/row_operations.cc: http://gerrit.cloudera.org:8080/#/c/20061/1/src/kudu/common/row_operations.cc@478 PS1, Line 478: op->type == RowOperationsPB_Type_UPSERT || : op->type == RowOperationsPB_Type_UPSERT_IGNORE RowOperationsPB_Type_UPSERT means RowOperationsPB_Type_INSERT or RowOperationsPB_Type_UPDATE, according to context, I guess it is for RowOperationsPB_Type_UPDATE. So should we add RowOperationsPB_Type_UPDATE at this? http://gerrit.cloudera.org:8080/#/c/20061/1/src/kudu/common/row_operations.cc@491 PS1, Line 491: "column value"); nit: alignment http://gerrit.cloudera.org:8080/#/c/20061/1/src/kudu/common/row_operations.cc@495 PS1, Line 495: // Check if the provided counter value is less than what is in memory : // and update the counter in memory. This auto-incrementing counter is set by the user at client? If a user set a smaller value, server side will ignore this set and change it to a bigger one using an obscure way for users. In kudu, this is correct, but for users it seems not good enough. Whether it is necessary to provide more response messages or return not ok? -- To view, visit http://gerrit.cloudera.org:8080/20061 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib5cc4d80f77c165452572948f68c76fc70394d47 Gerrit-Change-Number: 20061 Gerrit-PatchSet: 1 Gerrit-Owner: 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: Yuqi Du <[email protected]> Gerrit-Comment-Date: Wed, 14 Jun 2023 08:07:02 +0000 Gerrit-HasComments: Yes
