Tim Armstrong has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9241 )

Change subject: IMPALA-6077: remove Parquet BIT_PACKED def level support
......................................................................


Patch Set 2:

(4 comments)

Thanks for catching my mistake in the tests.

http://gerrit.cloudera.org:8080/#/c/9241/2/testdata/workloads/functional-query/queries/QueryTest/parquet-def-levels.test
File 
testdata/workloads/functional-query/queries/QueryTest/parquet-def-levels.test:

http://gerrit.cloudera.org:8080/#/c/9241/2/testdata/workloads/functional-query/queries/QueryTest/parquet-def-levels.test@55
PS2, Line 55: # IMPALA-6077: unsupported BIT_PACKED encoding fails when 
materializing columns.
            : select count(id), count(tinyint_col), count(smallint_col), 
count(int_col),
            :   count(bigint_col), count(float_col), count(double_col), 
count(date_string_col),
            :   count(string_col), count(timestamp_col), count(year), 
count(month), count(day)
            : from alltypesagg_bitpacked
> This query seems to be the same as the next query, and should not materiali
Done


http://gerrit.cloudera.org:8080/#/c/9241/2/testdata/workloads/functional-query/queries/QueryTest/parquet-def-levels.test@64
PS2, Line 64: materializing
> I am not 100% sure about this, but I think that if a column is not complex,
Yeah that's a good point, I switched it to just selecting the column value. We 
don't currently serve count(colname) from stats but could easily since it only 
depends on the non-null count.

I also made a mistake here forgetting to update a query - one of them is meant 
to be a count(*), which we have always served from stats.


http://gerrit.cloudera.org:8080/#/c/9241/2/testdata/workloads/functional-query/queries/QueryTest/parquet-def-levels.test@65
PS2, Line 65: select count(id), count(tinyint_col), count(smallint_col), 
count(int_col),
            :   count(bigint_col), count(float_col), count(double_col), 
count(date_string_col),
            :   count(string_col), count(timestamp_col), count(year), 
count(month), count(day)
> Is it necessary to list every column here? If one column is enough for the
Done


http://gerrit.cloudera.org:8080/#/c/9241/2/testdata/workloads/functional-query/queries/QueryTest/parquet-def-levels.test@71
PS2, Line 71: ====
I also added a couple of queries to future-proof this, where we could serve 
them from metadata in theory but don't right now.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I12c75b7f162dd7de8e26cf31be142b692e3624ae
Gerrit-Change-Number: 9241
Gerrit-PatchSet: 2
Gerrit-Owner: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com>
Gerrit-Reviewer: Lars Volker <l...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Fri, 09 Feb 2018 16:45:24 +0000
Gerrit-HasComments: Yes

Reply via email to