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 2018/10/12 06:16:29 UTC

[kudu-CR] [tools] Add a describe table tool 

Will Berkeley has uploaded this change for review. ( http://gerrit.cloudera.org:8080/11665


Change subject: [tools] Add a describe table tool
......................................................................

[tools] Add a describe table tool

This adds a very basic tool to describe a table, printing out its
schema, its partition schema, and the current range partitions.

Sample output:

$ kudu table describe localhost:7053 default.loadgen_auto_f6d439ab21e4408486786289297f7db0
Schema [
	primary key (key),
	10:key[int64 NOT NULL],
	11:int_val[int32 NULLABLE],
	12:string_val[string NULLABLE]
]

HASH (key) PARTITIONS 2,
RANGE (key) (
    PARTITION UNBOUNDED
)

I also reorganized the order of `kudu table` commands so they are in
alphabetical order.

Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
---
M src/kudu/client/client.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_table.cc
6 files changed, 245 insertions(+), 18 deletions(-)



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] Add a describe table tool

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

Change subject: [tools] Add a describe table tool
......................................................................


Patch Set 5: Verified+1

Failure looks unrelated.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 20:04:08 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] Add a describe table tool

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has removed Kudu Jenkins from this change.  ( http://gerrit.cloudera.org:8080/11665 )

Change subject: [tools] Add a describe table tool
......................................................................


Removed reviewer Kudu Jenkins.
-- 
To view, visit http://gerrit.cloudera.org:8080/11665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] Add a describe table tool

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

Change subject: [tools] Add a describe table tool
......................................................................


Patch Set 5:

Yeah, we can do that. Though, re: the CREATE TABLE on /table, we went a different direction so that's only what you need to establish an external table over the existing table:

    CREATE EXTERNAL TABLE `<placeholder>` STORED AS KUDU
    TBLPROPERTIES(
        'kudu.table_name' = 
    'default.loadgen_auto_f6d439ab21e4408486786289297f7db0',
        'kudu.master_addresses' = '127.0.0.1:7053,127.0.0.1:7052,127.0.0.1:7051')

Do you prefer the table or SQL syntax? I think I prefer the latter because it is more compact. It is more likely to fit when printed in a terminal window and easier to follow when it wraps.

I don't want to add anything that references or evokes impala specifically, though, like the TBLPROPERTIES or STORED AS KUDU parts. This is meant to be a useful little tool for working directly with Kudu, esp if there's no impala.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 22:13:52 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] Add a describe table tool

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

Change subject: [tools] Add a describe table tool
......................................................................


Patch Set 5: -Verified

I compromised between making this compact, so it's easier to read, and having it be complete. I want it to be a quick tool to get the fundamental info about a table, suitable to be copy-pasted or even sent in a screenshot (since this sometimes happens). Tables are lousy when copy-pasted or stuffed into too few columns, and a lot of space is wasted. Including everything in a SQL-ish SHOW CREATE TABLE format is too busy. A lot of the space is info that will pretty much never be interesting.


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Sat, 13 Oct 2018 10:02:12 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] Add a describe table tool

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

Change subject: [tools] Add a describe table tool
......................................................................


Patch Set 9:

Forgot to mention I added the changes in a new review: http://gerrit.cloudera.org:8080/11736


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 9
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:32:58 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] Add a describe table tool

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

Change subject: [tools] Add a describe table tool
......................................................................


Patch Set 9:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/11665/9/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/11665/9/src/kudu/client/client.h@593
PS9, Line 593:   friend Status tools::ListPartitions(
             :       const client::sp::shared_ptr<client::KuduTable>& table,
             :       std::vector<Partition>* partitions);
> This is no longer the preferred way to add private APIs. Instead, take the 
Done


http://gerrit.cloudera.org:8080/#/c/11665/9/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

http://gerrit.cloudera.org:8080/#/c/11665/9/src/kudu/tools/kudu-admin-test.cc@1379
PS9, Line 1379:       "(\n"
              :       "    key INT32 NOT NULL,\n"
              :       "    int_val INT32 NOT NULL,\n"
              :       "    string_val STRING NULLABLE,\n"
              :       "    PRIMARY KEY (key)\n"
              :       ")\n"
              :       "RANGE (key) (\n"
              :       "    PARTITION UNBOUNDED"
              :       "\n"
              :       ")\n"
              :       "REPLICAS 1");
> Could avoid the excess double quotes (and simplify the formatting somewhat)
Somebody suggested this on the original review, IIRC.

The issue is you can't keep the indentation if you use a raw literal, so it needs to be on one line or not indented.


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

http://gerrit.cloudera.org:8080/#/c/11665/9/src/kudu/tools/tool_action_common.cc@861
PS9, Line 861: // The strategy for retrieving the partitions from the metacache is adapted
             : // from KuduScanTokenBuilder::Data::Build.
> Would it be possible to share code between the two methods? Seems like you 
I experimented with this and it just seemed a lot harder to follow, particularly the more complex and lengthier logic in the scan token builder.


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

http://gerrit.cloudera.org:8080/#/c/11665/9/src/kudu/tools/tool_action_table.cc@147
PS9, Line 147:   cout << "TABLE " << table_name << " " << schema.ToString() << endl;
> Do you really want to print this if ListPartitions() fails?
Yup, it's still useful info and not contingent on the partitions to make sense.


http://gerrit.cloudera.org:8080/#/c/11665/9/src/kudu/tools/tool_action_table.cc@168
PS9, Line 168:     partition_strs.push_back(std::move(range_partition_str));
> Nit: emplace_back in new code.
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 9
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot (241)
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 19 Oct 2018 17:30:50 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tools] Add a describe table tool

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

Change subject: [tools] Add a describe table tool
......................................................................


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/11665/1/src/kudu/tools/tool_action_common.h
File src/kudu/tools/tool_action_common.h:

http://gerrit.cloudera.org:8080/#/c/11665/1/src/kudu/tools/tool_action_common.h@41
PS1, Line 41: class faststring;
> warning: invalid case style for class 'faststring' [readability-identifier-
Too bad, that's the name.


http://gerrit.cloudera.org:8080/#/c/11665/1/src/kudu/tools/tool_action_common.cc
File src/kudu/tools/tool_action_common.cc:

http://gerrit.cloudera.org:8080/#/c/11665/1/src/kudu/tools/tool_action_common.cc@39
PS1, Line 39: #include "kudu/util/async_util.h"
> warning: #includes are not sorted properly [llvm-include-order]
Done


http://gerrit.cloudera.org:8080/#/c/11665/1/src/kudu/tools/tool_action_common.cc@137
PS1, Line 137: using client::KuduTablet;
> warning: using decl 'KuduTablet' is unused [misc-unused-using-decls]
Done


http://gerrit.cloudera.org:8080/#/c/11665/1/src/kudu/tools/tool_action_common.cc@138
PS1, Line 138: using client::KuduTabletServer;
> warning: using decl 'KuduTabletServer' is unused [misc-unused-using-decls]
Done


http://gerrit.cloudera.org:8080/#/c/11665/1/src/kudu/tools/tool_action_common.cc@139
PS1, Line 139: using consensus::ConsensusServiceProxy;
> warning: using decl 'ConsensusServiceProxy' is unused [misc-unused-using-de
It is used, Mr. Tidy Bot.


http://gerrit.cloudera.org:8080/#/c/11665/1/src/kudu/tools/tool_action_common.cc@889
PS1, Line 889:     } else if (!s.ok()) {
> warning: do not use 'else' after 'break' [readability-else-after-return]
Done


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

http://gerrit.cloudera.org:8080/#/c/11665/1/src/kudu/tools/tool_action_table.cc@167
PS1, Line 167:     partition_strs.push_back(std::move(range_partition_str));
> warning: std::move of the const variable 'range_partition_str' has no effec
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 1
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 10:26:32 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tools] Add a describe table tool

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

Change subject: [tools] Add a describe table tool
......................................................................

[tools] Add a describe table tool

This adds a very basic tool to describe a table, printing out its
schema, its partition schema, the current range partitions, and the
replication factor. It eschews completeness for brevity, leaving out,
for example, column storage attributes, since they clutter the output
and make it hard to read. My use case for this tool is as an easy way a
user can gather the fundamental info about a table and show it to
someone else. It should be easy to copy and paste, usually fit on one
line in a terminal window, and not depend on fancy formatting or a
fixed-width font to display correctly.

Sample output:

$ kudu table describe localhost:7053 default.loadgen_auto_f6d439ab21e4408486786289297f7db0
TABLE default.loadgen_auto_f6d439ab21e4408486786289297f7db0 (
    key INT64 NOT NULL,
    int_val INT32 NULLABLE,
    string_val STRING NULLABLE,
    PRIMARY KEY (key)
)
HASH (key) PARTITIONS 2,
RANGE (key) (
    PARTITION UNBOUNDED
)
REPLICAS 1

Getting this "SQL-ish" look required changing how schemas and column
schemas are stringified, so there are some knock-on changes in various
tests as well.

I also reorganized the order of `kudu table` commands so they are in
alphabetical order.

Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Reviewed-on: http://gerrit.cloudera.org:8080/11665
Tested-by: Kudu Jenkins
Reviewed-by: Mike Percy <mp...@apache.org>
---
M src/kudu/client/client-test.cc
M src/kudu/client/client-unittest.cc
M src/kudu/client/client.h
M src/kudu/client/schema.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_operations-test.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/schema.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_table.cc
M src/kudu/tserver/tablet_server-test.cc
17 files changed, 375 insertions(+), 130 deletions(-)

Approvals:
  Kudu Jenkins: Verified
  Mike Percy: Looks good to me, approved

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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 9
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] Add a describe table tool

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke, 

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

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

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

Change subject: [tools] Add a describe table tool
......................................................................

[tools] Add a describe table tool

This adds a very basic tool to describe a table, printing out its
schema, its partition schema, and the current range partitions.

Sample output:

$ kudu table describe localhost:7053 default.loadgen_auto_f6d439ab21e4408486786289297f7db0
Schema [
	primary key (key),
	key[int64 NOT NULL],
	int_val[int32 NULLABLE],
	string_val[string NULLABLE]
]

HASH (key) PARTITIONS 2,
RANGE (key) (
    PARTITION UNBOUNDED
)

I also reorganized the order of `kudu table` commands so they are in
alphabetical order.

Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
---
M src/kudu/client/client.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_table.cc
6 files changed, 251 insertions(+), 25 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 4
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] Add a describe table tool

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

Change subject: [tools] Add a describe table tool
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11665/7/src/kudu/common/schema.cc
File src/kudu/common/schema.cc:

http://gerrit.cloudera.org:8080/#/c/11665/7/src/kudu/common/schema.cc@77
PS7, Line 77: $1
$0



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 16 Oct 2018 00:00:35 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tools] Add a describe table tool

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has removed a vote on this change.

Change subject: [tools] Add a describe table tool
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/11665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] Add a describe table tool

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

Change subject: [tools] Add a describe table tool
......................................................................


Patch Set 7: Verified+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Sat, 13 Oct 2018 18:42:21 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] Add a describe table tool

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

Change subject: [tools] Add a describe table tool
......................................................................


Patch Set 5: Code-Review+1


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 23:00:56 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] Add a describe table tool

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

Change subject: [tools] Add a describe table tool
......................................................................


Patch Set 8: Code-Review+2


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 8
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 16 Oct 2018 05:13:06 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] Add a describe table tool

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Will Berkeley has removed Kudu Jenkins from this change.  ( http://gerrit.cloudera.org:8080/11665 )

Change subject: [tools] Add a describe table tool
......................................................................


Removed reviewer Kudu Jenkins with the following votes:

* Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/11665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: deleteReviewer
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] Add a describe table tool

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Alexey Serbin, Attila Bukor, Andrew Wong, Adar Dembo, Grant Henke, 

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

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

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

Change subject: [tools] Add a describe table tool
......................................................................

[tools] Add a describe table tool

This adds a very basic tool to describe a table, printing out its
schema, its partition schema, the current range partitions, and the
replication factor. It eschews completeness for brevity, leaving out,
for example, column storage attributes, since they clutter the output
and make it hard to read. My use case for this tool is as an easy way a
user can gather the fundamental info about a table and show it to
someone else. It should be easy to copy and paste, usually fit on one
line in a terminal window, and not depend on fancy formatting or a
fixed-width font to display correctly.

Sample output:

$ kudu table describe localhost:7053 default.loadgen_auto_f6d439ab21e4408486786289297f7db0
TABLE default.loadgen_auto_f6d439ab21e4408486786289297f7db0 (
    key INT64 NOT NULL,
    int_val INT32 NULLABLE,
    string_val STRING NULLABLE,
    PRIMARY KEY (key)
)
HASH (key) PARTITIONS 2,
RANGE (key) (
    PARTITION UNBOUNDED
)
REPLICAS 1

Getting this "SQL-ish" look required changing how schemas and column
schemas are stringified, so there are some knock-on changes in various
tests as well.

I also reorganized the order of `kudu table` commands so they are in
alphabetical order.

Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
---
M src/kudu/client/client-unittest.cc
M src/kudu/client/client.h
M src/kudu/client/schema.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/schema.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_table.cc
10 files changed, 348 insertions(+), 103 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 6
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] Add a describe table tool

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Alexey Serbin, Attila Bukor, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke, 

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

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

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

Change subject: [tools] Add a describe table tool
......................................................................

[tools] Add a describe table tool

This adds a very basic tool to describe a table, printing out its
schema, its partition schema, the current range partitions, and the
replication factor. It eschews completeness for brevity, leaving out,
for example, column storage attributes, since they clutter the output
and make it hard to read. My use case for this tool is as an easy way a
user can gather the fundamental info about a table and show it to
someone else. It should be easy to copy and paste, usually fit on one
line in a terminal window, and not depend on fancy formatting or a
fixed-width font to display correctly.

Sample output:

$ kudu table describe localhost:7053 default.loadgen_auto_f6d439ab21e4408486786289297f7db0
TABLE default.loadgen_auto_f6d439ab21e4408486786289297f7db0 (
    key INT64 NOT NULL,
    int_val INT32 NULLABLE,
    string_val STRING NULLABLE,
    PRIMARY KEY (key)
)
HASH (key) PARTITIONS 2,
RANGE (key) (
    PARTITION UNBOUNDED
)
REPLICAS 1

Getting this "SQL-ish" look required changing how schemas and column
schemas are stringified, so there are some knock-on changes in various
tests as well.

I also reorganized the order of `kudu table` commands so they are in
alphabetical order.

Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
---
M src/kudu/client/client-test.cc
M src/kudu/client/client-unittest.cc
M src/kudu/client/client.h
M src/kudu/client/schema.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_operations-test.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/schema.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_table.cc
M src/kudu/tserver/tablet_server-test.cc
17 files changed, 375 insertions(+), 130 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/65/11665/7
-- 
To view, visit http://gerrit.cloudera.org:8080/11665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] Add a describe table tool

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Alexey Serbin, Kudu Jenkins, Andrew Wong, Adar Dembo, Grant Henke, 

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

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

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

Change subject: [tools] Add a describe table tool
......................................................................

[tools] Add a describe table tool

This adds a very basic tool to describe a table, printing out its
schema, its partition schema, and the current range partitions.

Sample output:

$ kudu table describe localhost:7053 default.loadgen_auto_f6d439ab21e4408486786289297f7db0
Schema [
	primary key (key),
	10:key[int64 NOT NULL],
	11:int_val[int32 NULLABLE],
	12:string_val[string NULLABLE]
]

HASH (key) PARTITIONS 2,
RANGE (key) (
    PARTITION UNBOUNDED
)

I also reorganized the order of `kudu table` commands so they are in
alphabetical order.

Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
---
M src/kudu/client/client.h
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_table.cc
6 files changed, 251 insertions(+), 25 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 2
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot

[kudu-CR] [tools] Add a describe table tool

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

Change subject: [tools] Add a describe table tool
......................................................................


Patch Set 5:

Would it be possible to output Impala-compatible syntax ala /table in the Master UI? BTW can we fix that thing to include all this new good stuff?

Well, that is equivalent to "show create table" but I think it's pretty legit, technically describe table might only show schema stuff.

From impala-shell:

> describe mpercy_test8;
Query: describe mpercy_test8
+------+--------+---------+-------------+----------+---------------+---------------+---------------------+------------+
| name | type   | comment | primary_key | nullable | default_value | encoding      | compression         | block_size |
+------+--------+---------+-------------+----------+---------------+---------------+---------------------+------------+
| f0   | bigint |         | true        | false    |               | AUTO_ENCODING | DEFAULT_COMPRESSION | 0          |
| f1   | string |         | false       | true     |               | AUTO_ENCODING | DEFAULT_COMPRESSION | 0          |
| f2   | string |         | false       | true     |               | AUTO_ENCODING | DEFAULT_COMPRESSION | 0          |
| f3   | string |         | false       | true     |               | AUTO_ENCODING | DEFAULT_COMPRESSION | 0          |
| f4   | string |         | false       | true     |               | AUTO_ENCODING | DEFAULT_COMPRESSION | 0          |
| f5   | string |         | false       | true     |               | AUTO_ENCODING | DEFAULT_COMPRESSION | 0          |
| f6   | string |         | false       | true     |               | AUTO_ENCODING | DEFAULT_COMPRESSION | 0          |
| f7   | string |         | false       | true     |               | AUTO_ENCODING | DEFAULT_COMPRESSION | 0          |
| f8   | string |         | false       | true     |               | AUTO_ENCODING | DEFAULT_COMPRESSION | 0          |
| f9   | string |         | false       | true     |               | AUTO_ENCODING | DEFAULT_COMPRESSION | 0          |
+------+--------+---------+-------------+----------+---------------+---------------+---------------------+------------+
Fetched 10 row(s) in 0.04s

> show create table mpercy_test8;
Query: show create table mpercy_test8
+-----------------------------------------------------------------------------------------------------+
| result                                                                                              |
+-----------------------------------------------------------------------------------------------------+
| CREATE TABLE default.mpercy_test8 (                                                                 |
|   f0 BIGINT NOT NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,                        |
|   f1 STRING NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,                            |
|   f2 STRING NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,                            |
|   f3 STRING NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,                            |
|   f4 STRING NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,                            |
|   f5 STRING NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,                            |
|   f6 STRING NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,                            |
|   f7 STRING NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,                            |
|   f8 STRING NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,                            |
|   f9 STRING NULL ENCODING AUTO_ENCODING COMPRESSION DEFAULT_COMPRESSION,                            |
|   PRIMARY KEY (f0)                                                                                  |
| )                                                                                                   |
| PARTITION BY HASH (f0) PARTITIONS 450                                                               |
| STORED AS KUDU                                                                                      |
| TBLPROPERTIES ('kudu.master_addresses'='vc1320.halxg.cloudera.com', 'kudu.num_tablet_replicas'='1') |
+-----------------------------------------------------------------------------------------------------+
Fetched 1 row(s) in 0.04s


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 5
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Fri, 12 Oct 2018 21:12:52 +0000
Gerrit-HasComments: No

[kudu-CR] [tools] Add a describe table tool

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

Change subject: [tools] Add a describe table tool
......................................................................


Patch Set 7:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/11665/7/src/kudu/common/schema.cc
File src/kudu/common/schema.cc:

http://gerrit.cloudera.org:8080/#/c/11665/7/src/kudu/common/schema.cc@77
PS7, Line 77: $1
> $0
Done



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 7
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Tue, 16 Oct 2018 01:38:07 +0000
Gerrit-HasComments: Yes

[kudu-CR] [tools] Add a describe table tool

Posted by "Will Berkeley (Code Review)" <ge...@cloudera.org>.
Hello Tidy Bot, Mike Percy, Alexey Serbin, Attila Bukor, Andrew Wong, Adar Dembo, Grant Henke, 

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

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

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

Change subject: [tools] Add a describe table tool
......................................................................

[tools] Add a describe table tool

This adds a very basic tool to describe a table, printing out its
schema, its partition schema, the current range partitions, and the
replication factor. It eschews completeness for brevity, leaving out,
for example, column storage attributes, since they clutter the output
and make it hard to read. My use case for this tool is as an easy way a
user can gather the fundamental info about a table and show it to
someone else. It should be easy to copy and paste, usually fit on one
line in a terminal window, and not depend on fancy formatting or a
fixed-width font to display correctly.

Sample output:

$ kudu table describe localhost:7053 default.loadgen_auto_f6d439ab21e4408486786289297f7db0
TABLE default.loadgen_auto_f6d439ab21e4408486786289297f7db0 (
    key INT64 NOT NULL,
    int_val INT32 NULLABLE,
    string_val STRING NULLABLE,
    PRIMARY KEY (key)
)
HASH (key) PARTITIONS 2,
RANGE (key) (
    PARTITION UNBOUNDED
)
REPLICAS 1

Getting this "SQL-ish" look required changing how schemas and column
schemas are stringified, so there are some knock-on changes in various
tests as well.

I also reorganized the order of `kudu table` commands so they are in
alphabetical order.

Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
---
M src/kudu/client/client-test.cc
M src/kudu/client/client-unittest.cc
M src/kudu/client/client.h
M src/kudu/client/schema.cc
M src/kudu/common/partial_row-test.cc
M src/kudu/common/row_changelist-test.cc
M src/kudu/common/row_operations-test.cc
M src/kudu/common/schema-test.cc
M src/kudu/common/schema.cc
M src/kudu/tablet/tablet_bootstrap-test.cc
M src/kudu/tools/CMakeLists.txt
M src/kudu/tools/kudu-admin-test.cc
M src/kudu/tools/kudu-tool-test.cc
M src/kudu/tools/tool_action_common.cc
M src/kudu/tools/tool_action_common.h
M src/kudu/tools/tool_action_table.cc
M src/kudu/tserver/tablet_server-test.cc
17 files changed, 375 insertions(+), 130 deletions(-)


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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 8
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>

[kudu-CR] [tools] Add a describe table tool

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

Change subject: [tools] Add a describe table tool
......................................................................


Patch Set 9:

(5 comments)

http://gerrit.cloudera.org:8080/#/c/11665/9/src/kudu/client/client.h
File src/kudu/client/client.h:

http://gerrit.cloudera.org:8080/#/c/11665/9/src/kudu/client/client.h@593
PS9, Line 593:   friend Status tools::ListPartitions(
             :       const client::sp::shared_ptr<client::KuduTable>& table,
             :       std::vector<Partition>* partitions);
This is no longer the preferred way to add private APIs. Instead, take the example of GetHiveMetastoreUris(): actually add a public function to KuduClient (or KuduTable, or whatever) but annotate it with KUDU_NO_EXPORT.


http://gerrit.cloudera.org:8080/#/c/11665/9/src/kudu/tools/kudu-admin-test.cc
File src/kudu/tools/kudu-admin-test.cc:

http://gerrit.cloudera.org:8080/#/c/11665/9/src/kudu/tools/kudu-admin-test.cc@1379
PS9, Line 1379:       "(\n"
              :       "    key INT32 NOT NULL,\n"
              :       "    int_val INT32 NOT NULL,\n"
              :       "    string_val STRING NULLABLE,\n"
              :       "    PRIMARY KEY (key)\n"
              :       ")\n"
              :       "RANGE (key) (\n"
              :       "    PARTITION UNBOUNDED"
              :       "\n"
              :       ")\n"
              :       "REPLICAS 1");
Could avoid the excess double quotes (and simplify the formatting somewhat) with raw string literals. See https://en.cppreference.com/w/cpp/language/string_literal.

Below too.


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

http://gerrit.cloudera.org:8080/#/c/11665/9/src/kudu/tools/tool_action_common.cc@861
PS9, Line 861: // The strategy for retrieving the partitions from the metacache is adapted
             : // from KuduScanTokenBuilder::Data::Build.
Would it be possible to share code between the two methods? Seems like you could encapsulate the bulk of the logic in a "for each partition" method that takes an std::function and calls it in the while loop after LookupTabletByKey().


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

http://gerrit.cloudera.org:8080/#/c/11665/9/src/kudu/tools/tool_action_table.cc@147
PS9, Line 147:   cout << "TABLE " << table_name << " " << schema.ToString() << endl;
Do you really want to print this if ListPartitions() fails?


http://gerrit.cloudera.org:8080/#/c/11665/9/src/kudu/tools/tool_action_table.cc@168
PS9, Line 168:     partition_strs.push_back(std::move(range_partition_str));
Nit: emplace_back in new code.



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

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ib02f2a94cf11d0e60b32cd85be920ca9b99dd977
Gerrit-Change-Number: 11665
Gerrit-PatchSet: 9
Gerrit-Owner: Will Berkeley <wd...@gmail.com>
Gerrit-Reviewer: Adar Dembo <ad...@cloudera.com>
Gerrit-Reviewer: Alexey Serbin <as...@cloudera.com>
Gerrit-Reviewer: Andrew Wong <aw...@cloudera.com>
Gerrit-Reviewer: Attila Bukor <ab...@apache.org>
Gerrit-Reviewer: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Kudu Jenkins
Gerrit-Reviewer: Mike Percy <mp...@apache.org>
Gerrit-Reviewer: Tidy Bot
Gerrit-Reviewer: Will Berkeley <wd...@gmail.com>
Gerrit-Comment-Date: Wed, 17 Oct 2018 16:42:01 +0000
Gerrit-HasComments: Yes