[kudu-CR] [Java] Add setRow to Operation
Grant Henke has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12409 ) Change subject: [Java] Add setRow to Operation .. [Java] Add setRow to Operation Adds a setRow method to Operation to allow for existing rows to be added to an operation vs being manually built. Change-Id: I7adee20166e90249209e80700db315172669edb5 Reviewed-on: http://gerrit.cloudera.org:8080/12409 Reviewed-by: Adar Dembo Tested-by: Kudu Jenkins --- M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java 1 file changed, 24 insertions(+), 1 deletion(-) Approvals: Adar Dembo: Looks good to me, approved Kudu Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/12409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I7adee20166e90249209e80700db315172669edb5 Gerrit-Change-Number: 12409 Gerrit-PatchSet: 3 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] [Java] Add setRow to Operation
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12409 ) Change subject: [Java] Add setRow to Operation .. Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java: http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java@201 PS1, Line 201: Preconditions.checkArgument(row.getSchema().equals(table.getSchema()), : "The row's schema must be equal to the table schema"); > Ah, I assumed Schema had an overridden equals() that behaved like its C++ e I would like for it to have an overridden equals, but some if it's internals make that difficult. There is some history on that somewhere. -- To view, visit http://gerrit.cloudera.org:8080/12409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7adee20166e90249209e80700db315172669edb5 Gerrit-Change-Number: 12409 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 17:36:25 + Gerrit-HasComments: Yes
[kudu-CR] [Java] Add setRow to Operation
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12409 ) Change subject: [Java] Add setRow to Operation .. Patch Set 2: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java: http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java@201 PS1, Line 201: Preconditions.checkArgument(row.getSchema() == table.getSchema(), : "The row's schema must be equal by reference to the ta > There is a cost, but it's pretty low given they need to be equal by referen Ah, I assumed Schema had an overridden equals() that behaved like its C++ equivalent. -- To view, visit http://gerrit.cloudera.org:8080/12409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7adee20166e90249209e80700db315172669edb5 Gerrit-Change-Number: 12409 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 17:35:29 + Gerrit-HasComments: Yes
[kudu-CR] [Java] Add setRow to Operation
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12409 ) Change subject: [Java] Add setRow to Operation .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/12409/1//COMMIT_MSG Commit Message: PS1: > Not really understanding the motivation. You provided some sample code near This is useful for integration use cases where you translate from some third-party row class to Kudu's partial row and then convert it to an operation to send to kudu. Today you need to know the operation before you convert which is more tightly coupled than needed, or you need to copy the partial row over to the ops partial row which is expensive. The next 2 patches in this series are a good example of usage. Additionally I have a repartitioning patch coming up that will use this. http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java: http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java@188 PS1, Line 188: tables > table's Done http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java@201 PS1, Line 201: Preconditions.checkArgument(row.getSchema().equals(table.getSchema()), : "The row's schema must be equal to the table schema"); > Won't this be expensive when working with many operations? There is a cost, but it's pretty low given they need to be equal by reference, not by value. I will change to using == incase the equals implementation ever changes. -- To view, visit http://gerrit.cloudera.org:8080/12409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7adee20166e90249209e80700db315172669edb5 Gerrit-Change-Number: 12409 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 17:20:07 + Gerrit-HasComments: Yes
[kudu-CR] [Java] Add setRow to Operation
Hello Will Berkeley, Mike Percy, Kudu Jenkins, Adar Dembo, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/12409 to look at the new patch set (#2). Change subject: [Java] Add setRow to Operation .. [Java] Add setRow to Operation Adds a setRow method to Operation to allow for existing rows to be added to an operation vs being manually built. Change-Id: I7adee20166e90249209e80700db315172669edb5 --- M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java 1 file changed, 24 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/12409/2 -- To view, visit http://gerrit.cloudera.org:8080/12409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I7adee20166e90249209e80700db315172669edb5 Gerrit-Change-Number: 12409 Gerrit-PatchSet: 2 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] [Java] Add setRow to Operation
Adar Dembo has posted comments on this change. ( http://gerrit.cloudera.org:8080/12409 ) Change subject: [Java] Add setRow to Operation .. Patch Set 1: (3 comments) http://gerrit.cloudera.org:8080/#/c/12409/1//COMMIT_MSG Commit Message: PS1: Not really understanding the motivation. You provided some sample code near setRow(), but can you put it in context? When is this useful? http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java File java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java: http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java@188 PS1, Line 188: tables table's http://gerrit.cloudera.org:8080/#/c/12409/1/java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java@201 PS1, Line 201: Preconditions.checkArgument(row.getSchema().equals(table.getSchema()), : "The row's schema must be equal to the table schema"); Won't this be expensive when working with many operations? -- To view, visit http://gerrit.cloudera.org:8080/12409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7adee20166e90249209e80700db315172669edb5 Gerrit-Change-Number: 12409 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Adar Dembo Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Wed, 13 Feb 2019 00:25:06 + Gerrit-HasComments: Yes
[kudu-CR] [Java] Add setRow to Operation
Grant Henke has posted comments on this change. ( http://gerrit.cloudera.org:8080/12409 ) Change subject: [Java] Add setRow to Operation .. Patch Set 1: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I7adee20166e90249209e80700db315172669edb5 Gerrit-Change-Number: 12409 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley Gerrit-Comment-Date: Sat, 09 Feb 2019 02:35:07 + Gerrit-HasComments: No
[kudu-CR] [Java] Add setRow to Operation
Grant Henke has removed a vote on this change. Change subject: [Java] Add setRow to Operation .. Removed Verified-1 by Kudu Jenkins (120) -- To view, visit http://gerrit.cloudera.org:8080/12409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: deleteVote Gerrit-Change-Id: I7adee20166e90249209e80700db315172669edb5 Gerrit-Change-Number: 12409 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Mike Percy Gerrit-Reviewer: Will Berkeley
[kudu-CR] [Java] Add setRow to Operation
Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12409 Change subject: [Java] Add setRow to Operation .. [Java] Add setRow to Operation Adds a setRow method to Operation to allow for existing rows to be added to an operation vs being manually built. Change-Id: I7adee20166e90249209e80700db315172669edb5 --- M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java 1 file changed, 24 insertions(+), 1 deletion(-) git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/09/12409/1 -- To view, visit http://gerrit.cloudera.org:8080/12409 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I7adee20166e90249209e80700db315172669edb5 Gerrit-Change-Number: 12409 Gerrit-PatchSet: 1 Gerrit-Owner: Grant Henke
