You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Will Berkeley (Code Review)" <ge...@cloudera.org> on 2016/11/17 20:42:25 UTC
[kudu-CR] KUDU-1747 addColumn API is wrong wrt nullability and default values
Will Berkeley has uploaded a new change for review.
http://gerrit.cloudera.org:8080/5132
Change subject: KUDU-1747 addColumn API is wrong wrt nullability and default values
......................................................................
KUDU-1747 addColumn API is wrong wrt nullability and default values
Previously, the java client permitted adding
- a non-null column with a default
- a nullable column with no default
but did not allow adding
- a nullable column with a default
even though it should be allowed. This patch adds that capability
by adding a 'defaultVal' argument to an overloaded
AlterTableOptions.addNullableColumn. It is backwards-compatible.
Additionally, a test was added to check behavior of the three
addColumn possibilities.
Change-Id: I0706839a3dbe21532afd524e2d3fc020d31a9973
---
M java/kudu-client/src/main/java/org/apache/kudu/client/AlterTableOptions.java
M java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
2 files changed, 43 insertions(+), 1 deletion(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/5132/1
--
To view, visit http://gerrit.cloudera.org:8080/5132
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I0706839a3dbe21532afd524e2d3fc020d31a9973
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
[kudu-CR] [java] KUDU-1746/KUDU-1747 Improve addColumn API
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has posted comments on this change.
Change subject: [java] KUDU-1746/KUDU-1747 Improve addColumn API
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5132/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java:
Line 129: }
> I would also expand the test to make sure that I can insert a row that has
Done
--
To view, visit http://gerrit.cloudera.org:8080/5132
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0706839a3dbe21532afd524e2d3fc020d31a9973
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-1746/KUDU-1747 Improve addColumn API
Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Dimitris Tsirogiannis, Kudu Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/5132
to look at the new patch set (#2).
Change subject: [java] KUDU-1746/KUDU-1747 Improve addColumn API
......................................................................
[java] KUDU-1746/KUDU-1747 Improve addColumn API
Previously, the java client permitted adding
- a non-null column with a default
- a nullable column with no default
but did not allow adding
- a nullable column with a default
even though it should be allowed. This patch adds that capability
by adding a 'defaultVal' argument to an overloaded
AlterTableOptions.addNullableColumn.
It also adds a more general method to add a column based on a
ColumnSchema. This more advanced API should be easier to maintain
as more column options are added to Kudu. The other addColumn
variants are reimplemented in terms of this one.
Previous methods remain for backward-compatibility.
Additionally, a test was added to check behavior of the three
addColumn possibilities (which tests the new addColumn(ColumnSchema)
since they are implemented using it).
Finally, I fixed a checkstyle warning in AsyncKuduScanner.
Change-Id: I0706839a3dbe21532afd524e2d3fc020d31a9973
---
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/test/java/org/apache/kudu/client/TestAlterTable.java
3 files changed, 86 insertions(+), 21 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/32/5132/2
--
To view, visit http://gerrit.cloudera.org:8080/5132
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I0706839a3dbe21532afd524e2d3fc020d31a9973
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
[kudu-CR] [java] KUDU-1746/KUDU-1747 Improve addColumn API
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has submitted this change and it was merged.
Change subject: [java] KUDU-1746/KUDU-1747 Improve addColumn API
......................................................................
[java] KUDU-1746/KUDU-1747 Improve addColumn API
Previously, the java client permitted adding
- a non-null column with a default
- a nullable column with no default
but did not allow adding
- a nullable column with a default
even though it should be allowed. This patch adds that capability
by adding a 'defaultVal' argument to an overloaded
AlterTableOptions.addNullableColumn.
It also adds a more general method to add a column based on a
ColumnSchema. This more advanced API should be easier to maintain
as more column options are added to Kudu. The other addColumn
variants are reimplemented in terms of this one.
Previous methods remain for backward-compatibility.
Additionally, a test was added to check behavior of the three
addColumn possibilities (which tests the new addColumn(ColumnSchema)
since they are implemented using it).
Finally, I fixed a checkstyle warning in AsyncKuduScanner.
Change-Id: I0706839a3dbe21532afd524e2d3fc020d31a9973
Reviewed-on: http://gerrit.cloudera.org:8080/5132
Tested-by: Kudu Jenkins
Reviewed-by: Jean-Daniel Cryans <jd...@apache.org>
---
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/test/java/org/apache/kudu/client/TestAlterTable.java
3 files changed, 86 insertions(+), 21 deletions(-)
Approvals:
Jean-Daniel Cryans: Looks good to me, approved
Kudu Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/5132
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: merged
Gerrit-Change-Id: I0706839a3dbe21532afd524e2d3fc020d31a9973
Gerrit-PatchSet: 3
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
[kudu-CR] KUDU-1747 addColumn API is wrong wrt nullability and default values
Posted by "Dimitris Tsirogiannis (Code Review)" <ge...@cloudera.org>.
Dimitris Tsirogiannis has posted comments on this change.
Change subject: KUDU-1747 addColumn API is wrong wrt nullability and default values
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
http://gerrit.cloudera.org:8080/#/c/5132/1/java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java
File java/kudu-client/src/test/java/org/apache/kudu/client/TestAlterTable.java:
Line 129: }
I would also expand the test to make sure that I can insert a row that has a null in the addNullableDef column and when I read it back I get the null value and not some default.
--
To view, visit http://gerrit.cloudera.org:8080/5132
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0706839a3dbe21532afd524e2d3fc020d31a9973
Gerrit-PatchSet: 1
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-HasComments: Yes
[kudu-CR] [java] KUDU-1746/KUDU-1747 Improve addColumn API
Posted by "Jean-Daniel Cryans (Code Review)" <ge...@cloudera.org>.
Jean-Daniel Cryans has posted comments on this change.
Change subject: [java] KUDU-1746/KUDU-1747 Improve addColumn API
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/5132
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-MessageType: comment
Gerrit-Change-Id: I0706839a3dbe21532afd524e2d3fc020d31a9973
Gerrit-PatchSet: 2
Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Dimitris Tsirogiannis <dt...@cloudera.com>
Gerrit-Reviewer: Jean-Daniel Cryans <jd...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-HasComments: No