Noemi Pap-Takacs has posted comments on this change. ( http://gerrit.cloudera.org:8080/23855 )
Change subject: IMPALA-14563: Throw AnalysisException when aggregating complex type ...................................................................... Patch Set 2: (4 comments) http://gerrit.cloudera.org:8080/#/c/23855/2//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/23855/2//COMMIT_MSG@9 PS2, Line 9: Currently, when the user tries to aggregate by a complex type column, : we throw an IllegalStateException. : : SELECT complex_col, count(*) FROM table_example GROUP BY complex_col; : : java.lang.IllegalStateException: null : : This does not provide appropriate context about the query's problem. nit: This commit message will become part of git history, therefore "currently" might be misleading. I would use something like "before this patch" and past tense to make it clear that this is the old version. Or you could completely remove this section and concentrate on what change this commit achieved, since that contains enough information. http://gerrit.cloudera.org:8080/#/c/23855/2//COMMIT_MSG@18 PS2, Line 18: we should throw nit: "we now throw" - this is the change achieved by the commit, this will be the "current" version when someone reads it. http://gerrit.cloudera.org:8080/#/c/23855/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java File fe/src/main/java/org/apache/impala/analysis/SelectStmt.java: http://gerrit.cloudera.org:8080/#/c/23855/2/fe/src/main/java/org/apache/impala/analysis/SelectStmt.java@1167 PS2, Line 1167: // reference the original expr in the error msg I think this comment is redundant here. http://gerrit.cloudera.org:8080/#/c/23855/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java File fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java: http://gerrit.cloudera.org:8080/#/c/23855/2/fe/src/test/java/org/apache/impala/analysis/AnalyzeStmtsTest.java@3006 PS2, Line 3006: cannot used 'cannot be used' -- To view, visit http://gerrit.cloudera.org:8080/23855 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ie431873efe7100e8150e93be2aa016381d687268 Gerrit-Change-Number: 23855 Gerrit-PatchSet: 2 Gerrit-Owner: Surya Hebbar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]> Gerrit-Reviewer: Peter Rozsa <[email protected]> Gerrit-Reviewer: Surya Hebbar <[email protected]> Gerrit-Comment-Date: Mon, 19 Jan 2026 10:55:07 +0000 Gerrit-HasComments: Yes
