Mihaly Szjatinya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/21825 )

Change subject: IMPALA-889: Add trim() function matching ANSI SQL definition
......................................................................


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/21825/8/fe/src/main/cup/sql-parser.cup
File fe/src/main/cup/sql-parser.cup:

http://gerrit.cloudera.org:8080/#/c/21825/8/fe/src/main/cup/sql-parser.cup@3885
PS8, Line 3885:
> What I meant is that string_expr doesn't seem necessary to me and just make
I think string_expr idea in itself is not bad, eventually it can be used for 
all string functions. But that's not for this scope. Removed for now.


http://gerrit.cloudera.org:8080/#/c/21825/9/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
File fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java:

http://gerrit.cloudera.org:8080/#/c/21825/9/fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java@1932
PS9, Line 1932:     AnalysisError("select trim(foo from '     &@&127+  &@   ')",
> When I checked out the patch this test failed - the difference seems to be
>When I checked out the patch this test failed - the >difference seems to be 
>that the error message had capital >TRIM.

Fixed.

>I also have an issue with the error message:
>In this case besides LEADING/TRAILING/BOTH it could also >accept a valid 
>column name. I think that this mentioning >this would be useful in the typical 
>scenario of mistyping a >column name.

Added.



--
To view, visit http://gerrit.cloudera.org:8080/21825
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3c4fa6d0d8d0684c4b6d8dac8fd531d205e4f7b4
Gerrit-Change-Number: 21825
Gerrit-PatchSet: 10
Gerrit-Owner: Mihaly Szjatinya <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Daniel Becker <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Mihaly Szjatinya <[email protected]>
Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]>
Gerrit-Reviewer: Peter Rozsa <[email protected]>
Gerrit-Comment-Date: Mon, 28 Oct 2024 13:08:22 +0000
Gerrit-HasComments: Yes

Reply via email to