Csaba Ringhofer has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/16419 )

Change subject: IMPALA-10078: Proper codegen for KuduPartitionExpr
......................................................................


Patch Set 5:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/16419/5/tests/query_test/test_kudu.py
File tests/query_test/test_kudu.py:

http://gerrit.cloudera.org:8080/#/c/16419/5/tests/query_test/test_kudu.py@570
PS5, Line 570:     with self.temp_kudu_table(kudu_client, [INT32], 
num_partitions=3) as kudu_table:
             :       impala_table_name = 
self.get_kudu_table_base_name(kudu_table.name)
             :       props = "TBLPROPERTIES('kudu.table_name'='%s')" % 
kudu_table.name
             :       cursor.execute("CREATE EXTERNAL TABLE %s STORED AS KUDU 
%s" % (
             :           impala_table_name, props))
Do we need the temp_kudu_table + external kudu table pointing to it, couldn't 
we just use a unique database, and create a managed Kudu table in it? The 
cleanup of unique database would handle the dropping of the table.

Note that I am not too familiar with KuduTestSuite, so maybe there is a reason 
why it is done differently here.


http://gerrit.cloudera.org:8080/#/c/16419/5/tests/query_test/test_kudu.py@598
PS5, Line 598:   def extract_kudu_rows_from_profile(profile):
optional: This could be moved to common code, e.g. near 
https://github.com/apache/impala/blob/master/tests/common/test_result_verifier.py#L574

A function that takes two regexps (first to match the section second to match 
the line) and a profile seems a generally useful one.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ifcae34f71b407837e2c5f1b97aa230e490a268df
Gerrit-Change-Number: 16419
Gerrit-PatchSet: 5
Gerrit-Owner: Daniel Becker <daniel.bec...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com>
Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Reviewer: Qifan Chen <qc...@cloudera.com>
Gerrit-Reviewer: Thomas Tauber-Marshall <tmarsh...@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com>
Gerrit-Comment-Date: Mon, 21 Sep 2020 13:19:58 +0000
Gerrit-HasComments: Yes

Reply via email to