You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Tamas Mate (Code Review)" <ge...@cloudera.org> on 2020/02/10 18:19:16 UTC

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Tamas Mate has uploaded this change for review. ( http://gerrit.cloudera.org:8080/15199


Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................

IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

This change modifies the output of the SHOW STABLE STATS for Kudu
tables:
 - #Rows column is moved after partition key details
 - The values of #Rows column changed from '-1' to 'N/A'
 - 'Total' row is appended after parititions similar to HdfsTable

Example output can be seen in the doc changes.

Testing:
kudu_stats.test is modified to verify the new style and 'Total" row.

Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
---
M docs/topics/impala_compute_stats.xml
M docs/topics/impala_show.xml
M fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test
4 files changed, 70 insertions(+), 41 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/15199/2
-- 
To view, visit http://gerrit.cloudera.org:8080/15199
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................


Patch Set 4:

Also something I happened to notice that you could fix in this change - in fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java line 102, there's an AnaysisException being thrown and the text of the exception is slightly wrong. It should say that SHOW PARTITIONS "must target an HDFS or Kudu table"


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Feb 2020 18:37:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................

IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

This change modifies the output of the SHOW TABLE STATS and SHOW
PARTITIONS for Kudu tables.
 - PARTITIONS: the #Row column has been removed
 - TABLE STATS: instead of showing partition informations it returns a
 resultset similar to HDFS table stats, #Rows, #Partitions, Size, Format
 and Location

Example outputs can be seen in the doc changes.

Testing:
* kudu_stats.test is modified to verify the new result set
* kudu_partition_ddl.test is modified to verify the new partitions style
* Updated unit test with the new error message

Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Reviewed-on: http://gerrit.cloudera.org:8080/15199
Reviewed-by: Thomas Tauber-Marshall <tm...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M docs/topics/impala_compute_stats.xml
M docs/topics/impala_show.xml
M fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_partition_ddl.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test
8 files changed, 190 insertions(+), 149 deletions(-)

Approvals:
  Thomas Tauber-Marshall: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 9
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................


Patch Set 8:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5487/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 8
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 18 Mar 2020 13:04:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................


Patch Set 2:

Yeah, its definitely pretty weird to me that "show table stats" shows partition information for Kudu tables but not for HDFS tables. Seems like that's what "show partitions" is for, and "show table stats" should be for table-level info.

So I would say - for "show table stats" only show stuff that's table-level and that we have info for, so definitely "# rows", probably a "format -> KUDU" column like how HDFS has "format->TEXT/PARQUET/whatever", and maybe stuff like "# partitions" or "size" if those things are available.

And then for "show partitions" don't add the "total # rows", just exclude the "# rows" column entirely.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 18:21:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

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

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................


Patch Set 3:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15199/3/docs/topics/impala_show.xml
File docs/topics/impala_show.xml:

http://gerrit.cloudera.org:8080/#/c/15199/3/docs/topics/impala_show.xml@979
PS3, Line 979: +-------------+-------+
             : | Stat        | Value |
             : +-------------+-------+
             : | Format      | KUDU  |
             : | #Rows       | 2000  |
             : | #Partitions | 10    |
             : +-------------+-------+
we should be consist with how stats are displayed for HDFS tables:

 show table stats tpch.lineitem;
 +---------+--------+----------+--------------+-------------------+--------+-------------------+-----------------------------------------------------+
 | #Rows   | #Files | Size     | Bytes Cached | Cache Replication | Format | Incremental stats | Location                                            
 |
 +---------+--------+----------+--------------+-------------------+--------+-------------------+-----------------------------------------------------+
 | 6001215 | 1      | 718.94MB | NOT CACHED   | NOT CACHED        | TEXT   | false             | hdfs://localhost:20500/test-warehouse/tpch.lineitem |
 +---------+--------+----------+--------------+-------------------+--------+-------------------+-----------------------------------------------------+

So the rows and columns should be inverted. It should look like this:

 +-------------+-------------+
 | #Rows       | #Partitions |
 +-------------+-------------+
 | 2000        | 10          |
 +-------------+-------------+


http://gerrit.cloudera.org:8080/#/c/15199/3/fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java
File fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java:

http://gerrit.cloudera.org:8080/#/c/15199/3/fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java@245
PS3, Line 245: 
nit: extra newline



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 25 Feb 2020 19:28:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

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

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................


Patch Set 2:

Hi Sahil, thank you for looking into this and adding Thomas.

You mean removing all the columns and just returning the total number of rows?


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Feb 2020 15:54:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................

IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

This change modifies the output of the SHOW TABLE STATS and SHOW
PARTITIONS for Kudu tables.
 - PARTITIONS: the #Row column has been removed
 - TABLE STATS: instead of showing partition informations it returns a
 resultset similar to HDFS table stats, #Rows, #Partitions, Size, Format
 and Location

Example outputs can be seen in the doc changes.

Testing:
* kudu_stats.test is modified to verify the new result set
* kudu_partition_ddl.test is modified to verify the new partitions style
* Updated unit test with the new error message

Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
---
M docs/topics/impala_compute_stats.xml
M docs/topics/impala_show.xml
M fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeDDLTest.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_partition_ddl.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test
8 files changed, 190 insertions(+), 149 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/15199/8
-- 
To view, visit http://gerrit.cloudera.org:8080/15199
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 8
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

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

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................


Patch Set 2:

IMO it would be better to format the output of 'show table stats [impala-kudu-table-name]' as follows:

+-----------------+
| # Rows                 |
+-----------------+
| [total-num-rows] |
+-----------------+

Having the partition information with 'NA' can be confusing, it makes it seem like there is a bug in Impala or Kudu that is causing the stats to not be available. Plus it doesn't add any extra information.

Added Thomas to the review to see what he thinks as well.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 2
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Thu, 13 Feb 2020 17:45:34 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................


Patch Set 4:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/15199/4//COMMIT_MSG@9
PS4, Line 9: S
typo


http://gerrit.cloudera.org:8080/#/c/15199/4/docs/topics/impala_show.xml
File docs/topics/impala_show.xml:

http://gerrit.cloudera.org:8080/#/c/15199/4/docs/topics/impala_show.xml@972
PS4, Line 972: file format
maybe remove this since there is  a "format" column in the new output


http://gerrit.cloudera.org:8080/#/c/15199/4/docs/topics/impala_show.xml@1002
PS4, Line 1002: | -1    | 1      | 370.45MB | NOT CACHED   | TEXT   | false             |
Not your change obviously but I guess the hdfs examples in this file are mostly wrong, both here (eg. Locations column isn't shown) and for 'show partitions' below. Can you file a JIRA for that?


http://gerrit.cloudera.org:8080/#/c/15199/4/fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java
File fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java:

http://gerrit.cloudera.org:8080/#/c/15199/4/fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java@143
PS4, Line 143:         org.apache.kudu.client.KuduTable kuduTable =
Looks like we might also be able to get the size of the table, i.e. with kuduTable.getTableStatistics().getOnDiskSize(). Might be nice to add since the HDFS output has "Size" shown


http://gerrit.cloudera.org:8080/#/c/15199/4/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:

http://gerrit.cloudera.org:8080/#/c/15199/4/fe/src/main/java/org/apache/impala/service/Frontend.java@1188
PS4, Line 1188:         return FeKuduTable.Utils.getTableStats((FeKuduTable) table);
maybe add a Preconditions.checkState(op == TShowStatsOp.TABLE)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Feb 2020 18:24:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................


Patch Set 6:

(4 comments)

Patch is looking good, just a few more minor things and this can go in

http://gerrit.cloudera.org:8080/#/c/15199/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15199/6//COMMIT_MSG@9
PS6, Line 9: S
typo


http://gerrit.cloudera.org:8080/#/c/15199/6/docs/topics/impala_show.xml
File docs/topics/impala_show.xml:

http://gerrit.cloudera.org:8080/#/c/15199/6/docs/topics/impala_show.xml@987
PS6, Line 987:         Impala only computes the number of rows for the whole Kudu table, partition level
Maybe move this down with the 'show partitions' example below


http://gerrit.cloudera.org:8080/#/c/15199/6/fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java:

http://gerrit.cloudera.org:8080/#/c/15199/6/fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java@102
PS6, Line 102: " must target an HDFS or Kudu "
             :             + "table: "
nit: cleaner to just move the whole string to the next line instead of breaking it up into two strings


http://gerrit.cloudera.org:8080/#/c/15199/6/fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java
File fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java:

http://gerrit.cloudera.org:8080/#/c/15199/6/fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java@136
PS6, Line 136: STRING
Looks like hdfs 'show stats' uses BIGINT for #Rows, lets be consistent with that, and the same for "#Partitions".



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 16 Mar 2020 18:18:26 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5481/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Mar 2020 06:53:52 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................

IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

This change modifies the output of the SHOW TABLE STATS and SHOW
PARTITIONS for Kudu tables.
 - PARTITIONS: the #Row column has been removed
 - TABLE STATS: instead of showing partition informations it returns a
 resultset similar to HDFS table stats, #Rows, #Partitions, Size, Format
 and Location

Example outputs can be seen in the doc changes.

Testing:
kudu_stats.test is modified to verify the new result set
kudu_partition_ddl.test is modified to verify the new partitions style

Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
---
M docs/topics/impala_compute_stats.xml
M docs/topics/impala_show.xml
M fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_partition_ddl.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test
7 files changed, 189 insertions(+), 148 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/15199/7
-- 
To view, visit http://gerrit.cloudera.org:8080/15199
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................

IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

This change modifies the output of the SHOW STABLE STATS and SHOW
PARTITIONS for Kudu tables.
 - PARTITIONS: the #Row column has been removed
 - TABLE STATS: instead of showing partition informations it returns a
brief statistics resultset: #Rows and #Partitions

Example outputs can be seen in the doc changes.

Testing:
kudu_stats.test is modified to verify the new result set
kudu_partition_ddl.test is modified to verify the new partitions style

Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
---
M docs/topics/impala_compute_stats.xml
M docs/topics/impala_show.xml
M fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_partition_ddl.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test
6 files changed, 185 insertions(+), 142 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/15199/3
-- 
To view, visit http://gerrit.cloudera.org:8080/15199
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 3
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

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

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................


Patch Set 6:

(4 comments)

Thank you for the detailed review. Addressed the comments, let me know if I should add anything else.

http://gerrit.cloudera.org:8080/#/c/15199/6//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/15199/6//COMMIT_MSG@9
PS6, Line 9: S
> typo
Done


http://gerrit.cloudera.org:8080/#/c/15199/6/docs/topics/impala_show.xml
File docs/topics/impala_show.xml:

http://gerrit.cloudera.org:8080/#/c/15199/6/docs/topics/impala_show.xml@987
PS6, Line 987:         Impala only computes the number of rows for the whole Kudu table, partition level
> Maybe move this down with the 'show partitions' example below
Done


http://gerrit.cloudera.org:8080/#/c/15199/6/fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
File fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java:

http://gerrit.cloudera.org:8080/#/c/15199/6/fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java@102
PS6, Line 102: " must target an HDFS or Kudu "
             :             + "table: "
> nit: cleaner to just move the whole string to the next line instead of brea
Done


http://gerrit.cloudera.org:8080/#/c/15199/6/fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java
File fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java:

http://gerrit.cloudera.org:8080/#/c/15199/6/fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java@136
PS6, Line 136: STRING
> Looks like hdfs 'show stats' uses BIGINT for #Rows, lets be consistent with
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Mon, 16 Mar 2020 21:11:53 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................

IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

This change modifies the output of the SHOW STABLE STATS and SHOW
PARTITIONS for Kudu tables.
 - PARTITIONS: the #Row column has been removed
 - TABLE STATS: instead of showing partition informations it returns a
brief statistics resultset: #Rows and #Partitions

Example outputs can be seen in the doc changes.

Testing:
kudu_stats.test is modified to verify the new result set
kudu_partition_ddl.test is modified to verify the new partitions style

Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
---
M docs/topics/impala_compute_stats.xml
M docs/topics/impala_show.xml
M fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_partition_ddl.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test
6 files changed, 180 insertions(+), 142 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/15199/4
-- 
To view, visit http://gerrit.cloudera.org:8080/15199
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 4
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................


Patch Set 8: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 8
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 18 Mar 2020 17:59:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Thomas Tauber-Marshall (Code Review)" <ge...@cloudera.org>.
Thomas Tauber-Marshall has posted comments on this change. ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................


Patch Set 8: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 8
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Wed, 18 Mar 2020 16:53:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Tamas Mate (Code Review)" <ge...@cloudera.org>.
Tamas Mate has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................

IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

This change modifies the output of the SHOW STABLE STATS and SHOW
PARTITIONS for Kudu tables.
 - PARTITIONS: the #Row column has been removed
 - TABLE STATS: instead of showing partition informations it returns a
 resultset similar to HDFS table stats, #Rows, #Partitions, Size, Format
 and Location

Example outputs can be seen in the doc changes.

Testing:
kudu_stats.test is modified to verify the new result set
kudu_partition_ddl.test is modified to verify the new partitions style

Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
---
M docs/topics/impala_compute_stats.xml
M docs/topics/impala_show.xml
M fe/src/main/java/org/apache/impala/analysis/ShowStatsStmt.java
M fe/src/main/java/org/apache/impala/catalog/FeKuduTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M testdata/workloads/functional-query/queries/QueryTest/kudu_partition_ddl.test
M testdata/workloads/functional-query/queries/QueryTest/kudu_stats.test
7 files changed, 186 insertions(+), 145 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/99/15199/6
-- 
To view, visit http://gerrit.cloudera.org:8080/15199
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 6
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

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

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................


Patch Set 7:

Missed to update the AnalysisException's message in the unit tests, will get back with a new patch soon.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Mar 2020 12:50:55 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/15199 )

Change subject: IMPALA-5308: Resolve confusing Kudu SHOW TABLE STATS output
......................................................................


Patch Set 7: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/5481/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ice4b8df65f0a53fe14b8fbe35d82c9887ab9a041
Gerrit-Change-Number: 15199
Gerrit-PatchSet: 7
Gerrit-Owner: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Sahil Takiar <st...@cloudera.com>
Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tm...@cloudera.com>
Gerrit-Comment-Date: Tue, 17 Mar 2020 11:53:10 +0000
Gerrit-HasComments: No