Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/16066 )
Change subject: IMPALA-9482: Support for BINARY columns ...................................................................... Patch Set 22: (4 comments) http://gerrit.cloudera.org:8080/#/c/16066/14/be/src/runtime/descriptors.h File be/src/runtime/descriptors.h: http://gerrit.cloudera.org:8080/#/c/16066/14/be/src/runtime/descriptors.h@256 PS14, Line 256: return col_descs_[slot_desc->col_path().back()]; > Change the analyzer to disallow complex types in select list if they have b Thanks for digging into this! Sorry that my initial confusion is that "col_descs_" are the top-level columns of the table, but the last item in SchemaPath is not always the top-level column index. Usually the first item of SchemaPath is the top-level column index, and the next items are the index inside the nested type. E.g. the 6th column in table complextypestbl is nested_struct struct< a: int, b: array<int>, ... > If the query selects "nested_struct.a" in the SelectList, the corresponding SchemaPath is [5, 0]. Here [5] is the SchemaPath of "nested_struct". But we are using 0 (the last item) here as the index of col_descs_. So I hope we can add a test of selecting the binary column directly inside a struct<binary> top level column. Maybe I've missed something. Just explaning my confusion. http://gerrit.cloudera.org:8080/#/c/16066/14/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java File fe/src/main/java/org/apache/impala/analysis/LikePredicate.java: http://gerrit.cloudera.org:8080/#/c/16066/14/fe/src/main/java/org/apache/impala/analysis/LikePredicate.java@116 PS14, Line 116: private static boolean isLikeableType(Type type) { > :D poor other types - tbh I don't remember whether the naming was intention haha http://gerrit.cloudera.org:8080/#/c/16066/22/testdata/bin/generate-schema-statements.py File testdata/bin/generate-schema-statements.py: http://gerrit.cloudera.org:8080/#/c/16066/22/testdata/bin/generate-schema-statements.py@222 PS22, Line 222: 'BINARY': 'bytes' nit: it'd be nice to add a trailing comma so future changes don't need to touch this line. http://gerrit.cloudera.org:8080/#/c/16066/22/testdata/datasets/functional/functional_schema_template.sql File testdata/datasets/functional/functional_schema_template.sql: http://gerrit.cloudera.org:8080/#/c/16066/22/testdata/datasets/functional/functional_schema_template.sql@3532 PS22, Line 3532: binary_in_complex_types Can we add some data to this table and add some e2e tests? e.g. select binary_member_col.b from binary_in_complex_types; select a.item from binary_in_complex_types t, t.binary_item_col; select m.key from binary_in_complex_types t, t.binary_key_col; select m.value from binary_in_complex_types t, t.binary_value_col; -- To view, visit http://gerrit.cloudera.org:8080/16066 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I36861a9ca6c2047b0d76862507c86f7f153bc582 Gerrit-Change-Number: 16066 Gerrit-PatchSet: 22 Gerrit-Owner: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Quanlong Huang <huangquanl...@gmail.com> Gerrit-Reviewer: Steve Carlin <scar...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Reviewer: Zoltan Borok-Nagy <borokna...@cloudera.com> Gerrit-Comment-Date: Thu, 18 Aug 2022 09:43:26 +0000 Gerrit-HasComments: Yes
