Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14982 )
Change subject: IMPALA-8801: Date type support for ORC scanner ...................................................................... Patch Set 6: (3 comments) Thanks for creating the JIRA! The patch looks good to me actually. Just replying the discussion for IMPALA-9288. http://gerrit.cloudera.org:8080/#/c/14982/2/be/src/exec/orc-metadata-utils.cc File be/src/exec/orc-metadata-utils.cc: http://gerrit.cloudera.org:8080/#/c/14982/2/be/src/exec/orc-metadata-utils.cc@181 PS2, Line 181: case orc::TypeKind::DATE: : if (type.type == TYPE_DATE) return Status::OK(); : break; : d > If I extend the DATE case to also allow "type.type == TYPE_TIMESTAMP" then > the DCHECK fails in UpdateInputBatch(). Yes, in this case we need to modify the casting in OrcTimestampReader::UpdateInputBatch(), since we allow reading ORC Date values and materialize them into Timestamp slots. Note that we create the reader based on the slot descriptor, not the orc type (see OrcColumnReader::Create). So for orc_type = Date and slot_type = Timestamp, we are actually creating a OrcTimestampReader. > BTW, It'd be better to add test coverage for this type compactibility check > in test_scanners.py (See TestOrc.test_type_conversions). Maybe you miss the last line in my last comment. Do you think we should add test on this? http://gerrit.cloudera.org:8080/#/c/14982/6/tests/query_test/test_scanners.py File tests/query_test/test_scanners.py: http://gerrit.cloudera.org:8080/#/c/14982/6/tests/query_test/test_scanners.py@306 PS6, Line 306: class TestOrcDateType(ImpalaTestSuite): nit: merge this into TestOrc below? http://gerrit.cloudera.org:8080/#/c/14982/6/tests/query_test/test_scanners.py@1335 PS6, Line 1335: def test_type_conversions(self, vector, unique_database): It'd be better to add test coverage for date and timestamp type compactibility check here. -- To view, visit http://gerrit.cloudera.org:8080/14982 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I672a2cdd2452a46b676e0e36942fd310f55c4956 Gerrit-Change-Number: 14982 Gerrit-PatchSet: 6 Gerrit-Owner: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Attila Jeges <[email protected]> Gerrit-Reviewer: Csaba Ringhofer <[email protected]> Gerrit-Reviewer: Gabor Kaszab <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Norbert Luksa <[email protected]> Gerrit-Reviewer: Quanlong Huang <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 14 Jan 2020 11:52:09 +0000 Gerrit-HasComments: Yes
