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

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


Patch Set 7:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/15395/8/be/src/exec/orc-metadata-utils.cc
File be/src/exec/orc-metadata-utils.cc:

http://gerrit.cloudera.org:8080/#/c/15395/8/be/src/exec/orc-metadata-utils.cc@87
PS8, Line 87:   DCHECK(ValidateFullAcidFileSchema().ok()); // Should have 
already been validated.
It would be nice if this printed the validation error. Maybe we need a 
DCHECK_OK macro, like EXPECT_OK?


http://gerrit.cloudera.org:8080/#/c/15395/5/testdata/workloads/functional-query/queries/QueryTest/describe-path.test
File testdata/workloads/functional-query/queries/QueryTest/describe-path.test:

http://gerrit.cloudera.org:8080/#/c/15395/5/testdata/workloads/functional-query/queries/QueryTest/describe-path.test@143
PS5, Line 143: describe functional_orc_def.alltypes
> Yeah, I wasn't sure about this, but it was handy during development.
That's fine, I think there's some similar behaviour with nested types if you 
describe nested fields that are not explicitly listed, e.g. you can refer to a 
nested item with the .item member or implicitly by omitting it.



--
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: 7
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: Fri, 27 Mar 2020 17:05:51 +0000
Gerrit-HasComments: Yes

Reply via email to