Gabor Kaszab has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/14044 )

Change subject: IMPALA-8160: [DOCS] Document CAST (...FORMAT..) function
......................................................................


Patch Set 2:

(13 comments)

Thanks Alex for taking care of it! I have some comments but other than that 
this is fine.

http://gerrit.cloudera.org:8080/#/c/14044/2/docs/topics/impala_conversion_functions.xml
File docs/topics/impala_conversion_functions.xml:

http://gerrit.cloudera.org:8080/#/c/14044/2/docs/topics/impala_conversion_functions.xml@166
PS2, Line 166: The patterns supported in the <codeph>FORMAT</codeph> clause are 
not the same format
             :             patterns used with the other Impala conversion 
functions, e.g.
I would mention here that this format is introduced to support ISO:SQL:2016 
standard patterns.


http://gerrit.cloudera.org:8080/#/c/14044/2/docs/topics/impala_conversion_functions.xml@192
PS2, Line 192: If a fewer number of digits in 
nit: "If fewer number of digits <are> in"


http://gerrit.cloudera.org:8080/#/c/14044/2/docs/topics/impala_conversion_functions.xml@193
PS2, Line 193:  the current date is used to complete
current date is only used for completing the year pattern.


http://gerrit.cloudera.org:8080/#/c/14044/2/docs/topics/impala_conversion_functions.xml@303
PS2, Line 303: The year is adjusted to be nearest to the nearest century to the 
current
             :                         date:
Here I'd list all the possible use-cases. For that there is a link to an Oracle 
doc from the design doc of this feature. For finding the link just check the 
section of "RR".


http://gerrit.cloudera.org:8080/#/c/14044/2/docs/topics/impala_conversion_functions.xml@307
PS2, Line 307: For string to datetime semantics, round year when last 2 digits 
of
             :                             current year is greater than 49.
Not sure what the meaning of this sentence is.


http://gerrit.cloudera.org:8080/#/c/14044/2/docs/topics/impala_conversion_functions.xml@538
PS2, Line 538:                         Must be specified at the end of the 
<varname>pattern</varname> string
TZH and TZM can be anywhere in the pattern. TZM don't even has to be right 
after TZH. I wouldn't articulate it here, as this is not something we should 
suggest to users, though.


http://gerrit.cloudera.org:8080/#/c/14044/2/docs/topics/impala_conversion_functions.xml@545
PS2, Line 545: -15 and 15
The design doc incorrectly mentions this limit. In fact Impala parses the 
optional +/- sign and the 2 digits for TZH and without checking the parsed 
value simply ignores it.


http://gerrit.cloudera.org:8080/#/c/14044/2/docs/topics/impala_conversion_functions.xml@572
PS2, Line 572: Unsigned numbers between 0 and 59 are allowed for the source
             :                         <varname>expression</varname>.
There is no range check for TZM. Similarly to TZH after parsin Impala ignores 
this value.


http://gerrit.cloudera.org:8080/#/c/14044/2/docs/topics/impala_conversion_functions.xml@628
PS2, Line 628:  t
typo


http://gerrit.cloudera.org:8080/#/c/14044/2/docs/topics/impala_conversion_functions.xml@635
PS2, Line 635: CAST(“2019-.
             :                         ;101” AS DATE FORMAT “YYYY-MM-DD”)
This was a typo in the design doc. Actually this is the query that would 
succeed:
CAST("2019-.;10 10" AS DATE FORMAT "YYYY-MM-DD")
Note the space char between the month and day part of the input.


http://gerrit.cloudera.org:8080/#/c/14044/2/docs/topics/impala_conversion_functions.xml@642
PS2, Line 642: ph>T</
Please mention that this is for accepting ISO8601 datetime formats.


http://gerrit.cloudera.org:8080/#/c/14044/2/docs/topics/impala_conversion_functions.xml@655
PS2, Line 655: >Z
Same comment as for "T"


http://gerrit.cloudera.org:8080/#/c/14044/2/docs/topics/impala_conversion_functions.xml@668
PS2, Line 668: Examples:
It's great to have a few examples here. I'd add one more to have some coverage 
for TZH, TZM patterns as well.
e.g.:
Query:
SELECT CAST('2019.10.10 13:30:40.123456 +01:30' AS TIMESTAMP
    FORMAT 'YYYY-MM-DD HH24:MI:SS.FF9 TZH:TZM');
Result:
2019-10-10 13:30:40.123456000



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I6235ffd03ac56e648552058ff02491a55289c092
Gerrit-Change-Number: 14044
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni <arod...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <arod...@cloudera.com>
Gerrit-Reviewer: Attila Jeges <atti...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <gaborkas...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com>
Gerrit-Comment-Date: Tue, 13 Aug 2019 11:56:20 +0000
Gerrit-HasComments: Yes

Reply via email to