You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org> on 2022/12/20 23:38:21 UTC
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Wenzhe Zhou has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19384
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the last key column.
Testing:
- Manually ran integration test with Impala for creating table
with non unique primary key, ran tests for operations:
insert/update/delete for non unique primary key.
- TODO add unit-test
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/Type.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/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
6 files changed, 176 insertions(+), 17 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/1
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 1
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#17). ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the key column. The non-unique
key columns and the auto-incrementing column together form the
effective primary key.
UPSERT/UPSERT_IGNORE operations are not supported now for Kudu table
with auto-incrementing column due to limitation in Kudu server.
Auto-Incrementing column cannot be added, removed or renamed with
Alter Table APIs.
Testing:
- Added unit-test for Java client library.
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
describe/insert/update/delete/upsert/CTAS/select/alter, etc.
Passed Kudu related end-end tests.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduScanner.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/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
11 files changed, 580 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/17
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 17
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kurt Deschler, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19384
to look at the new patch set (#21).
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_incrementing_id" will
be added automatically to the table as the key column. The non-unique
key columns and the auto-incrementing column together form the
effective primary key.
UPSERT/UPSERT_IGNORE operations are not supported now for Kudu table
with auto-incrementing column due to limitation in Kudu server.
Auto-Incrementing column cannot be added, removed or renamed with
Alter Table APIs.
Testing:
- Added unit-test for Java client library.
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
describe/insert/update/delete/upsert/CTAS/select/alter, etc.
Passed Kudu related end-to-end tests.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduScanner.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/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
10 files changed, 676 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/21
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 21
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 5:
This patch is dependent on the additional fields added to src/kudu/common/common.proto by https://gerrit.cloudera.org/#/c/19097/.
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 22 Dec 2022 19:45:06 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 18:
(16 comments)
http://gerrit.cloudera.org:8080/#/c/19384/18//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19384/18//COMMIT_MSG@11
PS18, Line 11: auto_increment_id
auto_incrementing_id
http://gerrit.cloudera.org:8080/#/c/19384/18//COMMIT_MSG@27
PS18, Line 27: end-end
nit: end-to-end ?
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@159
PS18, Line 159: Type type = Type.getTypeForDataType(pb.getType());
: ColumnTypeAttributes typeAttributes = pb.hasTypeAttributes() ?
: pbToColumnTypeAttributes(pb.getTypeAttributes()) : null;
: Object defaultValue = pb.hasWriteDefaultValue() ?
: byteStringToObject(type, typeAttributes, pb.getWriteDefaultValue()) : null;
nit: could move these lines down to be after line 175?
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@167
PS18, Line 167: int desiredBlockSize = pb.getCfileBlockSize();
nit: this could be moved down to be after line 175 as well?
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@170
PS18, Line 170: default encoding
Why default? It seems they are coming from the 'pb' parameter.
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@171
PS18, Line 171: return new ColumnSchema.AutoIncrementingColumnSchemaBuilder()
: .encoding(encoding)
: .compressionAlgorithm(compressionAlgorithm)
: .build();
After looking at this one more time I realized I'm confused :)
It seems in this case we are discarding whatever is set in the other fields of the 'pb' parameter. Why is so? I'd think this helper method takes all the fields in the 'pb' as the input, treating those as the source of truth, but only one extra artificial field is set: nonUniqueKey().
It seems now the semantics of this helper method has changed. It would be great to clarify why we need to change it up to such extent.
Thanks!
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@179
PS18, Line 179: !isKeyUnique
nit: remove the negation and swap lines 180 & 182?
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@792
PS18, Line 792: schema
nit: a schema
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@798
PS18, Line 798: assertEquals(1, schema.getPrimaryKeyColumnCount());
This now looks a bit different from we have in the C++ client: https://gerrit.cloudera.org/#/c/19272/19/src/kudu/client/client-unittest.cc@233
Should we reconcile?
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@799
PS18, Line 799: table
nit: a table
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@810
PS18, Line 810: assertEquals(3, schema.getColumnCount());
: assertEquals(2, schema.getPrimaryKeyColumnCount());
Isn't this some sort of a contradiction to the POLA principle (https://en.wikipedia.org/wiki/Principle_of_least_astonishment)? At lines 797 & 798 the schema shows different numbers. I'd expect those to be the same.
http://gerrit.cloudera.org:8080/#/c/19384/18/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/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2570
PS18, Line 2570: schema
nit for here and elsewhere in the code below: a schema
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2579
PS18, Line 2579: assertFalse(schema.isPrimaryKeyUnique());
Could you also add an assert to verify the number of columns in the resulting table's schema?
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2580
PS18, Line 2580: 2, schema.getPrimaryKeyColumnCount()
Now this behavior looks a bit different from what we have in the C++ client: https://gerrit.cloudera.org/#/c/19272/19/src/kudu/client/client-unittest.cc@233
Should we reconcile?
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2610
PS18, Line 2610: assertTrue(schema.hasAutoIncrementingColumn());
Could you also add an assertion on the number of columns in the resulting table's schema?
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2630
PS18, Line 2630: }
Could you also add a test where a new table with a single column is created, but the only column is the auto-incremented column (i.e. produced with ColumnSchema.AutoIncrementingColumnSchemaBuilder())? A couple of sub-scenarios would be:
* the only column is set to be the unique primary key
* the only column is set to be the non-unique primary key
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 18 Jan 2023 04:39:31 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#16). ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the key column. The non-unique
key columns and the auto-incrementing column together form the
effective primary key.
UPSERT/UPSERT_IGNORE operations are not supported now for Kudu table
with auto-incrementing column due to limitation in Kudu server.
Auto-Incrementing column cannot be added, removed or renamed with
Alter Table APIs.
Testing:
- Added unit-test for Java client library.
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
describe/insert/update/delete/upsert/CTAS/select/alter, etc.
Passed Kudu related end-end tests.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduScanner.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/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
11 files changed, 562 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/16
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 16
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 20:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@810
PS18, Line 810: assertTrue(schema.hasAutoIncrementingColumn());
: assertEquals(3, schema.getColumnCount());
> At lines 797 & 798, the schema does not consist auto-incrementing column be
Now the column counts are same
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 20
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 19 Jan 2023 05:15:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_incrementing_id" will
be added automatically to the table as the key column. The non-unique
key columns and the auto-incrementing column together form the
effective primary key.
UPSERT/UPSERT_IGNORE operations are not supported now for Kudu table
with auto-incrementing column due to limitation in Kudu server.
Auto-Incrementing column cannot be added, removed or renamed with
Alter Table APIs.
Testing:
- Added unit-test for Java client library.
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
describe/insert/update/delete/upsert/CTAS/select/alter, etc.
Passed Kudu related end-to-end tests.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Reviewed-on: http://gerrit.cloudera.org:8080/19384
Reviewed-by: Alexey Serbin <al...@apache.org>
Reviewed-by: Abhishek Chennaka <ac...@cloudera.com>
Tested-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/AsyncKuduScanner.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/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
10 files changed, 676 insertions(+), 19 deletions(-)
Approvals:
Alexey Serbin: Looks good to me, approved; Verified
Abhishek Chennaka: Looks good to me, but someone else must approve
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 22
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 21: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 21
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 20 Jan 2023 01:54:27 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#14). ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the last key column.
Testing:
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
insert/update/delete/CTAS.
- Added unit-test for Java client.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
9 files changed, 390 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/14
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 14
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 11:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19384/11/java/kudu-client/src/main/java/org/apache/kudu/Schema.java
File java/kudu-client/src/main/java/org/apache/kudu/Schema.java:
http://gerrit.cloudera.org:8080/#/c/19384/11/java/kudu-client/src/main/java/org/apache/kudu/Schema.java@46
PS11, Line 46: increment
Can we replace increment by incrementing everywhere to maintain consistency?
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 11
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Jan 2023 16:54:08 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 12:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19384/11/java/kudu-client/src/main/java/org/apache/kudu/Schema.java
File java/kudu-client/src/main/java/org/apache/kudu/Schema.java:
http://gerrit.cloudera.org:8080/#/c/19384/11/java/kudu-client/src/main/java/org/apache/kudu/Schema.java@46
PS11, Line 46: to_increm
> Can we replace increment by incrementing everywhere to maintain consistency
Done
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 12
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Jan 2023 17:22:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kurt Deschler, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19384
to look at the new patch set (#5).
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the last key column.
Testing:
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
insert/update/delete/CTAS.
- Adds unit-test for Java client.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
9 files changed, 319 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/5
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 5
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#15). ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the key column. The non-unique
key columns and the auto-incrementing column together form the
effective primary key.
UPSERT/UPSERT_IGNORE operations are not supported now for Kudu table
with auto-incrementing column due to limitation in Kudu server.
Auto-Incrementing column cannot be added, removed or renamed with
Alter Table APIs.
Testing:
- Added unit-test for Java client library.
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
describe/insert/update/delete/upsert/CTAS/select/alter, etc.
Passed Kudu related end-end tests.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduScanner.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/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
11 files changed, 562 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/15
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 15
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 21: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 21
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 19 Jan 2023 21:33:45 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kurt Deschler, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19384
to look at the new patch set (#11).
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the last key column.
Testing:
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
insert/update/delete/CTAS.
- Adds unit-test for Java client.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
9 files changed, 451 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/11
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 11
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 18: Code-Review+1
Thanks for adding the alter table side of work as well.
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Jan 2023 19:03:15 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kurt Deschler, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19384
to look at the new patch set (#8).
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the last key column.
Testing:
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
insert/update/delete/CTAS.
- Adds unit-test for Java client.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
9 files changed, 426 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/8
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 8
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kurt Deschler, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19384
to look at the new patch set (#7).
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the last key column.
Testing:
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
insert/update/delete/CTAS.
- Adds unit-test for Java client.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
9 files changed, 422 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/7
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 7
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kurt Deschler, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19384
to look at the new patch set (#12).
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the last key column.
Testing:
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
insert/update/delete/CTAS.
- Adds unit-test for Java client.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
9 files changed, 451 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/12
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 12
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 6:
(20 comments)
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
File java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java:
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java@181
PS6, Line 181: auto incrementing
nit for here and elsewhere: is it possible to get into some consistent naming of the column in the comments? I saw at least three variants in this patch:
* auto incrementing
* auto_incrementing
* auto-incrementing
Probably, we should use 'auto-incrementing' since that's what is used in https://gerrit.cloudera.org/#/c/19097 ?
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java@360
PS6, Line 360: is part
nit: is a part
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java@400
PS6, Line 400: auto incrementing column
nit: an auto incrementing column
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/Schema.java
File java/kudu-client/src/main/java/org/apache/kudu/Schema.java:
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/Schema.java@44
PS6, Line 44: primary
primary key
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/Schema.java@343
PS6, Line 343: Get column name of auto incrementing column
nit: Get the name of the auto-incrementing column
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/Schema.java@351
PS6, Line 351: Get type of auto incrementing column
nit: Get the type of the auto-incrementing column
http://gerrit.cloudera.org:8080/#/c/19384/6/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/19384/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@655
PS6, Line 655: // Create a copy of mutable list and insert auto incrementing column as last key
: // columns
: List<ColumnSchema> colSchemas = new ArrayList<>(schema.getColumns());
: Preconditions.checkNotNull(colSchemas);
I thought auto-incrementing column comes after columns marked as key columns, not all the columns, no?
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@373
PS6, Line 373: auto_incrementing
nit: auto-incrementing
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@157
PS6, Line 157: public static ColumnSchema pbToColumnSchema(Common.ColumnSchemaPB pb,
: boolean hasAutoIncrementing)
Why to introduce this 'hasAutoIncementing' parameter when Common.ColumnSchemaPB has the 'is_auto_incrementing' field? https://gerrit.cloudera.org/#/c/19097/12/src/kudu/common/common.proto@142
If this parameter is taken into the consideration when translating Common.ColumnSchemaPB into run-time structures, what's the semantics of the Common.ColumnSchemaPB.is_auto_incrementing field in 'pb' passed as the first parameter then? I'm confused.
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
File java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java:
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java@79
PS6, Line 79: assertNotEquals(stringCol1, isNonUniqueKey);
In this comparison, 'test' default value is mixed in. Does it make sense to verify difference for columns which are different only in non-unique-key property, not mixing in anything else?
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java@86
PS6, Line 86: assertNotEquals(stringCol1, isAutoIncrementing);
ditto
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java@88
PS6, Line 88: // Different by type
In addition, does it make sense to add 'same' scenarios for auto-incrementing and non-unique-key properties on columns?
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@805
PS6, Line 805: auto_incrementing
nit: auto-incrementing
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@805
PS6, Line 805: primary key
nit: the primary key
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@817
PS6, Line 817: auto_incrementing
nit: the auto-incrementing column
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@828
PS6, Line 828: auto_incrementing
auto-incrementing
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@838
PS6, Line 838: auto_incrementing
nit: auto-incrementing
http://gerrit.cloudera.org:8080/#/c/19384/6/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/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2582
PS6, Line 2582: } catch (IllegalArgumentException e) {
: fail(e.getMessage());
: }
What this try/catch block is for? Would be great to add a comment.
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2612
PS6, Line 2612: } catch (IllegalArgumentException e) {
: fail(e.getMessage());
: }
What this try/catch block is for? Would be great to add a comment.
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2631
PS6, Line 2631: }
Could you also add scenarios to try creating:
* schema with auto-incrementing column which isn't marked with nonUniqueKey(true)
* schema with auto-incrementing column which is marked with with key(true)
* schema with auto-incrementing column which is marked as nullable
* schema with auto-incrementing column which is marked as immutable
* schema with auto-incrementing column with type String or INT32 (instead of INT64)
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 23 Dec 2022 01:54:34 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#18). ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the key column. The non-unique
key columns and the auto-incrementing column together form the
effective primary key.
UPSERT/UPSERT_IGNORE operations are not supported now for Kudu table
with auto-incrementing column due to limitation in Kudu server.
Auto-Incrementing column cannot be added, removed or renamed with
Alter Table APIs.
Testing:
- Added unit-test for Java client library.
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
describe/insert/update/delete/upsert/CTAS/select/alter, etc.
Passed Kudu related end-end tests.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduScanner.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/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
11 files changed, 586 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/18
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 18
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kurt Deschler, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19384
to look at the new patch set (#6).
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the last key column.
Testing:
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
insert/update/delete/CTAS.
- Adds unit-test for Java client.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
9 files changed, 336 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/6
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 10:
(5 comments)
It seems there is a build failure:
15:16:19 [ 69%] Building CXX object src/kudu/client/CMakeFiles/kudu_client_test_util.dir/client-test-util.cc.o
15:16:19 /home/jenkins-slave/workspace/kudu-master/0/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:111: error: cannot find symbol
15:16:19 .setIsAutoIncrementing(column.isAutoIncrementing())
15:16:19 ^
15:16:19 symbol: method setIsAutoIncrementing(boolean)
15:16:19 location: class Builder
15:16:19 /home/jenkins-slave/workspace/kudu-master/0/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:177: error: cannot find symbol
15:16:19 .autoIncrementing(pb.getIsAutoIncrementing())
15:16:19 ^
15:16:19 symbol: method getIsAutoIncrementing()
15:16:19 location: variable pb of type ColumnSchemaPB
15:16:19 /home/jenkins-slave/workspace/kudu-master/0/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:209: error: cannot find symbol
15:16:19 if (columnPb.getIsAutoIncrementing()) {
15:16:19 ^
15:16:19 symbol: method getIsAutoIncrementing()
15:16:19 location: variable columnPb of type ColumnSchemaPB
http://gerrit.cloudera.org:8080/#/c/19384/10/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
File java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java:
http://gerrit.cloudera.org:8080/#/c/19384/10/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java@397
PS10, Line 397: /**
: * Sets if the column is auto-incrementing column. False by default.
: * @param autoIncrementing a boolean that indicates if the column is
: * an auto-incrementing column
: * @return this instance
: */
: public ColumnSchemaBuilder autoIncrementing(boolean autoIncrementing) {
: this.autoIncrementing = autoIncrementing;
: return this;
: }
BTW, did you explore an alternative here: add a new helper class AutoIncrementingColumnSchemaBuilder instead with only a subset of methods that make sense for an auto-incrementing column and don't add this ColumnSchemaBuilder.autoIncrementing() method?
The new helper would set the type and other related attributes for the column automatically.
I guess going forward with that alternative would require corresponding change in the C++ client as well, so it's not necessary to make the change in this patch right away. Just curious whether it's a viable alternative from you point of view.
Thanks!
http://gerrit.cloudera.org:8080/#/c/19384/10/java/kudu-client/src/main/java/org/apache/kudu/Schema.java
File java/kudu-client/src/main/java/org/apache/kudu/Schema.java:
http://gerrit.cloudera.org:8080/#/c/19384/10/java/kudu-client/src/main/java/org/apache/kudu/Schema.java@125
PS10, Line 125: findKey
nit: maybe, rename this into 'isKeyFound' for better readability?
http://gerrit.cloudera.org:8080/#/c/19384/6/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/19384/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@655
PS6, Line 655: // Make a copy of mutable list of column schema, then add an auto-incrementing
: // column after the columns marked as key columns.
: List<ColumnSchema> colSchemas = new ArrayList<>(schema.getColumns());
: Preconditions.checkNotNull(colSchemas);
> Yes, auto-incrementing column is added after all columns which are marked a
Thank you for the clarification!
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@157
PS6, Line 157: public static ColumnSchema pbToColumnSchema(Common.ColumnSchemaPB pb,
: boolean isKeyUnique) {
> We need to mark all of key columns as non unique key columns, including the
Ah, I see -- it seems I missed the context of the call sites for this method and was confused by the name of the new parameter.
Thanks for the update.
http://gerrit.cloudera.org:8080/#/c/19384/6/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/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2631
PS6, Line 2631: columns.add(new ColumnSchema.ColumnSchemaBuilder("key", Type.INT64)
> added new test cases
Thanks a lot!
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Jan 2023 05:44:01 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kurt Deschler, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19384
to look at the new patch set (#13).
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the last key column.
Testing:
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
insert/update/delete/CTAS.
- Added unit-test for Java client.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
9 files changed, 373 insertions(+), 24 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/13
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 13
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 14: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 14
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 10 Jan 2023 06:44:19 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#20). ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_incrementing_id" will
be added automatically to the table as the key column. The non-unique
key columns and the auto-incrementing column together form the
effective primary key.
UPSERT/UPSERT_IGNORE operations are not supported now for Kudu table
with auto-incrementing column due to limitation in Kudu server.
Auto-Incrementing column cannot be added, removed or renamed with
Alter Table APIs.
Testing:
- Added unit-test for Java client library.
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
describe/insert/update/delete/upsert/CTAS/select/alter, etc.
Passed Kudu related end-to-end tests.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduScanner.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/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
10 files changed, 676 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/20
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 20
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19384
to look at the new patch set (#2).
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the last key column.
Testing:
- Manually ran integration test with Impala for creating table
with non unique primary key, ran tests for operations:
insert/update/delete for non unique primary key.
- TODO add unit-test
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAsyncKuduClient.java
7 files changed, 207 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/2
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 2
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19384
to look at the new patch set (#4).
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the last key column.
Testing:
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
insert/update/delete/CTAS.
- Adds unit-test for Java client.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
9 files changed, 319 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/4
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 4
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 13:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19384/10/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
File java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java:
http://gerrit.cloudera.org:8080/#/c/19384/10/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java@397
PS10, Line 397: * @param defaultValue a Java object representation of the default value that's read
: * @return this instance
: */
: public ColumnSchemaBuilder defaultValue(Object defaultValue) {
: this.defaultValue = defaultValue;
: return this;
: }
:
: /**
:
> That's good suggestion. It will reduce the error.
added new helper class AutoIncrementingColumnSchemaBuilder.
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 13
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Jan 2023 22:38:57 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 13:
(2 comments)
> (2 comments)
>
> The build failure was due to the dependency on the server side
> change https://gerrit.cloudera.org/#/c/19097/. The code change is
> dependent on the additional fields added to src/kudu/common/common.proto
> by that patch.
Well, you could base your patch on top of that one: first check out the version of that change in your workspace and then create a branch on top of that. To check out the branch in question from gerrit, use corresponding link from the 'Download' dropdown in the upper right corner of the gerrit's UI.
http://gerrit.cloudera.org:8080/#/c/19384/13/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/19384/13/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@652
PS13, Line 652: AutoIncrementingColumnSchemaBuilder(
: Schema.getAutoIncrementingColumnName())
Maybe, add AutoIncrementingColumnSchemaBuilder() constructor that takes no parameters and sets the name for the column to be Schema.getAutoIncrementingColumnName()?
http://gerrit.cloudera.org:8080/#/c/19384/13/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:
http://gerrit.cloudera.org:8080/#/c/19384/13/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@161
PS13, Line 161: } else {
: Type type = Type.getTypeForDataType(pb.getType());
: ColumnTypeAttributes typeAttributes = pb.hasTypeAttributes() ?
: pbToColumnTypeAttributes(pb.getTypeAttributes()) : null;
: Object defaultValue = pb.hasWriteDefaultValue() ?
: byteStringToObject(type, typeAttributes, pb.getWriteDefaultValue()) : null;
: ColumnSchema.Encoding encoding = ColumnSchema.Encoding.valueOf(pb.getEncoding().name());
: ColumnSchema.CompressionAlgorithm compressionAlgorithm =
: ColumnSchema.CompressionAlgorithm.valueOf(pb.getCompression().name());
: int desiredBlockSize = pb.getCfileBlockSize();
: ColumnSchema.ColumnSchemaBuilder csb =
: new ColumnSchema.ColumnSchemaBuilder(pb.getName(), type);
: if (pb.getIsKey() && !isKeyUnique) {
: csb.nonUniqueKey(true);
: } else {
: csb.key(pb.getIsKey());
: }
: return csb.nullable(pb.getIsNullable())
: .immutable(pb.getImmutable())
: .defaultValue(defaultValue)
: .encoding(encoding)
: .compressionAlgorithm(compressionAlgorithm)
: .desiredBlockSize(desiredBlockSize)
: .typeAttributes(typeAttributes)
: .comment(pb.getComment())
: .build();
: }
nit: for better readability, move this out of the 'else' case since there is 'return' in the 'if()' clause above.
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 13
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 05 Jan 2023 00:41:07 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 21:
Test failed in AdminCliTest.TestUnsafeChangeConfigForConfigWithTwoNodes, which seems irrelevant.
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 21
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 19 Jan 2023 23:06:29 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 21: Verified+1
unrelated test failure in AdminCliTest.TestUnsafeChangeConfigForConfigWithTwoNodes (DEBUG)
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 21
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Fri, 20 Jan 2023 04:00:39 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has uploaded a new patch set (#19). ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_incrementing_id" will
be added automatically to the table as the key column. The non-unique
key columns and the auto-incrementing column together form the
effective primary key.
UPSERT/UPSERT_IGNORE operations are not supported now for Kudu table
with auto-incrementing column due to limitation in Kudu server.
Auto-Incrementing column cannot be added, removed or renamed with
Alter Table APIs.
Testing:
- Added unit-test for Java client library.
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
describe/insert/update/delete/upsert/CTAS/select/alter, etc.
Passed Kudu related end-to-end tests.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduScanner.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/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
11 files changed, 652 insertions(+), 19 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/19
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 19
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19384
to look at the new patch set (#3).
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the last key column.
Testing:
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
insert/update/delete/CTAS.
- Adds unit-test for Java client.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
M java/kudu-test-utils/src/main/java/org/apache/kudu/test/ClientTestUtil.java
8 files changed, 250 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/3
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 3
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 20:
(3 comments)
Overall looks good to me!
The only issues left are some nits pointed by LINT build:
http://jenkins.kudu.apache.org/job/kudu-gerrit/27073/BUILD_TYPE=LINT/console
An excerpt:
11:20:08 > Task :kudu-client:checkstyleMain
11:20:08 [ant:checkstyle] [WARN] /home/jenkins-slave/workspace/kudu-master/2/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java:161:7: 'method def' child has incorrect indentation level 6, expected level should be 4. [Indentation]
11:20:08 [ant:checkstyle] [WARN] /home/jenkins-slave/workspace/kudu-master/2/java/kudu-client/src/main/java/org/apache/kudu/Schema.java:121:9: Local variable name 'KeyColumnCount' must match pattern '^[a-z]([a-z0-9][a-zA-Z0-9]*)?$'. [LocalVariableName]
11:20:08 [ant:checkstyle] [WARN] /home/jenkins-slave/workspace/kudu-master/2/java/kudu-client/src/main/java/org/apache/kudu/Schema.java:140:10: 'else' child has incorrect indentation level 9, expected level should be 10. [Indentation]
11:20:08
11:20:08 > Task :kudu-client:checkstyleMain FAILED
11:20:16
11:20:16 > Task :kudu-client:checkstyleTest
11:20:16 [ant:checkstyle] [WARN] /home/jenkins-slave/workspace/kudu-master/2/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:899:5: Distance between variable 'session' declaration and its first usage is 15, but allowed 3. Consider making that variable final if you still need to store its value in advance (before method calls that might have side effects on the original value). [VariableDeclarationUsageDistance]
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@810
PS18, Line 810: assertTrue(schema.hasAutoIncrementingColumn());
: assertEquals(3, schema.getColumnCount());
> Now the column counts are same
That's a great news. Thank you very much for updating that!
http://gerrit.cloudera.org:8080/#/c/19384/18/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/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2580
PS18, Line 2580: chema.isPrimaryKeyUnique());
> Updated code to have same behavior as C++ client.
Thanks a lot!
http://gerrit.cloudera.org:8080/#/c/19384/18/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2630
PS18, Line 2630:
> Added a test case with a new table which has single auto-incrementing colum
Ah, I see -- that makes sense. Thank you!
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 20
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Thu, 19 Jan 2023 19:23:52 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has removed a vote on this change.
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Removed Verified-1 by Kudu Jenkins (120)
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 21
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kurt Deschler, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19384
to look at the new patch set (#9).
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the last key column.
Testing:
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
insert/update/delete/CTAS.
- Adds unit-test for Java client.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
9 files changed, 437 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/9
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 9
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Hello Marton Greber, Alexey Serbin, Kurt Deschler, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19384
to look at the new patch set (#10).
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
KUDU-1945: Support non unique primary key for Java client
This patch adds new APIs to create ColumnSchema with non unique
primary key for Java client. When a table with non unique primary
key is created, Auto-Incrementing Column "auto_increment_id" will
be added automatically to the table as the last key column.
Testing:
- Manually ran integration test with Impala for creating table
with non unique primary key, and ran queries for operations:
insert/update/delete/CTAS.
- Adds unit-test for Java client.
Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
---
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/AsyncKuduClient.java
M java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
M java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
M java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.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
9 files changed, 451 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/84/19384/10
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 10:
(2 comments)
The build failure was due to the dependency on the server side change https://gerrit.cloudera.org/#/c/19097/. The code change is dependent on the additional fields added to src/kudu/common/common.proto by that patch.
http://gerrit.cloudera.org:8080/#/c/19384/10/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
File java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java:
http://gerrit.cloudera.org:8080/#/c/19384/10/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java@397
PS10, Line 397: /**
: * Sets if the column is auto-incrementing column. False by default.
: * @param autoIncrementing a boolean that indicates if the column is
: * an auto-incrementing column
: * @return this instance
: */
: public ColumnSchemaBuilder autoIncrementing(boolean autoIncrementing) {
: this.autoIncrementing = autoIncrementing;
: return this;
: }
> BTW, did you explore an alternative here: add a new helper class AutoIncrem
That's good suggestion. It will reduce the error.
http://gerrit.cloudera.org:8080/#/c/19384/10/java/kudu-client/src/main/java/org/apache/kudu/Schema.java
File java/kudu-client/src/main/java/org/apache/kudu/Schema.java:
http://gerrit.cloudera.org:8080/#/c/19384/10/java/kudu-client/src/main/java/org/apache/kudu/Schema.java@125
PS10, Line 125: findKey
> nit: maybe, rename this into 'isKeyFound' for better readability?
Done
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 10
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Jan 2023 06:57:37 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945: Support non unique primary key for Java client
Posted by "Wenzhe Zhou (Code Review)" <ge...@cloudera.org>.
Wenzhe Zhou has posted comments on this change. ( http://gerrit.cloudera.org:8080/19384 )
Change subject: KUDU-1945: Support non unique primary key for Java client
......................................................................
Patch Set 6:
(19 comments)
Thanks for review.
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java
File java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java:
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java@181
PS6, Line 181: auto incrementing
> nit for here and elsewhere: is it possible to get into some consistent nami
Done
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java@360
PS6, Line 360: is part
> nit: is a part
Done
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/ColumnSchema.java@400
PS6, Line 400: auto incrementing column
> nit: an auto incrementing column
Done
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/Schema.java
File java/kudu-client/src/main/java/org/apache/kudu/Schema.java:
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/Schema.java@44
PS6, Line 44: primary
> primary key
Done
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/Schema.java@343
PS6, Line 343: Get column name of auto incrementing column
> nit: Get the name of the auto-incrementing column
Done
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/Schema.java@351
PS6, Line 351: Get type of auto incrementing column
> nit: Get the type of the auto-incrementing column
Done
http://gerrit.cloudera.org:8080/#/c/19384/6/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/19384/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduClient.java@655
PS6, Line 655: // Create a copy of mutable list and insert auto incrementing column as last key
: // columns
: List<ColumnSchema> colSchemas = new ArrayList<>(schema.getColumns());
: Preconditions.checkNotNull(colSchemas);
> I thought auto-incrementing column comes after columns marked as key column
Yes, auto-incrementing column is added after all columns which are marked as key columns. Updated comments.
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java
File java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java:
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/client/AsyncKuduScanner.java@373
PS6, Line 373: auto_incrementing
> nit: auto-incrementing
Done
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java
File java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java:
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/main/java/org/apache/kudu/client/ProtobufHelper.java@157
PS6, Line 157: public static ColumnSchema pbToColumnSchema(Common.ColumnSchemaPB pb,
: boolean hasAutoIncrementing)
> Why to introduce this 'hasAutoIncementing' parameter when Common.ColumnSche
We need to mark all of key columns as non unique key columns, including the key columns which are not auto-incrementing columns if the table has auto_incrementing column. Rename second parameter as isUniqueKey.
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java
File java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java:
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java@79
PS6, Line 79: assertNotEquals(stringCol1, isNonUniqueKey);
> In this comparison, 'test' default value is mixed in. Does it make sense t
Change test code to compare with unique key
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java@86
PS6, Line 86: assertNotEquals(stringCol1, isAutoIncrementing);
> ditto
Done
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/TestColumnSchema.java@88
PS6, Line 88: // Different by type
> In addition, does it make sense to add 'same' scenarios for auto-incrementi
Done
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java:
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@805
PS6, Line 805: primary key
> nit: the primary key
Done
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@817
PS6, Line 817: auto_incrementing
> nit: the auto-incrementing column
Done
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@828
PS6, Line 828: auto_incrementing
> auto-incrementing
Done
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduClient.java@838
PS6, Line 838: auto_incrementing
> nit: auto-incrementing
Done
http://gerrit.cloudera.org:8080/#/c/19384/6/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/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2582
PS6, Line 2582: } catch (IllegalArgumentException e) {
: fail(e.getMessage());
: }
> What this try/catch block is for? Would be great to add a comment.
remove try/catch
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2612
PS6, Line 2612: } catch (IllegalArgumentException e) {
: fail(e.getMessage());
: }
> What this try/catch block is for? Would be great to add a comment.
remove try/catch
http://gerrit.cloudera.org:8080/#/c/19384/6/java/kudu-client/src/test/java/org/apache/kudu/client/TestKuduTable.java@2631
PS6, Line 2631: }
> Could you also add scenarios to try creating:
added new test cases
--
To view, visit http://gerrit.cloudera.org:8080/19384
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I7e2501d6b3d66f6466959e4f3f1ed0f5e08dfe5c
Gerrit-Change-Number: 19384
Gerrit-PatchSet: 6
Gerrit-Owner: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Wenzhe Zhou <wz...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Jan 2023 01:17:41 +0000
Gerrit-HasComments: Yes