Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/17811 )
Change subject: WIP IMPALA-9498: Allow returning arrays in select list ...................................................................... Patch Set 17: (8 comments) Sorry for the late review. I managed to go through the tests and the commit msg. Will continue checking the rest tomorrow. http://gerrit.cloudera.org:8080/#/c/17811/17//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/17811/17//COMMIT_MSG@7 PS17, Line 7: WIP Is this still a work in progress? What would be needed to make this "live"? http://gerrit.cloudera.org:8080/#/c/17811/17//COMMIT_MSG@15 PS17, Line 15: Things intentionally postponed for later commits: By later commits do you mean later version of this patch or covered by separate Jiras? If the latest then could you open Jiras to not to forget them and could you include the Jira IDs here? http://gerrit.cloudera.org:8080/#/c/17811/17//COMMIT_MSG@26 PS17, Line 26: Finalize behavior with column masking/row filtering Same question: is this expected in this patch or a separate one? http://gerrit.cloudera.org:8080/#/c/17811/17//COMMIT_MSG@29 PS17, Line 29: Was there anything specific in terms of testing that is worth mentioning here? http://gerrit.cloudera.org:8080/#/c/17811/17/testdata/workloads/functional-query/queries/QueryTest/nested-array-in-select-list.test File testdata/workloads/functional-query/queries/QueryTest/nested-array-in-select-list.test: http://gerrit.cloudera.org:8080/#/c/17811/17/testdata/workloads/functional-query/queries/QueryTest/nested-array-in-select-list.test@34 PS17, Line 34: IllegalStateException: Sorting is not supported for collection columns. The error msg is misleading here: Sorting is not done by collection columns in the query. The issue is that you do a sort in a query where you actually query arrays as well, but you don't sort by collection columns. http://gerrit.cloudera.org:8080/#/c/17811/17/testdata/workloads/functional-query/queries/QueryTest/nested-array-in-select-list.test@37 PS17, Line 37: in nit: typo http://gerrit.cloudera.org:8080/#/c/17811/17/testdata/workloads/functional-query/queries/QueryTest/nested-array-in-select-list.test@147 PS17, Line 147: ==== Could you write a test that has a WHERE filter (on a non-array column)? Could you create a view that contains arrays and query these arrays from the views? I see that there is coverage for inline views but not for regular ones. These tests are run on a small dataset. Have you performed some tests on a way bigger dataset? E.g. multiple files, multiple partitions, where the data doesn't fit in a single row batch. http://gerrit.cloudera.org:8080/#/c/17811/17/tests/query_test/test_nested_types.py File tests/query_test/test_nested_types.py: http://gerrit.cloudera.org:8080/#/c/17811/17/tests/query_test/test_nested_types.py@155 PS17, Line 155: nested-array-in-select-list I find the naming a bit misleading: this is not just for nested arrays, but for regular arrays as well, right? -- To view, visit http://gerrit.cloudera.org:8080/17811 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ibb1e42ffb21c7ddc033aba0f754b0108e46f34d0 Gerrit-Change-Number: 17811 Gerrit-PatchSet: 17 Gerrit-Owner: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com> Gerrit-Reviewer: Csaba Ringhofer <csringho...@cloudera.com> Gerrit-Reviewer: Daniel Becker <daniel.bec...@cloudera.com> Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Comment-Date: Tue, 09 Nov 2021 16:20:25 +0000 Gerrit-HasComments: Yes
