You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Gabor Kaszab (Code Review)" <ge...@cloudera.org> on 2019/11/08 09:06:12 UTC
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Gabor Kaszab has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14665
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
IMPALA-9131: Use single quotes around FORMAT clause in CAST
When running a CAST(..FORMAT..) query then the header of the output
shows the value of the FORMAT clause surrounded by double quotes.
However, the SQL way is to use single quotes for strings so this
patch changes the printout from using double quotes to use single
quotes instead.
Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
---
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
2 files changed, 3 insertions(+), 3 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/14665/1
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14665/3/be/src/runtime/datetime-iso-sql-format-parser.cc
File be/src/runtime/datetime-iso-sql-format-parser.cc:
http://gerrit.cloudera.org:8080/#/c/14665/3/be/src/runtime/datetime-iso-sql-format-parser.cc@219
PS3, Line 219: )) {
Why was this condition removed?
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Nov 2019 15:34:47 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 1:
Norbi: Both single quote and double quote are allowed to use for the format string so those tests are valid as well. This change is only for the printout so I think there is no need to rewrite the existing ones as they are still valid.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Nov 2019 11:42:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 6: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 6
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Dec 2019 13:50:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 4:
Actually, that issue with the backslashes followed by a single quote is introduced with this change so should be taken care here.
After some investigation what I see is that backslashes are treated differently when given in format or input.
- In the format when the user provides \\' it will be parsed as two backslashes and a single quote.
- On the contrary when the user gives the same in the input then the two double quotes would be treated as a single one followed by a single quote.
The IsSeparator() implementation is changed to accept a double quote followed by a single quote as a single quote, hence the sample output you gave passes. I'm thinking of having two separate implementation of IsSeparator(0 one for the format and one for the input to tackle this difference.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 Nov 2019 08:11:24 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 2:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14665/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc
File be/src/runtime/datetime-iso-sql-format-tokenizer.cc:
http://gerrit.cloudera.org:8080/#/c/14665/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@246
PS2, Line 246: strncmp(*current_pos, "FX", 2)
Not this change, but i think this should be strncasecmp(..) instead. Format tokens should be case insesnitive, right?
http://gerrit.cloudera.org:8080/#/c/14665/2/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
File fe/src/main/java/org/apache/impala/analysis/CastExpr.java:
http://gerrit.cloudera.org:8080/#/c/14665/2/fe/src/main/java/org/apache/impala/analysis/CastExpr.java@209
PS2, Line 209: formatClause = " FORMAT '" + castFormat_ + "'";
This still needs to be fixed:
impala> select cast("2010-'02'01" as date FORMAT "YYYY-'MM'DD");
+-----------------------------------------------------+
| cast('2010-\'02\'01' as date format 'yyyy-'mm'dd') |
+-----------------------------------------------------+
| 2010-02-01 |
+-----------------------------------------------------+
The escaping of apostrophes in the FORMAT string in the header is incorrect.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Mon, 18 Nov 2019 11:32:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14665/3/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
File fe/src/main/java/org/apache/impala/analysis/CastExpr.java:
http://gerrit.cloudera.org:8080/#/c/14665/3/fe/src/main/java/org/apache/impala/analysis/CastExpr.java@197
PS3, Line 197: public String getCastFormatWithEscapedSingleQuotes() {
: Preconditions.checkNotNull(castFormat_);
: Preconditions.checkState(!castFormat_.isEmpty());
: return castFormat_.replaceAll("([^\\\\])'", "$1\\\\'");
: }
This doesn't work under every circumstance, e.g.:
- unescaped apostrophe at the beginning of the format string:
select cast("'2010-02-01" as date format "'yyyy-mm-dd");
the printout in the header is incorrect:
cast('\'2010-02-01' as date format ''yyyy-mm-dd')
- escaped \ in front of an unescaped apostrophe in a text token:
select cast("2010-\\'-02-01" as date FORMAT "FXYYYY-\"\\'\"-MM-DD");
the printout in the header is incorrect:
cast('2010-\\\'-02-01' as date format 'fxyyyy-\"\\'\"-mm-dd')
Maybe you could use something from the StringEscapeUtils class in org.apache.commons.lang3 package?
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Nov 2019 12:19:12 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Norbert Luksa, Anonymous Coward (536), Attila Jeges, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14665
to look at the new patch set (#3).
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
IMPALA-9131: Use single quotes around FORMAT clause in CAST
When running a CAST(..FORMAT..) query then the header of the output
shows the value of the FORMAT clause surrounded by double quotes.
However, the SQL way is to use single quotes for strings so this
patch changes the printout from using double quotes to use single
quotes instead.
Additionally, this fixes a bug where it wasn't possible to have a
single quote separator in a format string that was itself surrounded
by single quotes. As a fix the single quote separator can be escaped
by a backslash in this case.
Another additional fix to make the FX modifier case-insensitive.
Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
---
M be/src/runtime/datetime-iso-sql-format-parser.cc
M be/src/runtime/datetime-iso-sql-format-tokenizer.cc
M be/src/runtime/datetime-iso-sql-format-tokenizer.h
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M tests/query_test/test_cast_with_format.py
6 files changed, 87 insertions(+), 14 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/14665/3
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 4:
I've found one more anomaly about apoostrophe separator:
This is succeeding although \ is not a separator:
> select cast("2010\\'02-01" as date FORMAT "YYYY'MM-DD");
This works too;
> select cast("2010\\\'02-01" as date FORMAT "YYYY'MM-DD");
This fails:
> select cast("2010\\\\'02-01" as date FORMAT "YYYY'MM-DD");
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Nov 2019 12:57:45 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5157/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Nov 2019 10:30:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5026/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Nov 2019 15:32:19 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 4:
> I've found one more anomaly about apostrophe separator:
>
> This is succeeding although \ is not a separator:
> > select cast("2010\\'02-01" as date FORMAT "YYYY'MM-DD");
>
> This works too;
> > select cast("2010\\\'02-01" as date FORMAT "YYYY'MM-DD");
>
> This fails:
> > select cast("2010\\\\'02-01" as date FORMAT "YYYY'MM-DD");
This issue might not be related to this patch-set. Maybe it has to be fixed separately?
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Nov 2019 13:04:13 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Norbert Luksa (Code Review)" <ge...@cloudera.org>.
Norbert Luksa has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 1:
Should not the queries in QueryTest/date.test and QueryTest/cast_format_table.test be changed to use single quotes instead?
It's also used in quer_test/test_cast_with_format.py.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 13:27:21 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14665/1/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
File fe/src/main/java/org/apache/impala/analysis/CastExpr.java:
http://gerrit.cloudera.org:8080/#/c/14665/1/fe/src/main/java/org/apache/impala/analysis/CastExpr.java@209
PS1, Line 209: castFormat_
> Are single quotes allowed inside 'castFormat_' ?
Yes, there are two ways you can use single quotes within castFormat_:
1: It's a valid separator char
2: Can be in a free text token.
As a free text this works fine:
select cast(date'2019-11-10' as string format 'YYYY"\'" ');
+------------------------------------------------------+
| cast(date '2019-11-10' as string format 'yyyy"\'" ') |
+------------------------------------------------------+
However, providing single quote as a separator seems a bit problematic:
apparent;y there is no way to have a single quote separator when the format is surrounded by single quotes (treats it as a closing quote).
> select cast(date'2019-10-11' as string format 'YYYY'MM');
>
select cast(date'2019-11-10' as string format "DAY'");
+-------------------------------------------------+
| cast(date '2019-11-10' as string format 'day'') |
+-------------------------------------------------+
I'm trying to figure out a way to handle single quote as separator. I also consider removing it from the valid separator list as it seems more complicated to implement than the gain we have.
But anyway, this issue is not related to the fact that the whole format should be surrounded by double quotes in the printout.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Mon, 11 Nov 2019 13:42:22 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 6:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14665/5/be/src/runtime/datetime-iso-sql-format-tokenizer.cc
File be/src/runtime/datetime-iso-sql-format-tokenizer.cc:
http://gerrit.cloudera.org:8080/#/c/14665/5/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@231
PS5, Line 231: bool read_escaped_single_quotes) {
> Please add DCHECKs for 'current_pos' and 'str_end' being valid pointers.
Done
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 6
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Dec 2019 09:41:27 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 2:
Made an adjustment that escaped single quotes are now accepted as separators in the format.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Nov 2019 14:48:40 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 6: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 6
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Dec 2019 09:55:41 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14665/3/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
File fe/src/main/java/org/apache/impala/analysis/CastExpr.java:
http://gerrit.cloudera.org:8080/#/c/14665/3/fe/src/main/java/org/apache/impala/analysis/CastExpr.java@197
PS3, Line 197: public String getCastFormatWithEscapedSingleQuotes() {
: Preconditions.checkNotNull(castFormat_);
: Preconditions.checkState(!castFormat_.isEmpty());
: StringBuilder result = new StringBuilder();
:
> This doesn't work under every circumstance, e.g.:
That library seems to be deprecated for me (moved to different location). I didn't wabt toi include a dependency on that so I wrote some parsing to escape the single quotes and take into account the preceeding escaped backslashes.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Nov 2019 15:44:26 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 2:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14665/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc
File be/src/runtime/datetime-iso-sql-format-tokenizer.cc:
http://gerrit.cloudera.org:8080/#/c/14665/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@237
PS2, Line 237: ++(*current_pos);
Giving this a second thought it sounds a bit weird if an IsSomething() function has a side effect of also increasing a pointer or such. This works at least so it's good for a PoC but I'll try to find an alternative way.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Fri, 15 Nov 2019 20:16:18 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14665/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc
File be/src/runtime/datetime-iso-sql-format-tokenizer.cc:
http://gerrit.cloudera.org:8080/#/c/14665/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@237
PS2, Line 237: ++(*current_pos);
> Giving this a second thought it sounds a bit weird if an IsSomething() func
Well, leaving it this way wouldn't cause any harm as calling this function multiple times would still produce the same result. Anyway, I haven't found any better solution.
http://gerrit.cloudera.org:8080/#/c/14665/2/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@246
PS2, Line 246: strncasecmp(*current_pos, "FX"
> Not this change, but i think this should be strncasecmp(..) instead. Format
I added the fix to this patch (didn't want to create a new jira and handle it separately.)
http://gerrit.cloudera.org:8080/#/c/14665/2/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
File fe/src/main/java/org/apache/impala/analysis/CastExpr.java:
http://gerrit.cloudera.org:8080/#/c/14665/2/fe/src/main/java/org/apache/impala/analysis/CastExpr.java@209
PS2, Line 209: } else {
> This still needs to be fixed:
Indeed, i have fixed everything but the original issue with my previous patches :)
Done
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Nov 2019 09:48:24 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 3:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14665/3/be/src/runtime/datetime-iso-sql-format-parser.cc
File be/src/runtime/datetime-iso-sql-format-parser.cc:
http://gerrit.cloudera.org:8080/#/c/14665/3/be/src/runtime/datetime-iso-sql-format-parser.cc@219
PS3, Line 219: || strncmp(*format, "\\\\\\'", 4) == 0)
> If I left this here then \\\' in the input would have meant double escaped
Makes sense, thanks!
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 3
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Nov 2019 17:35:35 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 6:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5184/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 6
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Dec 2019 10:10:31 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Norbert Luksa, Anonymous Coward (536), Attila Jeges, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14665
to look at the new patch set (#4).
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
IMPALA-9131: Use single quotes around FORMAT clause in CAST
When running a CAST(..FORMAT..) query then the header of the output
shows the value of the FORMAT clause surrounded by double quotes.
However, the SQL way is to use single quotes for strings so this
patch changes the printout from using double quotes to use single
quotes instead.
Additionally, this fixes a bug where it wasn't possible to have a
single quote separator in a format string that was itself surrounded
by single quotes. As a fix the single quote separator can be escaped
by a backslash in this case.
Another additional fix to make the FX modifier case-insensitive.
Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
---
M be/src/runtime/datetime-iso-sql-format-parser.cc
M be/src/runtime/datetime-iso-sql-format-tokenizer.cc
M be/src/runtime/datetime-iso-sql-format-tokenizer.h
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M tests/query_test/test_cast_with_format.py
6 files changed, 118 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/14665/4
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 6:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5307/ DRY_RUN=true
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 6
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Dec 2019 09:42:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 4:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14665/3/be/src/runtime/datetime-iso-sql-format-parser.cc
File be/src/runtime/datetime-iso-sql-format-parser.cc:
http://gerrit.cloudera.org:8080/#/c/14665/3/be/src/runtime/datetime-iso-sql-format-parser.cc@219
PS3, Line 219: )) {
> Why was this condition removed?
If I left this here then \\\' in the input would have meant double escaped single quote instead of an escaped backslash followed by an escaped single quote.
Additionally, I figured that there is no need to take care of double escaped single quotes as it only makes sense for double quotes.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Nov 2019 15:58:16 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5172/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 5
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 Nov 2019 10:10:18 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 4:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5159/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Nov 2019 16:25:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Norbert Luksa, Anonymous Coward (536), Attila Jeges, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14665
to look at the new patch set (#2).
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
IMPALA-9131: Use single quotes around FORMAT clause in CAST
When running a CAST(..FORMAT..) query then the header of the output
shows the value of the FORMAT clause surrounded by double quotes.
However, the SQL way is to use single quotes for strings so this
patch changes the printout from using double quotes to use single
quotes instead.
Additionally, this fixes a bug where it wasn't possible to have a
single quote separator in a format string that was itself surrounded
by single quotes. As a fix the single quote separator can be escaped
by a backslash in this case.
Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
---
M be/src/runtime/datetime-iso-sql-format-parser.cc
M be/src/runtime/datetime-iso-sql-format-tokenizer.cc
M be/src/runtime/datetime-iso-sql-format-tokenizer.h
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M tests/query_test/test_cast_with_format.py
6 files changed, 59 insertions(+), 13 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/14665/2
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 2
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 4:
Thanks for spotting this, Attila! I've opened a separate Jira to cover that issue:
https://issues.apache.org/jira/browse/IMPALA-9206
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 4
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Nov 2019 14:19:57 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Anonymous Coward (Code Review)" <ge...@cloudera.org>.
Anonymous Coward (536) has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 1: Code-Review+1
Thanks for this fix Gabor.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 09:28:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
IMPALA-9131: Use single quotes around FORMAT clause in CAST
When running a CAST(..FORMAT..) query then the header of the output
shows the value of the FORMAT clause surrounded by double quotes.
However, the SQL way is to use single quotes for strings so this
patch changes the printout from using double quotes to use single
quotes instead.
Additionally, this fixes a bug where it wasn't possible to have a
single quote separator in a format string that was itself surrounded
by single quotes. As a fix the single quote separator can be escaped
by a backslash in this case.
Another additional fix to make the FX modifier case-insensitive.
Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Reviewed-on: http://gerrit.cloudera.org:8080/14665
Reviewed-by: Attila Jeges <at...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M be/src/runtime/datetime-iso-sql-format-parser.cc
M be/src/runtime/datetime-iso-sql-format-tokenizer.cc
M be/src/runtime/datetime-iso-sql-format-tokenizer.h
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M tests/query_test/test_cast_with_format.py
6 files changed, 142 insertions(+), 15 deletions(-)
Approvals:
Attila Jeges: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 7
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Norbert Luksa, Anonymous Coward (536), Attila Jeges, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14665
to look at the new patch set (#6).
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
IMPALA-9131: Use single quotes around FORMAT clause in CAST
When running a CAST(..FORMAT..) query then the header of the output
shows the value of the FORMAT clause surrounded by double quotes.
However, the SQL way is to use single quotes for strings so this
patch changes the printout from using double quotes to use single
quotes instead.
Additionally, this fixes a bug where it wasn't possible to have a
single quote separator in a format string that was itself surrounded
by single quotes. As a fix the single quote separator can be escaped
by a backslash in this case.
Another additional fix to make the FX modifier case-insensitive.
Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
---
M be/src/runtime/datetime-iso-sql-format-parser.cc
M be/src/runtime/datetime-iso-sql-format-tokenizer.cc
M be/src/runtime/datetime-iso-sql-format-tokenizer.h
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M tests/query_test/test_cast_with_format.py
6 files changed, 142 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/14665/6
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 6
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Hello Norbert Luksa, Anonymous Coward (536), Attila Jeges, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14665
to look at the new patch set (#5).
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
IMPALA-9131: Use single quotes around FORMAT clause in CAST
When running a CAST(..FORMAT..) query then the header of the output
shows the value of the FORMAT clause surrounded by double quotes.
However, the SQL way is to use single quotes for strings so this
patch changes the printout from using double quotes to use single
quotes instead.
Additionally, this fixes a bug where it wasn't possible to have a
single quote separator in a format string that was itself surrounded
by single quotes. As a fix the single quote separator can be escaped
by a backslash in this case.
Another additional fix to make the FX modifier case-insensitive.
Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
---
M be/src/runtime/datetime-iso-sql-format-parser.cc
M be/src/runtime/datetime-iso-sql-format-tokenizer.cc
M be/src/runtime/datetime-iso-sql-format-tokenizer.h
M fe/src/main/java/org/apache/impala/analysis/CastExpr.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeExprsTest.java
M tests/query_test/test_cast_with_format.py
6 files changed, 140 insertions(+), 15 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/65/14665/5
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 5
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 5: Code-Review+2
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14665/5/be/src/runtime/datetime-iso-sql-format-tokenizer.cc
File be/src/runtime/datetime-iso-sql-format-tokenizer.cc:
http://gerrit.cloudera.org:8080/#/c/14665/5/be/src/runtime/datetime-iso-sql-format-tokenizer.cc@231
PS5, Line 231: bool read_escaped_single_quotes) {
Please add DCHECKs for 'current_pos' and 'str_end' being valid pointers.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 5
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Fri, 29 Nov 2019 17:21:05 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Attila Jeges (Code Review)" <ge...@cloudera.org>.
Attila Jeges has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14665/1/fe/src/main/java/org/apache/impala/analysis/CastExpr.java
File fe/src/main/java/org/apache/impala/analysis/CastExpr.java:
http://gerrit.cloudera.org:8080/#/c/14665/1/fe/src/main/java/org/apache/impala/analysis/CastExpr.java@209
PS1, Line 209: castFormat_
Are single quotes allowed inside 'castFormat_' ?
If yes, I assume the printout should contain escaped single quotes instead.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Attila Jeges <at...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Norbert Luksa <no...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 14:56:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9131: Use single quotes around FORMAT clause in CAST
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14665 )
Change subject: IMPALA-9131: Use single quotes around FORMAT clause in CAST
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/4985/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests.
--
To view, visit http://gerrit.cloudera.org:8080/14665
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3310abfa6f3ccbbe4c437846c6dd05791153e6f7
Gerrit-Change-Number: 14665
Gerrit-PatchSet: 1
Gerrit-Owner: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Anonymous Coward (536)
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 08 Nov 2019 09:49:00 +0000
Gerrit-HasComments: No