You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Marton Greber (Code Review)" <ge...@cloudera.org> on 2023/03/01 17:53:45 UTC
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Marton Greber has uploaded this change for review. ( http://gerrit.cloudera.org:8080/19566
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
KUDU-1945 Auto-incrementing column, Python client
This patch adds the Python part of the client side changes for the auto
incrementing column feature.
A new ColumnSpec called non_unique_primary_key is added. Semantically
it behaves like primary_key:
- only one column can have the non_unique_primary_key ColumnSpec in a
given SchemaBuilder context,
- if it exists, it must be defined in the first place,
- compound keys are defined through a set function.
Functionally non-unique primary keys don't need to fulfill the
uniqueness constraint. An auto incrementing column is added in the
background automatically once a non-unique primary key is specified. The
non-unique keys and the auto incrementing columns together form the
effective primary key.
Some technical notes:
- The name of the auto incrementing column is hardcoded into the
kudu.Schema class. This is a reserved column name, users can't create
columns with it. On the client facing side, this reserved string is
reachable through: kudu.Schema.get_auto_incrementing_column_name().
- In this initial version there is no support for UPSERT and
UPSERT_IGNORE operations.
- With non-unique primary key, one can't use the tuple/list
initialization for new inserts.
Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
---
M python/kudu/libkudu_client.pxd
M python/kudu/schema.pyx
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/test_schema.py
5 files changed, 290 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/19566/1
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 1
Gerrit-Owner: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19566 )
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
Patch Set 6: Code-Review+1
(12 comments)
http://gerrit.cloudera.org:8080/#/c/19566/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19566/6//COMMIT_MSG@18
PS6, Line 18: Functionally non-unique primary keys don't need to fulfill the
nit: add a comma after 'Functionally'?
http://gerrit.cloudera.org:8080/#/c/19566/6//COMMIT_MSG@21
PS6, Line 21: columns
Could there be more than one auto-incrementing column in a table? I guess there could not, so maybe change 'columns' --> 'column'?
http://gerrit.cloudera.org:8080/#/c/19566/6//COMMIT_MSG@28
PS6, Line 28: :
nit: drop the column?
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/libkudu_client.pxd
File python/kudu/libkudu_client.pxd:
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/libkudu_client.pxd@191
PS6, Line 191: @staticmethod
: string GetAutoIncrementingColumnName()
nit: please move this static method to come first after KuduSchema() constructor-like methods?
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/schema.pyx
File python/kudu/schema.pyx:
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/schema.pyx@456
PS6, Line 456:
nit: remove this extra empty line?
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_client.py
File python/kudu/tests/test_client.py:
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_client.py@252
PS6, Line 252: i
nit: use an anonymous variable here instead of i (i.e. _)?
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_client.py@386
PS6, Line 386: with self.assertRaises(TypeError):
: op = table.new_insert([1,'text'])
Why to do this second time?
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_scantoken.py
File python/kudu/tests/test_scantoken.py:
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_scantoken.py@324
PS6, Line 324: kudu.Schema.get_auto_incrementing_column_name()
Could you also add a scenario with projection including only this column name?
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_scantoken.py@338
PS6, Line 338: assert not kudu.Schema.get_auto_incrementing_column_name() in schema.names
Could we have an empty projection for a table with auto-incrementing column?
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_schema.py
File python/kudu/tests/test_schema.py:
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_schema.py@315
PS6, Line 315: set_non_unique_primary_keys(['key'])
Could you add a scenario with specifying the same column name twice for non-unique primary keys?
set_non_unique_primary_keys(['key', 'key'])
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_schema.py@332
PS6, Line 332: (builder.add_column('key', 'int64')
: .nullable(False)
: .primary_key()
: .non_unique_primary_key())
Could you also add a scenario like the following:
builder.add_column('key', 'int64')
.nullable(False)
.primary_key()
builder.add_column('x', 'int64')
.nullable(False)
.non_unique_primary_key()
?
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_schema.py@430
PS6, Line 430: ['key1', 'key2']
Could you add a scenario where the names of the columns is reverted w.r.t. their order in the schema, i.e. ['key2', 'key1']?
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 6
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Tue, 07 Mar 2023 07:26:18 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19566
to look at the new patch set (#9).
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
KUDU-1945 Auto-incrementing column, Python client
This patch adds the Python part of the client side changes for the auto
incrementing column feature.
A new ColumnSpec called non_unique_primary_key is added. Semantically
it behaves like primary_key:
- only one column can have the non_unique_primary_key ColumnSpec in a
given SchemaBuilder context,
- if it exists, it must be defined in the first place,
- compound keys are defined through a set function.
Functionally, non-unique primary keys don't need to fulfill the
uniqueness constraint. An auto incrementing column is added in the
background automatically once a non-unique primary key is specified. The
non-unique keys and the auto incrementing column together form the
effective primary key.
Some technical notes:
- The name of the auto incrementing column is hardcoded into the
kudu.Schema class. This is a reserved column name, users can't create
columns with it. On the client facing side, this reserved string is
reachable through kudu.Schema.get_auto_incrementing_column_name().
- In this initial version there is no support for UPSERT and
UPSERT_IGNORE operations.
- With non-unique primary key, one can't use the tuple/list
initialization for new inserts.
Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
---
M python/kudu/libkudu_client.pxd
M python/kudu/schema.pyx
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/test_schema.py
5 files changed, 494 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/19566/9
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 9
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19566
to look at the new patch set (#5).
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
KUDU-1945 Auto-incrementing column, Python client
This patch adds the Python part of the client side changes for the auto
incrementing column feature.
A new ColumnSpec called non_unique_primary_key is added. Semantically
it behaves like primary_key:
- only one column can have the non_unique_primary_key ColumnSpec in a
given SchemaBuilder context,
- if it exists, it must be defined in the first place,
- compound keys are defined through a set function.
Functionally non-unique primary keys don't need to fulfill the
uniqueness constraint. An auto incrementing column is added in the
background automatically once a non-unique primary key is specified. The
non-unique keys and the auto incrementing columns together form the
effective primary key.
Some technical notes:
- The name of the auto incrementing column is hardcoded into the
kudu.Schema class. This is a reserved column name, users can't create
columns with it. On the client facing side, this reserved string is
reachable through: kudu.Schema.get_auto_incrementing_column_name().
- In this initial version there is no support for UPSERT and
UPSERT_IGNORE operations.
- With non-unique primary key, one can't use the tuple/list
initialization for new inserts.
Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
---
M python/kudu/libkudu_client.pxd
M python/kudu/schema.pyx
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/test_schema.py
5 files changed, 289 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/19566/5
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 5
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19566
to look at the new patch set (#3).
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
KUDU-1945 Auto-incrementing column, Python client
This patch adds the Python part of the client side changes for the auto
incrementing column feature.
A new ColumnSpec called non_unique_primary_key is added. Semantically
it behaves like primary_key:
- only one column can have the non_unique_primary_key ColumnSpec in a
given SchemaBuilder context,
- if it exists, it must be defined in the first place,
- compound keys are defined through a set function.
Functionally non-unique primary keys don't need to fulfill the
uniqueness constraint. An auto incrementing column is added in the
background automatically once a non-unique primary key is specified. The
non-unique keys and the auto incrementing columns together form the
effective primary key.
Some technical notes:
- The name of the auto incrementing column is hardcoded into the
kudu.Schema class. This is a reserved column name, users can't create
columns with it. On the client facing side, this reserved string is
reachable through: kudu.Schema.get_auto_incrementing_column_name().
- In this initial version there is no support for UPSERT and
UPSERT_IGNORE operations.
- With non-unique primary key, one can't use the tuple/list
initialization for new inserts.
Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
---
M python/kudu/compat.py
M python/kudu/libkudu_client.pxd
M python/kudu/schema.pyx
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/test_schema.py
6 files changed, 285 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/19566/3
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 3
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19566
to look at the new patch set (#2).
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
KUDU-1945 Auto-incrementing column, Python client
This patch adds the Python part of the client side changes for the auto
incrementing column feature.
A new ColumnSpec called non_unique_primary_key is added. Semantically
it behaves like primary_key:
- only one column can have the non_unique_primary_key ColumnSpec in a
given SchemaBuilder context,
- if it exists, it must be defined in the first place,
- compound keys are defined through a set function.
Functionally non-unique primary keys don't need to fulfill the
uniqueness constraint. An auto incrementing column is added in the
background automatically once a non-unique primary key is specified. The
non-unique keys and the auto incrementing columns together form the
effective primary key.
Some technical notes:
- The name of the auto incrementing column is hardcoded into the
kudu.Schema class. This is a reserved column name, users can't create
columns with it. On the client facing side, this reserved string is
reachable through: kudu.Schema.get_auto_incrementing_column_name().
- In this initial version there is no support for UPSERT and
UPSERT_IGNORE operations.
- With non-unique primary key, one can't use the tuple/list
initialization for new inserts.
Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
---
M python/kudu/libkudu_client.pxd
M python/kudu/schema.pyx
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/test_schema.py
5 files changed, 289 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/19566/2
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 2
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19566 )
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
Patch Set 9: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 9
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Sun, 19 Mar 2023 14:58:44 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/19566 )
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
Patch Set 9: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 9
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Mon, 13 Mar 2023 18:47:58 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/19566 )
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
Patch Set 7:
(12 comments)
http://gerrit.cloudera.org:8080/#/c/19566/6//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/19566/6//COMMIT_MSG@18
PS6, Line 18: Functionally, non-unique primary keys don't need to fulfill the
> nit: add a comma after 'Functionally'?
Done
http://gerrit.cloudera.org:8080/#/c/19566/6//COMMIT_MSG@21
PS6, Line 21: column
> Could there be more than one auto-incrementing column in a table? I guess
Done
http://gerrit.cloudera.org:8080/#/c/19566/6//COMMIT_MSG@28
PS6, Line 28:
> nit: drop the column?
Done
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/libkudu_client.pxd
File python/kudu/libkudu_client.pxd:
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/libkudu_client.pxd@191
PS6, Line 191: size_t num_columns()
:
> nit: please move this static method to come first after KuduSchema() constr
Done
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/schema.pyx
File python/kudu/schema.pyx:
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/schema.pyx@456
PS6, Line 456: self.spec.NonUniquePrimaryKey()
> nit: remove this extra empty line?
Done
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_client.py
File python/kudu/tests/test_client.py:
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_client.py@252
PS6, Line 252: _
> nit: use an anonymous variable here instead of i (i.e. _)?
Done
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_client.py@386
PS6, Line 386: with self.assertRaises(TypeError):
: op = table.new_insert([1,'text'])
> Why to do this second time?
The first one checks with a tuple (1,'text'), while the second one with list [1,'text'].
This is more like an awareness test, to have it be documented here.
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_scantoken.py
File python/kudu/tests/test_scantoken.py:
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_scantoken.py@324
PS6, Line 324:
> Could you also add a scenario with projection including only this column na
Done
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_scantoken.py@338
PS6, Line 338: schema = scanner.get_projection_schema()
> Could we have an empty projection for a table with auto-incrementing column
Done
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_schema.py
File python/kudu/tests/test_schema.py:
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_schema.py@315
PS6, Line 315: set_non_unique_primary_keys(['key'])
> Could you add a scenario with specifying the same column name twice for non
Done
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_schema.py@332
PS6, Line 332: (builder.add_column('key', 'int64')
: .nullable(False))
: builder.set_non_unique_primary_keys(['key'])
: with self.assertRaises(Kudu
> Could you also add a scenario like the following:
Done
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_schema.py@430
PS6, Line 430: gument):
> Could you add a scenario where the names of the columns is reverted w.r.t.
Done
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 7
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Thu, 09 Mar 2023 22:41:48 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19566
to look at the new patch set (#8).
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
KUDU-1945 Auto-incrementing column, Python client
This patch adds the Python part of the client side changes for the auto
incrementing column feature.
A new ColumnSpec called non_unique_primary_key is added. Semantically
it behaves like primary_key:
- only one column can have the non_unique_primary_key ColumnSpec in a
given SchemaBuilder context,
- if it exists, it must be defined in the first place,
- compound keys are defined through a set function.
Functionally, non-unique primary keys don't need to fulfill the
uniqueness constraint. An auto incrementing column is added in the
background automatically once a non-unique primary key is specified. The
non-unique keys and the auto incrementing column together form the
effective primary key.
Some technical notes:
- The name of the auto incrementing column is hardcoded into the
kudu.Schema class. This is a reserved column name, users can't create
columns with it. On the client facing side, this reserved string is
reachable through kudu.Schema.get_auto_incrementing_column_name().
- In this initial version there is no support for UPSERT and
UPSERT_IGNORE operations.
- With non-unique primary key, one can't use the tuple/list
initialization for new inserts.
Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
---
M python/kudu/libkudu_client.pxd
M python/kudu/schema.pyx
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/test_schema.py
5 files changed, 494 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/19566/8
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 8
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Attila Bukor, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19566
to look at the new patch set (#7).
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
KUDU-1945 Auto-incrementing column, Python client
This patch adds the Python part of the client side changes for the auto
incrementing column feature.
A new ColumnSpec called non_unique_primary_key is added. Semantically
it behaves like primary_key:
- only one column can have the non_unique_primary_key ColumnSpec in a
given SchemaBuilder context,
- if it exists, it must be defined in the first place,
- compound keys are defined through a set function.
Functionally, non-unique primary keys don't need to fulfill the
uniqueness constraint. An auto incrementing column is added in the
background automatically once a non-unique primary key is specified. The
non-unique keys and the auto incrementing column together form the
effective primary key.
Some technical notes:
- The name of the auto incrementing column is hardcoded into the
kudu.Schema class. This is a reserved column name, users can't create
columns with it. On the client facing side, this reserved string is
reachable through kudu.Schema.get_auto_incrementing_column_name().
- In this initial version there is no support for UPSERT and
UPSERT_IGNORE operations.
- With non-unique primary key, one can't use the tuple/list
initialization for new inserts.
Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
---
M python/kudu/libkudu_client.pxd
M python/kudu/schema.pyx
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/test_schema.py
5 files changed, 494 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/19566/7
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 7
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19566
to look at the new patch set (#6).
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
KUDU-1945 Auto-incrementing column, Python client
This patch adds the Python part of the client side changes for the auto
incrementing column feature.
A new ColumnSpec called non_unique_primary_key is added. Semantically
it behaves like primary_key:
- only one column can have the non_unique_primary_key ColumnSpec in a
given SchemaBuilder context,
- if it exists, it must be defined in the first place,
- compound keys are defined through a set function.
Functionally non-unique primary keys don't need to fulfill the
uniqueness constraint. An auto incrementing column is added in the
background automatically once a non-unique primary key is specified. The
non-unique keys and the auto incrementing columns together form the
effective primary key.
Some technical notes:
- The name of the auto incrementing column is hardcoded into the
kudu.Schema class. This is a reserved column name, users can't create
columns with it. On the client facing side, this reserved string is
reachable through: kudu.Schema.get_auto_incrementing_column_name().
- In this initial version there is no support for UPSERT and
UPSERT_IGNORE operations.
- With non-unique primary key, one can't use the tuple/list
initialization for new inserts.
Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
---
M python/kudu/libkudu_client.pxd
M python/kudu/schema.pyx
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/test_schema.py
5 files changed, 433 insertions(+), 4 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/19566/6
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 6
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
[kudu-CR] KUDU-1945 Auto-incrementing column, Python 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/19566 )
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
KUDU-1945 Auto-incrementing column, Python client
This patch adds the Python part of the client side changes for the auto
incrementing column feature.
A new ColumnSpec called non_unique_primary_key is added. Semantically
it behaves like primary_key:
- only one column can have the non_unique_primary_key ColumnSpec in a
given SchemaBuilder context,
- if it exists, it must be defined in the first place,
- compound keys are defined through a set function.
Functionally, non-unique primary keys don't need to fulfill the
uniqueness constraint. An auto incrementing column is added in the
background automatically once a non-unique primary key is specified. The
non-unique keys and the auto incrementing column together form the
effective primary key.
Some technical notes:
- The name of the auto incrementing column is hardcoded into the
kudu.Schema class. This is a reserved column name, users can't create
columns with it. On the client facing side, this reserved string is
reachable through kudu.Schema.get_auto_incrementing_column_name().
- In this initial version there is no support for UPSERT and
UPSERT_IGNORE operations.
- With non-unique primary key, one can't use the tuple/list
initialization for new inserts.
Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Reviewed-on: http://gerrit.cloudera.org:8080/19566
Tested-by: Kudu Jenkins
Reviewed-by: Abhishek Chennaka <ac...@cloudera.com>
Reviewed-by: Yingchun Lai <la...@apache.org>
Reviewed-by: Alexey Serbin <al...@apache.org>
---
M python/kudu/libkudu_client.pxd
M python/kudu/schema.pyx
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/test_schema.py
5 files changed, 494 insertions(+), 4 deletions(-)
Approvals:
Kudu Jenkins: Verified
Abhishek Chennaka: Looks good to me, but someone else must approve
Yingchun Lai: Looks good to me, approved
Alexey Serbin: Looks good to me, approved
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 10
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Alexey Serbin (Code Review)" <ge...@cloudera.org>.
Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/19566 )
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
Patch Set 9: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_client.py
File python/kudu/tests/test_client.py:
http://gerrit.cloudera.org:8080/#/c/19566/6/python/kudu/tests/test_client.py@386
PS6, Line 386: with self.assertRaises(TypeError):
: op = table.new_insert([1,'text'])
> The first one checks with a tuple (1,'text'), while the second one with lis
ah, indeed -- thanks for clarifying
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 9
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Sun, 19 Mar 2023 16:40:50 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/19566 )
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
Patch Set 9: Code-Review+1
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 9
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Yingchun Lai <la...@apache.org>
Gerrit-Comment-Date: Tue, 14 Mar 2023 03:53:03 +0000
Gerrit-HasComments: No
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Hello Alexey Serbin, Kudu Jenkins, Abhishek Chennaka,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/19566
to look at the new patch set (#4).
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
KUDU-1945 Auto-incrementing column, Python client
This patch adds the Python part of the client side changes for the auto
incrementing column feature.
A new ColumnSpec called non_unique_primary_key is added. Semantically
it behaves like primary_key:
- only one column can have the non_unique_primary_key ColumnSpec in a
given SchemaBuilder context,
- if it exists, it must be defined in the first place,
- compound keys are defined through a set function.
Functionally non-unique primary keys don't need to fulfill the
uniqueness constraint. An auto incrementing column is added in the
background automatically once a non-unique primary key is specified. The
non-unique keys and the auto incrementing columns together form the
effective primary key.
Some technical notes:
- The name of the auto incrementing column is hardcoded into the
kudu.Schema class. This is a reserved column name, users can't create
columns with it. On the client facing side, this reserved string is
reachable through: kudu.Schema.get_auto_incrementing_column_name().
- In this initial version there is no support for UPSERT and
UPSERT_IGNORE operations.
- With non-unique primary key, one can't use the tuple/list
initialization for new inserts.
Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
---
M python/kudu/libkudu_client.pxd
M python/kudu/schema.pyx
M python/kudu/tests/test_client.py
M python/kudu/tests/test_scantoken.py
M python/kudu/tests/test_schema.py
5 files changed, 281 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/66/19566/4
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 4
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Abhishek Chennaka (Code Review)" <ge...@cloudera.org>.
Abhishek Chennaka has posted comments on this change. ( http://gerrit.cloudera.org:8080/19566 )
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19566/2/python/kudu/schema.pyx
File python/kudu/schema.pyx:
http://gerrit.cloudera.org:8080/#/c/19566/2/python/kudu/schema.pyx@848
PS2, Line 848: get_auto_incrementing_column_name
I am trying to understand the purpose of this method. Are we relying on the c++ client's method when building schema or do we use this method?
http://gerrit.cloudera.org:8080/#/c/19566/2/python/kudu/tests/test_client.py
File python/kudu/tests/test_client.py:
http://gerrit.cloudera.org:8080/#/c/19566/2/python/kudu/tests/test_client.py@240
PS2, Line 240: non_unique_primary_key
Maybe have an another test with multiple non unique primary keys?
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 2
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 02 Mar 2023 18:16:43 +0000
Gerrit-HasComments: Yes
[kudu-CR] KUDU-1945 Auto-incrementing column, Python client
Posted by "Marton Greber (Code Review)" <ge...@cloudera.org>.
Marton Greber has posted comments on this change. ( http://gerrit.cloudera.org:8080/19566 )
Change subject: KUDU-1945 Auto-incrementing column, Python client
......................................................................
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/19566/2/python/kudu/schema.pyx
File python/kudu/schema.pyx:
http://gerrit.cloudera.org:8080/#/c/19566/2/python/kudu/schema.pyx@848
PS2, Line 848: """
> I am trying to understand the purpose of this method. Are we relying on the
As far as I understand KuduSchema.GetAutoIncrementingColumnName() is working because it is noted in libkudu_client.pxd. This get_auto_incrementing_column_name() method is basically the python wrapper function.
http://gerrit.cloudera.org:8080/#/c/19566/2/python/kudu/tests/test_client.py
File python/kudu/tests/test_client.py:
http://gerrit.cloudera.org:8080/#/c/19566/2/python/kudu/tests/test_client.py@240
PS2, Line 240: non_unique_primary_key
> Maybe have an another test with multiple non unique primary keys?
If I understand correctly, what has been tested on the cpp side, should work here as well. (I think we had test for multiple non-unique keys)
My approach has been to cover basic functionality, and asserting some errors with the tests. Else, I could just copy more tests, which we already have in the cpp patch.
Now I'm not sure about the python testing convetions, best practices. I would like to let Alexey opine on this.
I'm happy to add more tests etc.
--
To view, visit http://gerrit.cloudera.org:8080/19566
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I94622680c5eb32eb2746a3b84c73699c1a37618c
Gerrit-Change-Number: 19566
Gerrit-PatchSet: 5
Gerrit-Owner: Marton Greber <gr...@gmail.com>
Gerrit-Reviewer: Abhishek Chennaka <ac...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <al...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Marton Greber <gr...@gmail.com>
Gerrit-Comment-Date: Sat, 04 Mar 2023 21:18:17 +0000
Gerrit-HasComments: Yes