Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/9134 )
Change subject: IMPALA-5717: Support for reading ORC data files ...................................................................... Patch Set 6: (4 comments) Sorry that some tests were failed. I've fixed them in the next patch. http://gerrit.cloudera.org:8080/#/c/9134/6/be/src/exec/hdfs-orc-scanner.cc File be/src/exec/hdfs-orc-scanner.cc: http://gerrit.cloudera.org:8080/#/c/9134/6/be/src/exec/hdfs-orc-scanner.cc@339 PS6, Line 339: if (type.type != TYPE_DECIMAL) break; We should check scale as well. http://gerrit.cloudera.org:8080/#/c/9134/6/be/src/exec/hdfs-orc-scanner.cc@347 PS6, Line 347: } else { if orc_type.getPrecision <= 9, it's valid to have type.GetByteSize() == 4 http://gerrit.cloudera.org:8080/#/c/9134/6/be/src/exec/hdfs-orc-scanner.cc@767 PS6, Line 767: DCHECK_NE(slot_desc->type().GetByteSize(), 4); ORC decimal(1,1) is stored in int64 batch as well. We shouldn't assume this and should add a case for slot_desc->type().GetByteSize() == 4. http://gerrit.cloudera.org:8080/#/c/9134/6/testdata/bin/load_nested.py File testdata/bin/load_nested.py: http://gerrit.cloudera.org:8080/#/c/9134/6/testdata/bin/load_nested.py@297 PS6, Line 297: def load_orc(): : cluster.hdfs.ensure_home_dir() : db = "nested_orc" : tbl = "dummy" > After the refactor of the nested types test, I think the changes in this fi Yes! I'll remove these. -- To view, visit http://gerrit.cloudera.org:8080/9134 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ia7b6ae4ce3b9ee8125b21993702faa87537790a4 Gerrit-Change-Number: 9134 Gerrit-PatchSet: 6 Gerrit-Owner: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Joe McDonnell <joemcdonn...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Thu, 22 Mar 2018 15:25:39 +0000 Gerrit-HasComments: Yes
