You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Yingchun Lai (Code Review)" <ge...@cloudera.org> on 2022/09/16 08:36:44 UTC
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Yingchun Lai has uploaded this change for review. ( http://gerrit.cloudera.org:8080/18993
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
---
M java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
M java/kudu-client/src/main/java/org/apache/kudu/Schema.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
M java/kudu-client/src/main/java/org/apache/kudu/client/UpdateIgnore.java
A java/kudu-client/src/main/java/org/apache/kudu/client/UpsertIgnore.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/schema.cc
M src/kudu/common/common.proto
M src/kudu/common/schema.cc
M src/kudu/common/schema.h
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
26 files changed, 600 insertions(+), 46 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/18993/1
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 1
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18993
to look at the new patch set (#7).
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
This is a follow-up to b6eedb224f715ad86378a92d25f09c2084b0e2b7.
This patch includes the Java client-side changes
of the "new column attribute IMMUTABLE" feature,
including:
1. Adds a new 'immutable(boolean immutable)' method to
class ColumnSchemaBuilder to add/remove IMMUTABLE
attribute to/from a column.
2. Adds a new 'isImmutable()' method to class
ColumnSchema to check if the attribute is set for
a column schema.
3. Adds a new 'hasImmutableColumns()' method to class
Schema to check if there's at least one immutable
column for a table schema.
4. Adds a new 'changeImmutable(String name, boolean immutable)'
method to class AlterTableOptions to change the
immutable attribute for a column.
5. Adds a new UpsertIgnore operation in the client API:
use the newly added KuduTable.newUpsertIgnore() to
create a new instance of such operation.
Both UpsertIgnore and UpdateIgnore operations can be use
to ignore updating on immutable columns errors.
6. Adds unit tests to cover the newly introduced functionality.
Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
---
M java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
M java/kudu-client/src/main/java/org/apache/kudu/Schema.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
M java/kudu-client/src/main/java/org/apache/kudu/client/UpdateIgnore.java
A java/kudu-client/src/main/java/org/apache/kudu/client/UpsertIgnore.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
18 files changed, 535 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/18993/7
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 7
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18993 )
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18993/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18993/6//COMMIT_MSG@19
PS6, Line 19: hasImmutableColumn
> nit: Should it be 'hasImmutableColumns'?
Done
http://gerrit.cloudera.org:8080/#/c/18993/6/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java:
http://gerrit.cloudera.org:8080/#/c/18993/6/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java@488
PS6, Line 488: changeImmutable
> nit: To be consistent with c++ API, maybe we can replace the method with 's
There are some difference in the implemention and usage between Java and C++. In C++, adding column and altering column share the same KuduColumnSpec, it's actually convenient to create and alter a table like:
builer.AddColumn("imm_val")->Immutable()->Type(...)->...;
table_alterer->AlterColumn("int_val")->Immutable();
but in Java, they are separeted, using ColumnSchemaBuilder and AlterTableOptions. Here, using 'changeImmutable' is also to keep the same style with other functions in AlterTableOptions, like 'changeComment', 'changeEncoding', and etc.
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 7
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Sun, 09 Oct 2022 02:58:26 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18993
to look at the new patch set (#8).
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
This is a follow-up to b6eedb224f715ad86378a92d25f09c2084b0e2b7.
This patch includes the Java client-side changes
of the "new column attribute IMMUTABLE" feature,
including:
1. Adds a new 'immutable(boolean immutable)' method to
class ColumnSchemaBuilder to add/remove IMMUTABLE
attribute to/from a column.
2. Adds a new 'isImmutable()' method to class
ColumnSchema to check if the attribute is set for
a column schema.
3. Adds a new 'hasImmutableColumns()' method to class
Schema to check if there's at least one immutable
column for a table schema.
4. Adds a new 'changeImmutable(String name, boolean immutable)'
method to class AlterTableOptions to change the
immutable attribute for a column.
5. Adds a new UpsertIgnore operation in the client API:
use the newly added KuduTable.newUpsertIgnore() to
create a new instance of such operation.
Both UpsertIgnore and UpdateIgnore operations can be use
to ignore updating on immutable columns errors.
6. Adds unit tests to cover the newly introduced functionality.
Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
---
M java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
M java/kudu-client/src/main/java/org/apache/kudu/Schema.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
M java/kudu-client/src/main/java/org/apache/kudu/client/UpdateIgnore.java
A java/kudu-client/src/main/java/org/apache/kudu/client/UpsertIgnore.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
17 files changed, 511 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/18993/8
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 8
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18993
to look at the new patch set (#6).
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
This is a follow-up to b6eedb224f715ad86378a92d25f09c2084b0e2b7.
This patch includes the Java client-side changes
of the "new column attribute IMMUTABLE" feature,
including:
1. Adds a new 'immutable(boolean immutable)' method to
class ColumnSchemaBuilder to add/remove IMMUTABLE
attribute to/from a column.
2. Adds a new 'isImmutable()' method to class
ColumnSchema to check if the attribute is set for
a column schema.
3. Adds a new 'hasNullableColumns()' method to class
Schema to check if there's at least one immutable
column for a table schema.
4. Adds a new 'changeImmutable(String name, boolean immutable)'
method to class AlterTableOptions to change the
immutable attribute for a column.
5. Adds a new UpsertIgnore operation in the client API:
use the newly added KuduTable.newUpsertIgnore() to
create a new instance of such operation.
Both UpsertIgnore and UpdateIgnore operations can be use
to ignore updating on immutable columns errors.
6. Adds unit tests to cover the newly introduced functionality.
Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
---
M java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
M java/kudu-client/src/main/java/org/apache/kudu/Schema.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
M java/kudu-client/src/main/java/org/apache/kudu/client/UpdateIgnore.java
A java/kudu-client/src/main/java/org/apache/kudu/client/UpsertIgnore.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
18 files changed, 535 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/18993/6
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18993 )
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java:
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java@558
PS7, Line 558: (step.getType() == AlterTableRequestPB.StepType.ALTER_COLUMN &&
: step.getAlterColumn().getDelta().hasImmutable())) {
> Also fix a bug that should use 'hasImmutable()' but not 'hasImmutable()'.
should use 'hasImmutable()' but not 'getImmutable()'.
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 9
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 19 Oct 2022 13:41:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18993 )
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
Patch Set 7:
(8 comments)
http://gerrit.cloudera.org:8080/#/c/18993/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18993/6//COMMIT_MSG@10
PS6, Line 10: includes
> nit: contains
Done
http://gerrit.cloudera.org:8080/#/c/18993/6//COMMIT_MSG@28
PS6, Line 28: use
> used
Done
http://gerrit.cloudera.org:8080/#/c/18993/6//COMMIT_MSG@29
PS6, Line 29: to ignore updating on immutable columns errors.
> to ignore errors on updating cells of immutable columns
Done
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java:
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java@558
PS7, Line 558: (step.getType() == AlterTableRequestPB.StepType.ALTER_COLUMN &&
: step.getAlterColumn().getDelta().getImmutable())) {
> Just to double-check that I understand what's going on here.
(a) Yes, it is true when removing the immutable attribute, as shown in L493, no matter 'immutable' is true or false, this condition evaluate to true. I've added a new test to check it in the latest patch set.
(b) It is necessary. Consider a rare case that a Kudu cluster downgrade from a newer version to a lower version which dosen't support this feature.
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@2787
PS7, Line 2787: /**
: * Sends a request to the master to check if the cluster supports UpsertIgnore operations.
: * @return true if the cluster supports upsert ignore operations
: */
: @InterfaceAudience.Private
: public Deferred<Boolean> supportsUpsertIgnoreOperations() {
: PingRequest ping = PingRequest.makeMasterPingRequest(
: this.masterTable, timer, defaultAdminOperationTimeoutMs);
: ping.addRequiredFeature(Master.MasterFeatures.UPSERT_IGNORE_VALUE);
: Deferred<PingResponse> response = sendRpcToTablet(ping);
: return AsyncUtil.addBoth(response, new PingSupportsFeatureCallback());
: }
> The supportsIgnoreOperations() above has been added since it was required f
Thanks for you information.
We've ever discussed about it here(in the file header): https://gerrit.cloudera.org/c/18835/4/src/kudu/client/client-test.cc,
It's true that it's a kind of overkill, if a client could create such a table with immutable columns, of course the client can use
UPSERT_IGNORE ops to operate on the cluster. I've also check what will the client get if server doesn't support UPSERT_IGNORE, it will get Status::Corruption error, see: https://github.com/apache/kudu/blob/master/src/kudu/common/row_operations.cc#L234, I think it's acceptable, I've removed these method in the latest patch set.
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java:
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java@500
PS7, Line 500: /**
: * Sends a request to the master to check if the cluster supports upsert ignore operations.
: * @return true if the cluster supports ignore operations
: */
: @InterfaceAudience.Private
: public boolean supportsUpsertIgnoreOperations() throws KuduException {
: return joinAndHandleException(asyncClient.supportsUpsertIgnoreOperations());
: }
> The supportsIgnoreOperations() above has been added since it was required f
OK, removed.
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java:
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java@194
PS7, Line 194: An upsert ignore will
: * ignore updating on immutable columns errors. This is useful when the upsert is operated
: * on a row with immutable columns.
> nit: how about
Done
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java:
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2540
PS7, Line 2540: add a column with immutable attribute
> to change the immutable attribute on a column
Done
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 7
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 19 Oct 2022 13:32:36 +0000
Gerrit-HasComments: Yes
[kudu-CR] WIP KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18993
to look at the new patch set (#3).
Change subject: WIP KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
WIP KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
---
M java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
M java/kudu-client/src/main/java/org/apache/kudu/Schema.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
M java/kudu-client/src/main/java/org/apache/kudu/client/UpdateIgnore.java
A java/kudu-client/src/main/java/org/apache/kudu/client/UpsertIgnore.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
18 files changed, 509 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/18993/3
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 3
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18993
to look at the new patch set (#5).
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
This is a follow-up to b6eedb224f715ad86378a92d25f09c2084b0e2b7.
This patch includes the Java client-side changes
of the "new column attribute IMMUTABLE" feature,
including:
1. Adds a new 'immutable(boolean immutable)' method to
class ColumnSchemaBuilder to add/remove IMMUTABLE
attribute to/from a column.
2. Adds a new 'isImmutable()' method to class
ColumnSchema to check if the attribute is set for
a column schema.
3. Adds a new 'hasNullableColumns()' method to class
Schema to check if there's at least one immutable
column for a table schema.
4. Adds a new 'changeImmutable(String name, boolean immutable)'
method to class AlterTableOptions to change the
immutable attribute for a column.
5. Adds a new UpsertIgnore operation in the client API:
use the newly added KuduTable.newUpsertIgnore() to
create a new instance of such operation.
Both UpsertIgnore and UpdateIgnore operations can be use
to ignore updating on immutable columns errors.
6. Adds unit tests to cover the newly introduced functionality.
Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
---
M java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
M java/kudu-client/src/main/java/org/apache/kudu/Schema.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
M java/kudu-client/src/main/java/org/apache/kudu/client/UpdateIgnore.java
A java/kudu-client/src/main/java/org/apache/kudu/client/UpsertIgnore.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
18 files changed, 515 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/18993/5
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 5
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 10
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] WIP KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18993
to look at the new patch set (#2).
Change subject: WIP KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
WIP KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
---
M java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
M java/kudu-client/src/main/java/org/apache/kudu/Schema.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
M java/kudu-client/src/main/java/org/apache/kudu/client/UpdateIgnore.java
A java/kudu-client/src/main/java/org/apache/kudu/client/UpsertIgnore.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
M src/kudu/client/client-test.cc
M src/kudu/client/client.cc
M src/kudu/client/schema.cc
M src/kudu/common/common.proto
M src/kudu/common/schema.cc
M src/kudu/common/schema.h
M src/kudu/common/wire_protocol.cc
M src/kudu/integration-tests/alter_table-test.cc
26 files changed, 600 insertions(+), 46 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/18993/2
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 2
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18993 )
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
Patch Set 10:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18993/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java:
http://gerrit.cloudera.org:8080/#/c/18993/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2549
PS9, Line 2549: we can test the function
: // on the client side, the request will be processed by the generic RPC code and throw an
: // exception before reaching particular appli
> nit just for my education: Is that indeed so? I was under impression that
Indeed, you're right, the comment is not correct.
Updated.
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 10
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 20 Oct 2022 00:17:12 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yifan Zhang (Code Review)" <ge...@cloudera.org>.
Yifan Zhang has posted comments on this change. ( http://gerrit.cloudera.org:8080/18993 )
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
Patch Set 6: Code-Review+1
(2 comments)
http://gerrit.cloudera.org:8080/#/c/18993/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18993/6//COMMIT_MSG@19
PS6, Line 19: hasNullableColumns
nit: Should it be 'hasImmutableColumns'?
http://gerrit.cloudera.org:8080/#/c/18993/6/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java:
http://gerrit.cloudera.org:8080/#/c/18993/6/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java@488
PS6, Line 488: changeImmutable
nit: To be consistent with c++ API, maybe we can replace the method with 'setImmutable' and 'setMutable'?
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 6
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 06 Oct 2022 04:41:08 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/18993 )
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
Patch Set 9:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java:
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java@558
PS7, Line 558: (step.getType() == AlterTableRequestPB.StepType.ALTER_COLUMN &&
: step.getAlterColumn().getDelta().hasImmutable())) {
> (a) Yes, it is true when removing the immutable attribute, as shown in L493
Also fix a bug that should use 'hasImmutable()' but not 'hasImmutable()'.
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 9
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 19 Oct 2022 13:40:21 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18993 )
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
Patch Set 9: Code-Review+1
(3 comments)
Looks good to me, just a question to clarify on the statement in one comment.
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java:
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java@558
PS7, Line 558: (step.getType() == AlterTableRequestPB.StepType.ALTER_COLUMN &&
: step.getAlterColumn().getDelta().hasImmutable())) {
> should use 'hasImmutable()' but not 'getImmutable()'.
Ah, great! Thanks for the clarification and the fix.
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@2787
PS7, Line 2787: private static final class PingSupportsFeatureCallback implements Callback<Boolean, Object> {
: @Override
: public Boolean call(final Object resp) {
: if (resp instanceof Exception) {
: // The server returns an RpcRemoteException when the required feature is not supported.
: // The exception should have an ERROR_INVALID_REQUEST error code and at least one
: // unsupported feature flag.
: if (resp instanceof RpcRemoteException &&
: ((RpcRemoteException) resp).getErrPB().getCode() == ERROR_INVALID_REQUEST &&
: ((RpcRemoteException) resp).getErrPB().getUnsupportedFeatureFlagsCount() >= 1) {
: return false;
:
> Thanks for you information.
Yes, that makes sense. Thanks!
http://gerrit.cloudera.org:8080/#/c/18993/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java:
http://gerrit.cloudera.org:8080/#/c/18993/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2549
PS9, Line 2549: we can
: // test the function on the client side, the request will not be sent to server
: // because it's validated in the client side.
nit just for my education: Is that indeed so? I was under impression that the required feature flags from the RPC request are validated at the RPC level of a Kudu server. So, such a request wouldn't reach particular application code at the server side (e.g., AlterTable request requiring immutable columns feature wouldn't hit the code in CatalogManager::AlterTable()), but it will be processed by the generic RPC code running as a part of the kudu-master process.
Am I missing something here?
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 9
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 19 Oct 2022 17:57:13 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18993
to look at the new patch set (#9).
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
This is a follow-up to b6eedb224f715ad86378a92d25f09c2084b0e2b7.
This patch contains the Java client-side changes
of the "new column attribute IMMUTABLE" feature,
including:
1. Adds a new 'immutable(boolean immutable)' method to
class ColumnSchemaBuilder to add/remove IMMUTABLE
attribute to/from a column.
2. Adds a new 'isImmutable()' method to class
ColumnSchema to check if the attribute is set for
a column schema.
3. Adds a new 'hasImmutableColumns()' method to class
Schema to check if there's at least one immutable
column for a table schema.
4. Adds a new 'changeImmutable(String name, boolean immutable)'
method to class AlterTableOptions to change the
immutable attribute for a column.
5. Adds a new UpsertIgnore operation in the client API:
use the newly added KuduTable.newUpsertIgnore() to
create a new instance of such operation.
Both UpsertIgnore and UpdateIgnore operations can be used
to ignore errors on updating cells of immutable columns.
6. Adds unit tests to cover the newly introduced functionality.
Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
---
M java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
M java/kudu-client/src/main/java/org/apache/kudu/Schema.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
M java/kudu-client/src/main/java/org/apache/kudu/client/UpdateIgnore.java
A java/kudu-client/src/main/java/org/apache/kudu/client/UpsertIgnore.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
17 files changed, 511 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/18993/9
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 9
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18993 )
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
This is a follow-up to b6eedb224f715ad86378a92d25f09c2084b0e2b7.
This patch contains the Java client-side changes
of the "new column attribute IMMUTABLE" feature,
including:
1. Adds a new 'immutable(boolean immutable)' method to
class ColumnSchemaBuilder to add/remove IMMUTABLE
attribute to/from a column.
2. Adds a new 'isImmutable()' method to class
ColumnSchema to check if the attribute is set for
a column schema.
3. Adds a new 'hasImmutableColumns()' method to class
Schema to check if there's at least one immutable
column for a table schema.
4. Adds a new 'changeImmutable(String name, boolean immutable)'
method to class AlterTableOptions to change the
immutable attribute for a column.
5. Adds a new UpsertIgnore operation in the client API:
use the newly added KuduTable.newUpsertIgnore() to
create a new instance of such operation.
Both UpsertIgnore and UpdateIgnore operations can be used
to ignore errors on updating cells of immutable columns.
6. Adds unit tests to cover the newly introduced functionality.
Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Reviewed-on: http://gerrit.cloudera.org:8080/18993
Tested-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
M java/kudu-client/src/main/java/org/apache/kudu/Schema.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
M java/kudu-client/src/main/java/org/apache/kudu/client/UpdateIgnore.java
A java/kudu-client/src/main/java/org/apache/kudu/client/UpsertIgnore.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
17 files changed, 511 insertions(+), 39 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 11
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18993
to look at the new patch set (#4).
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
This is a follow-up to b6eedb224f715ad86378a92d25f09c2084b0e2b7.
This patch includes the Java client-side changes
of the "new column attribute IMMUTABLE" feature,
including:
1. Adds a new 'immutable(boolean immutable)' method to
class ColumnSchemaBuilder to add/remove IMMUTABLE
attribute to/from a column.
2. Adds a new 'isImmutable()' method to class
ColumnSchema to check if the attribute is set for
a column schema.
3. Adds a new 'hasNullableColumns()' method to class
Schema to check if there's at least one immutable
column for a table schema.
4. Adds a new 'changeImmutable(String name, boolean immutable)'
method to class AlterTableOptions to change the
immutable attribute for a column.
5. Adds a new UpsertIgnore operation in the client API:
use the newly added KuduTable.newUpsertIgnore() to
create a new instance of such operation.
Both UpsertIgnore and UpdateIgnore operations can be use
to ignore updating on immutable columns errors.
6. Adds unit tests to cover the newly introduced functionality.
Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
---
M java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
M java/kudu-client/src/main/java/org/apache/kudu/Schema.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
M java/kudu-client/src/main/java/org/apache/kudu/client/UpdateIgnore.java
A java/kudu-client/src/main/java/org/apache/kudu/client/UpsertIgnore.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
18 files changed, 509 insertions(+), 38 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/18993/4
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 4
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18993 )
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
Patch Set 10: Verified+1
unrelated failure (RELEASE):
Could not submit C++ distributed test job
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 10
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 20 Oct 2022 01:41:01 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Yifan Zhang, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/18993
to look at the new patch set (#10).
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
This is a follow-up to b6eedb224f715ad86378a92d25f09c2084b0e2b7.
This patch contains the Java client-side changes
of the "new column attribute IMMUTABLE" feature,
including:
1. Adds a new 'immutable(boolean immutable)' method to
class ColumnSchemaBuilder to add/remove IMMUTABLE
attribute to/from a column.
2. Adds a new 'isImmutable()' method to class
ColumnSchema to check if the attribute is set for
a column schema.
3. Adds a new 'hasImmutableColumns()' method to class
Schema to check if there's at least one immutable
column for a table schema.
4. Adds a new 'changeImmutable(String name, boolean immutable)'
method to class AlterTableOptions to change the
immutable attribute for a column.
5. Adds a new UpsertIgnore operation in the client API:
use the newly added KuduTable.newUpsertIgnore() to
create a new instance of such operation.
Both UpsertIgnore and UpdateIgnore operations can be used
to ignore errors on updating cells of immutable columns.
6. Adds unit tests to cover the newly introduced functionality.
Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
---
M java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
M java/kudu-client/src/main/java/org/apache/kudu/Schema.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableOptions.java
M java/kudu-client/src/main/java/org/apache/kudu/client/CreateTableRequest.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Operation.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/main/java/org/apache/kudu/client/Status.java
M java/kudu-client/src/main/java/org/apache/kudu/client/UpdateIgnore.java
A java/kudu-client/src/main/java/org/apache/kudu/client/UpsertIgnore.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduSession.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
17 files changed, 511 insertions(+), 39 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/93/18993/10
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 10
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18993 )
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
Patch Set 7:
(8 comments)
I apologize for the late response.
Overall looks good, just a few questions about methods that look like an extra and to double-check my understanding.
http://gerrit.cloudera.org:8080/#/c/18993/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/18993/6//COMMIT_MSG@10
PS6, Line 10: includes
nit: contains
This is to avoid the '... includes ..., including ...' pattern
http://gerrit.cloudera.org:8080/#/c/18993/6//COMMIT_MSG@28
PS6, Line 28: use
used
http://gerrit.cloudera.org:8080/#/c/18993/6//COMMIT_MSG@29
PS6, Line 29: to ignore updating on immutable columns errors.
to ignore errors on updating cells of immutable columns
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java:
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java@558
PS7, Line 558: (step.getType() == AlterTableRequestPB.StepType.ALTER_COLUMN &&
: step.getAlterColumn().getDelta().getImmutable())) {
Just to double-check that I understand what's going on here.
What if trying to alter a column, removing the immutable attribute from it? In that case:
(a) Does this condition evaluate to true?
(b) Is it necessary to request the support for the newly added feature in that case?
Thanks!
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@2787
PS7, Line 2787: /**
: * Sends a request to the master to check if the cluster supports UpsertIgnore operations.
: * @return true if the cluster supports upsert ignore operations
: */
: @InterfaceAudience.Private
: public Deferred<Boolean> supportsUpsertIgnoreOperations() {
: PingRequest ping = PingRequest.makeMasterPingRequest(
: this.masterTable, timer, defaultAdminOperationTimeoutMs);
: ping.addRequiredFeature(Master.MasterFeatures.UPSERT_IGNORE_VALUE);
: Deferred<PingResponse> response = sendRpcToTablet(ping);
: return AsyncUtil.addBoth(response, new PingSupportsFeatureCallback());
: }
The supportsIgnoreOperations() above has been added since it was required for Kudu backup&restore.
What's the anticipated use case for these? If that's just some theoretical one, I'd suggest not adding this method yet according to the YAGNI principle: https://www.martinfowler.com/bliki/Yagni.html
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java:
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/KuduClient.java@500
PS7, Line 500: /**
: * Sends a request to the master to check if the cluster supports upsert ignore operations.
: * @return true if the cluster supports ignore operations
: */
: @InterfaceAudience.Private
: public boolean supportsUpsertIgnoreOperations() throws KuduException {
: return joinAndHandleException(asyncClient.supportsUpsertIgnoreOperations());
: }
The supportsIgnoreOperations() above has been added since it was required for Kudu backup&restore.
What's the anticipated use case for these? If they are not used anywhere (but the tests added), maybe we don't really need to have this method, even if it's a part of the priva API? If you have some particular use-case to be added with next patch or so, maybe add these methods along with that new functionality?
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java
File java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java:
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/main/java/org/apache/kudu/client/KuduTable.java@194
PS7, Line 194: An upsert ignore will
: * ignore updating on immutable columns errors. This is useful when the upsert is operated
: * on a row with immutable columns.
nit: how about
The operation ignores errors of updating immutable cells in a row. This is useful when upserting rows in a table with immutable columns.
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java:
http://gerrit.cloudera.org:8080/#/c/18993/7/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2540
PS7, Line 2540: add a column with immutable attribute
to change the immutable attribute on a column
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 7
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 18 Oct 2022 23:51:04 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/18993 )
Change subject: KUDU-3353 [schema] Add an immutable attribute on column schema (part 3)
......................................................................
Patch Set 10: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/18993/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java:
http://gerrit.cloudera.org:8080/#/c/18993/9/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2549
PS9, Line 2549: we can test the function
: // on the client side, the request will be processed by the generic RPC code and throw an
: // exception before reaching particular appli
> Indeed, you're right, the comment is not correct.
Thank you for the confirmation.
--
To view, visit http://gerrit.cloudera.org:8080/18993
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifdfdcd123296803a3b5e856ec5eaac49c05b7f8d
Gerrit-Change-Number: 18993
Gerrit-PatchSet: 10
Gerrit-Owner: Yingchun Lai <ac...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yifan Zhang <ch...@163.com>
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 20 Oct 2022 01:41:36 +0000
Gerrit-HasComments: Yes