[Impala-ASF-CR] IMPALA-3718: Support subset of functional-query for Kudu

["Michael Brown (Code Review)" <ge...@cloudera.org>]

Michael Brown has posted comments on this change.

Change subject: IMPALA-3718: Support subset of functional-query for Kudu
......................................................................


Patch Set 1:

(6 comments)

In addition to the inline comments, I generally disagree on the surface with the use of xfail. It seems like many or all of them should be skips. Any reason you chose xfail?

Also, have you tested this end to end? Some ideas:

1. ./buildall.sh -format -testdata [etc]
2. Private data load job
3. ./buildall.sh -format -snapshot_file snapshot_from_private_job -metastore_snapshot_file metastore_snapshot_from_private_job [etc]

http://gerrit.cloudera.org:8080/#/c/4175/1/testdata/bin/generate-schema-statements.py
File testdata/bin/generate-schema-statements.py:

Line 594:         print "Ignore partitions on Kudu"
Include the db and table name here so we know what we're ignoring.


http://gerrit.cloudera.org:8080/#/c/4175/1/testdata/datasets/functional/functional_schema_template.sql
File testdata/datasets/functional/functional_schema_template.sql:

PS1, Line 598: SELECT row_number() over (order by year, month, id, day),
For my education, was the ordering of "id, day" in the ORDER BY intentional? Why that instead of "day, id"?


PS1, Line 1063: text_comma_backslash_newline
This table is defined as a kudu table in schema_constraints.csv L181, but it doesn't have a CREATE_KUDU section. Does it need one?


http://gerrit.cloudera.org:8080/#/c/4175/1/testdata/datasets/functional/schema_constraints.csv
File testdata/datasets/functional/schema_constraints.csv:

PS1, Line 177: table_name:testtbl, constraint:only, table_format:kudu/none/none
How is it that this constraint was already defined but only just now had a CREATE_KUDU added for it (functional_schema_template.sql, L789 )?


http://gerrit.cloudera.org:8080/#/c/4175/1/testdata/workloads/functional-query/queries/QueryTest/aggregation.test
File testdata/workloads/functional-query/queries/QueryTest/aggregation.test:

Line 836: select min(distinct NULL), max(distinct NULL) from alltypes
Why this change? Thanks.


http://gerrit.cloudera.org:8080/#/c/4175/1/testdata/workloads/functional-query/queries/QueryTest/create_kudu.test
File testdata/workloads/functional-query/queries/QueryTest/create_kudu.test:

PS1, Line 4: drop table if exists managed_kudu;
Fwiw, this won't be needed once the commit lies atop https://gerrit.cloudera.org/#/c/4155/


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

Gerrit-MessageType: comment
Gerrit-Change-Id: Iada88e078352e4462745d9a9a1b5111260d21acc
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Matthew Jacobs <mj...@cloudera.com>
Gerrit-Reviewer: Michael Brown <mi...@cloudera.com>
Gerrit-HasComments: Yes