Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/15103 )
Change subject: IMPALA-9324: Correctly handle ORC UNION type in scanner ...................................................................... Patch Set 1: Code-Review+1 (3 comments) Left some minor comments, but it LGTM. http://gerrit.cloudera.org:8080/#/c/15103/1/be/src/exec/hdfs-orc-scanner.cc File be/src/exec/hdfs-orc-scanner.cc: http://gerrit.cloudera.org:8080/#/c/15103/1/be/src/exec/hdfs-orc-scanner.cc@203 PS1, Line 203: "Root of the selected type returned by the ORC lib is not STRUCT: $0. " nit: this could be unified with orc-metadata-utils.cc:28, would it make sense to add an error code for this case as well? Or are both checks necessary? http://gerrit.cloudera.org:8080/#/c/15103/1/testdata/workloads/functional-query/queries/DataErrorsTest/orc-type-checks.test File testdata/workloads/functional-query/queries/DataErrorsTest/orc-type-checks.test: http://gerrit.cloudera.org:8080/#/c/15103/1/testdata/workloads/functional-query/queries/DataErrorsTest/orc-type-checks.test@160 PS1, Line 160: Note that before : # IMPALA-9324 this query will crash. nit: I think this comment here is not necessary. http://gerrit.cloudera.org:8080/#/c/15103/1/testdata/workloads/functional-query/queries/DataErrorsTest/orc-type-checks.test@172 PS1, Line 172: has an incompatible ORC schema for column '$DATABASE.ill_complextypes.u', Just a question: Isn't the fie is also part of the error msg? Can they be omitted? -- To view, visit http://gerrit.cloudera.org:8080/15103 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I452d27b4e281eada00b62ac58af773a3479163ec Gerrit-Change-Number: 15103 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Norbert Luksa <[email protected]> Gerrit-Comment-Date: Fri, 24 Jan 2020 10:50:26 +0000 Gerrit-HasComments: Yes
