You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Anonymous Coward (Code Review)" <ge...@cloudera.org> on 2021/09/16 07:41:32 UTC

[kudu-CR] [tool] kudu tool, support add column 

shenxingwuying@gmail.com has uploaded this change for review. ( http://gerrit.cloudera.org:8080/17851


Change subject: [tool] kudu tool, support add column
......................................................................

[tool] kudu tool, support add column

kudu tool not support adding column, add the tool

Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
---
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_table.cc
4 files changed, 160 insertions(+), 0 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/17851/1
-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 1
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>

[kudu-CR] [tool] CLI to add a column

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Attila Bukor, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17851

to look at the new patch set (#5).

Change subject: [tool] CLI to add a column
......................................................................

[tool] CLI to add a column

usage:
  kudu table add_column <master_addresses> <table_name> \
  <column_name> <data_type> [-encoding_type=<type>] \
  [-compression_type=<type>] [-default_value=<value>] \
  [-comment=<comment>]

Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
---
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_table.cc
4 files changed, 155 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/17851/5
-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 5
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-3339 [tool] CLI to add a column

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17851 )

Change subject: KUDU-3339 [tool] CLI to add a column
......................................................................


Patch Set 9: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17851/9//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17851/9//COMMIT_MSG@7
PS9, Line 7: KUDU-3339 [tool] CLI to add a column
nit: swap [tool] and KUDU-3339



-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 15 Dec 2021 12:35:03 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3339 [tool] CLI to add a column

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Attila Bukor, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17851

to look at the new patch set (#9).

Change subject: KUDU-3339 [tool] CLI to add a column
......................................................................

KUDU-3339 [tool] CLI to add a column

usage:
  kudu table add_column <master_addresses> <table_name> \
  <column_name> <data_type> [-encoding_type=<type>] \
  [-compression_type=<type>] [-default_value=<value>] \
  [-comment=<comment>]

Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
---
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_table.cc
4 files changed, 155 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/17851/9
-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] kudu tool, support add column

Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17851 )

Change subject: [tool] kudu tool, support add column
......................................................................


Patch Set 2:

(6 comments)

http://gerrit.cloudera.org:8080/#/c/17851/2//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17851/2//COMMIT_MSG@7
PS2, Line 7: kudu tool, support add column
CLI to add a column


http://gerrit.cloudera.org:8080/#/c/17851/2/src/kudu/client/schema.h
File src/kudu/client/schema.h:

http://gerrit.cloudera.org:8080/#/c/17851/2/src/kudu/client/schema.h@310
PS2, Line 310: /// @todo maybe should united as one, such as kudu::EncodingType(pb) and
             :   /// kudu::client::KuduColumnStorageAttributes::EncodingType, kudu::CompressionType(pb)
             :   /// and kudu::client::KuduColumnStorageAttributes::CompressionType
             :   /// @return Storage attributes of the column
I don't understand this comment.


http://gerrit.cloudera.org:8080/#/c/17851/2/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:

http://gerrit.cloudera.org:8080/#/c/17851/2/src/kudu/tools/tool_action_table.cc@128
PS2, Line 128:              "AUTO_ENCODING, PLAIN_ENCODING, PREFIX_ENCODING, RLE, DICT_ENCODING, "
             :               "BIT_SHUFFLE, GROUP_VARINT");
Type of encoding for the column including AUTO_ENCODING, PLAIN_ENCODING...


http://gerrit.cloudera.org:8080/#/c/17851/2/src/kudu/tools/tool_action_table.cc@131
PS2, Line 131: "DEFAULT_COMPRESSION, NO_COMPRESSION, SNAPPY, LZ4, ZLIB"
Type of compression for the column;


http://gerrit.cloudera.org:8080/#/c/17851/2/src/kudu/tools/tool_action_table.cc@985
PS2, Line 985:     const string& data_type_name = FindOrDie(context.required_args, kDataTypeArg);
             :     KuduColumnSchema::DataType data_type;
             :     RETURN_NOT_OK(KuduColumnSchema::StringToDataType(data_type_name, &data_type));
This required param should go at the top after column_name.


http://gerrit.cloudera.org:8080/#/c/17851/2/src/kudu/tools/tool_action_table.cc@984
PS2, Line 984:   {
             :     const string& data_type_name = FindOrDie(context.required_args, kDataTypeArg);
             :     KuduColumnSchema::DataType data_type;
             :     RETURN_NOT_OK(KuduColumnSchema::StringToDataType(data_type_name, &data_type));
             :     column_spec->Type(data_type);
             : 
             :     if (!FLAGS_default_value.empty()) {
             :       KuduValue* value = nullptr;
             :       RETURN_NOT_OK(ParseValueOfType(FLAGS_default_value, data_type, &value));
             :       column_spec->Default(value);
             :     }
             :   }
Nit: Don't see a need for the block scope and below as well.



-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 29 Sep 2021 23:16:14 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3339 [tool] CLI to add a column

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Attila Bukor, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17851

to look at the new patch set (#10).

Change subject: KUDU-3339 [tool] CLI to add a column
......................................................................

KUDU-3339 [tool] CLI to add a column

usage:
  kudu table add_column <master_addresses> <table_name> \
  <column_name> <data_type> [-encoding_type=<type>] \
  [-compression_type=<type>] [-default_value=<value>] \
  [-comment=<comment>]

Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
---
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_table.cc
4 files changed, 155 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/17851/10
-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3339 [tool] CLI to add a column

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/17851 )

Change subject: KUDU-3339 [tool] CLI to add a column
......................................................................

KUDU-3339 [tool] CLI to add a column

usage:
  kudu table add_column <master_addresses> <table_name> \
  <column_name> <data_type> [-encoding_type=<type>] \
  [-compression_type=<type>] [-default_value=<value>] \
  [-comment=<comment>]

Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Reviewed-on: http://gerrit.cloudera.org:8080/17851
Tested-by: Kudu Jenkins
Reviewed-by: Yingchun Lai <ac...@gmail.com>
---
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_table.cc
4 files changed, 155 insertions(+), 0 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Yingchun Lai: Looks good to me, approved

-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 11
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] KUDU-3339 [tool] CLI to add a column

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17851 )

Change subject: KUDU-3339 [tool] CLI to add a column
......................................................................


Patch Set 10: Code-Review+1


-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 16 Dec 2021 09:29:19 +0000
Gerrit-HasComments: No

[kudu-CR] [tool] CLI to add a column

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17851 )

Change subject: [tool] CLI to add a column
......................................................................


Patch Set 5:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17851/5//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17851/5//COMMIT_MSG@7
PS5, Line 7: [tool] CLI to add a column
I think it's worth opening a JIRA for this and add it to the commit message.



-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 5
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Tue, 23 Nov 2021 16:47:38 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3339 [tool] CLI to add a column

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17851 )

Change subject: KUDU-3339 [tool] CLI to add a column
......................................................................


Patch Set 8: Code-Review+1


-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 8
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Fri, 26 Nov 2021 07:27:08 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3339 [tool] CLI to add a column

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Attila Bukor, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17851

to look at the new patch set (#8).

Change subject: KUDU-3339 [tool] CLI to add a column
......................................................................

KUDU-3339 [tool] CLI to add a column

usage:
  kudu table add_column <master_addresses> <table_name> \
  <column_name> <data_type> [-encoding_type=<type>] \
  [-compression_type=<type>] [-default_value=<value>] \
  [-comment=<comment>]

Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
---
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_table.cc
4 files changed, 155 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/17851/8
-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 8
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] CLI to add a column

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17851 )

Change subject: [tool] CLI to add a column
......................................................................


Patch Set 5: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 5
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 24 Nov 2021 09:32:20 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3339 [tool] CLI to add a column

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17851 )

Change subject: KUDU-3339 [tool] CLI to add a column
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17851/7/src/kudu/client/schema.h
File src/kudu/client/schema.h:

http://gerrit.cloudera.org:8080/#/c/17851/7/src/kudu/client/schema.h@310
PS7, Line 310: column;
column schema.



-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 7
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Thu, 25 Nov 2021 12:57:19 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tool] CLI to add a column

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17851

to look at the new patch set (#4).

Change subject: [tool] CLI to add a column
......................................................................

[tool] CLI to add a column

kudu tool not support adding a column, support it

Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
---
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_table.cc
4 files changed, 155 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/17851/4
-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-3339 [tool] CLI to add a column

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17851 )

Change subject: KUDU-3339 [tool] CLI to add a column
......................................................................


Patch Set 10: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 10
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Tue, 21 Dec 2021 02:28:57 +0000
Gerrit-HasComments: No

[kudu-CR] KUDU-3339 [tool] CLI to add a column

Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17851 )

Change subject: KUDU-3339 [tool] CLI to add a column
......................................................................


Patch Set 9: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17851/9/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:

http://gerrit.cloudera.org:8080/#/c/17851/9/src/kudu/tools/tool_action_table.cc@1024
PS9, Line 1024:  
Nit: This indentation looks off.



-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Mon, 13 Dec 2021 15:39:04 +0000
Gerrit-HasComments: Yes

[kudu-CR] KUDU-3339 [tool] CLI to add a column

Posted by "Yingchun Lai (Code Review)" <ge...@cloudera.org>.
Yingchun Lai has posted comments on this change. ( http://gerrit.cloudera.org:8080/17851 )

Change subject: KUDU-3339 [tool] CLI to add a column
......................................................................


Patch Set 9:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17851/9/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:

http://gerrit.cloudera.org:8080/#/c/17851/9/src/kudu/tools/tool_action_table.cc@1024
PS9, Line 1024:  
> Nit: This indentation looks off.
+1, and Kudu use 2 blank spaces for indentation.



-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 9
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>
Gerrit-Comment-Date: Wed, 15 Dec 2021 02:13:14 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tool] kudu tool, support add column

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17851

to look at the new patch set (#2).

Change subject: [tool] kudu tool, support add column
......................................................................

[tool] kudu tool, support add column

kudu tool not support adding column, add the tool

Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
---
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_table.cc
4 files changed, 160 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/17851/2
-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 2
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] [tool] CLI to add a column

Posted by "Bankim Bhavsar (Code Review)" <ge...@cloudera.org>.
Bankim Bhavsar has posted comments on this change. ( http://gerrit.cloudera.org:8080/17851 )

Change subject: [tool] CLI to add a column
......................................................................


Patch Set 4: Code-Review+1

(1 comment)

http://gerrit.cloudera.org:8080/#/c/17851/4/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:

http://gerrit.cloudera.org:8080/#/c/17851/4/src/kudu/tools/tool_action_table.cc@1620
PS4, Line 1620:       .AddAction(std::move(add_column))
This list is alphabetically sorted so it'd be good to keep it that way



-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Fri, 19 Nov 2021 23:18:01 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tool] kudu tool, support add column

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17851

to look at the new patch set (#3).

Change subject: [tool] kudu tool, support add column
......................................................................

[tool] kudu tool, support add column

kudu tool not support adding column, add the tool

Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
---
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_table.cc
4 files changed, 160 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/17851/3
-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 3
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR] KUDU-3339 [tool] CLI to add a column

Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Hello Yingchun Lai, Attila Bukor, Kudu Jenkins, Bankim Bhavsar, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/17851

to look at the new patch set (#6).

Change subject: KUDU-3339 [tool] CLI to add a column
......................................................................

KUDU-3339 [tool] CLI to add a column

usage:
  kudu table add_column <master_addresses> <table_name> \
  <column_name> <data_type> [-encoding_type=<type>] \
  [-compression_type=<type>] [-default_value=<value>] \
  [-comment=<comment>]

Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
---
M src/kudu/client/schema.cc
M src/kudu/client/schema.h
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_table.cc
4 files changed, 155 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/51/17851/6
-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 6
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Yingchun Lai <ac...@gmail.com>

[kudu-CR] [tool] CLI to add a column

Posted by "Attila Bukor (Code Review)" <ge...@cloudera.org>.
Attila Bukor has posted comments on this change. ( http://gerrit.cloudera.org:8080/17851 )

Change subject: [tool] CLI to add a column
......................................................................


Patch Set 4:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/17851/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/17851/4//COMMIT_MSG@9
PS4, Line 9: kudu tool not support adding a column, support it
This body doesn't really add any extra information that the subject doesn't already tell.


http://gerrit.cloudera.org:8080/#/c/17851/4/src/kudu/tools/kudu-tool-test.cc
File src/kudu/tools/kudu-tool-test.cc:

http://gerrit.cloudera.org:8080/#/c/17851/4/src/kudu/tools/kudu-tool-test.cc@4725
PS4, Line 4725:   KuduSchemaBuilder schema_builder_2;
nit: remove the underscore before "2". Also, how about putting both this and the above schema building in separate blocks? That way the name could be reused.


http://gerrit.cloudera.org:8080/#/c/17851/4/src/kudu/tools/kudu-tool-test.cc@4736
PS4, Line 4736:   KuduSchema schema_2;
nit: can you use a more descriptive name, such as reference_schema?


http://gerrit.cloudera.org:8080/#/c/17851/4/src/kudu/tools/kudu-tool-test.cc@4747
PS4, Line 4747:   ASSERT_TRUE(table->schema().Column(2) == schema_2.Column(1));
nit: ASSERT_EQ?


http://gerrit.cloudera.org:8080/#/c/17851/4/src/kudu/tools/tool_action_table.cc
File src/kudu/tools/tool_action_table.cc:

http://gerrit.cloudera.org:8080/#/c/17851/4/src/kudu/tools/tool_action_table.cc@992
PS4, Line 992: value
Who owns this memory? Is this leaked?



-- 
To view, visit http://gerrit.cloudera.org:8080/17851
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I93e6c2af07e9fa5aa909b4a09b79b7c54401d4e3
Gerrit-Change-Number: 17851
Gerrit-PatchSet: 4
Gerrit-Owner: Anonymous Coward <sh...@gmail.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Bankim Bhavsar <ba...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Mon, 22 Nov 2021 19:02:43 +0000
Gerrit-HasComments: Yes