Adar Dembo has posted comments on this change.

Change subject: KUDU-1002. Add support for UPSERT
......................................................................


Patch Set 5:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/3101/5/src/kudu/client/client.h
File src/kudu/client/client.h:

Line 418: // Create a new write operation for this table. It is the caller's
        :   // responsibility to free it, unless it is passed to 
KuduSession::Apply().
We should probably document the semantics of each write operation.

INSERT: Adds a new row. Fails if the row already exists.
UPSERT: Adds a new row. If there's an existing row, updates it.
UPDATE: Update an existing row. Fails if the row does not exist.
DELETE: Deletes an existing row. Fails if the row does not exist.

Could also do it in an ASCII table where each row is a different write 
operation and the two columns are "effect if row does not exist" and "effect if 
row already exists".


http://gerrit.cloudera.org:8080/#/c/3101/5/src/kudu/common/wire_protocol.proto
File src/kudu/common/wire_protocol.proto:

Line 141: // A set of operations (INSERT, UPDATE, or DELETE) to apply to a 
table.
Maybe remove the list here so we don't feel compelled to keep it up-to-date?


http://gerrit.cloudera.org:8080/#/c/3101/5/src/kudu/tablet/tablet-test.cc
File src/kudu/tablet/tablet-test.cc:

Line 585:   // UPSERT a row that is in DRS..
Nit: extra period here.


http://gerrit.cloudera.org:8080/#/c/3101/5/src/kudu/tablet/tablet.cc
File src/kudu/tablet/tablet.cc:

Line 439: Status Tablet::ApplyUpsertAsUpdate(WriteTransactionState *tx_state,
Nit: WriteTransactionState*


http://gerrit.cloudera.org:8080/#/c/3101/5/src/kudu/tablet/tablet_random_access-test.cc
File src/kudu/tablet/tablet_random_access-test.cc:

Line 105:           // between update and UPSERT.
Nit: upsert (to be consistent with the rest).


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Ic83d40bb7d577509d64aa7986f9dcd0280400c09
Gerrit-PatchSet: 5
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Todd Lipcon <[email protected]>
Gerrit-Reviewer: Adar Dembo <[email protected]>
Gerrit-Reviewer: Dan Burkert <[email protected]>
Gerrit-Reviewer: David Ribeiro Alves <[email protected]>
Gerrit-Reviewer: Jean-Daniel Cryans
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <[email protected]>
Gerrit-HasComments: Yes

Reply via email to