Quanlong Huang has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/15395 )

Change subject: IMPALA-9484: Full ACID Milestone 1: properly scan files that 
has full ACID schema
......................................................................


Patch Set 10: Code-Review+1

(3 comments)

The other part of the patch looks good to me as well. Just need to verify that 
test_create_table_like_file_orc in test_ddl.py passes in S3 tests since this 
patch adds some data load statements in it.

Feel free to carry on my +1 and Tim's to be +2.

http://gerrit.cloudera.org:8080/#/c/15395/10/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java
File fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java:

http://gerrit.cloudera.org:8080/#/c/15395/10/fe/src/main/java/org/apache/impala/analysis/AlterTableStmt.java@112
PS10, Line 112:     // TODO: remove it or keep it by the end of this change 
request.
Is it the time to remove this TODO?


http://gerrit.cloudera.org:8080/#/c/15395/10/testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-orc.test
File 
testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-orc.test:

http://gerrit.cloudera.org:8080/#/c/15395/10/testdata/workloads/functional-query/queries/QueryTest/create-table-like-file-orc.test@7
PS10, Line 7: as select * from functional_orc_def.decimal_tiny;
Will this introduce the same failure in S3 tests like IMPALA-9345 found? 
Because Yarn and HiveServer2 are not launched in S3 tests (IMPALA-9365). Maybe 
we should skip the test on non-hdfs envs as IMPALA-9345 does. Otherwise, do you 
mind running the S3 tests once before merging this patch?


http://gerrit.cloudera.org:8080/#/c/15395/8/tests/query_test/test_scanners_fuzz.py
File tests/query_test/test_scanners_fuzz.py:

http://gerrit.cloudera.org:8080/#/c/15395/8/tests/query_test/test_scanners_fuzz.py@181
PS8, Line 181:       self.run_stmt_in_hive("insert into %s.%s select * from 
%s.%s" % (fuzz_db,
             :           fuzz_table, src_db, src_table))
> I checked and it seems fine to me. Note that in this CR I changed the data
Thanks for the verification!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2e2afec00c9a5cf87f1d61b5fe52b0085844bcb
Gerrit-Change-Number: 15395
Gerrit-PatchSet: 10
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Norbert Luksa <[email protected]>
Gerrit-Reviewer: Quanlong Huang <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Wed, 01 Apr 2020 03:55:17 +0000
Gerrit-HasComments: Yes

Reply via email to