You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Alex Rodoni (Code Review)" <ge...@cloudera.org> on 2019/11/16 00:33:33 UTC

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Alex Rodoni has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14722


Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................

IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

- The following format specifiers are documented:
- FX
- FM
- Free text

Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
---
M docs/topics/impala_conversion_functions.xml
1 file changed, 371 insertions(+), 380 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/14722/1
-- 
To view, visit http://gerrit.cloudera.org:8080/14722
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 3: Verified+1

Build Successful 

https://jenkins.impala.io/job/gerrit-docs-auto-test/543/ : Doc tests passed.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Nov 2019 20:51:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 4: Verified+1

Build Successful 

https://jenkins.impala.io/job/gerrit-docs-auto-test/544/ : Doc tests passed.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Dec 2019 20:10:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Alex Rodoni (Code Review)" <ge...@cloudera.org>.
Alex Rodoni has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................

IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

- The following format specifiers are documented:
- FX
- FM
- Free text

Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Reviewed-on: http://gerrit.cloudera.org:8080/14722
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Gabor Kaszab <ga...@cloudera.com>
---
M docs/topics/impala_conversion_functions.xml
1 file changed, 371 insertions(+), 396 deletions(-)

Approvals:
  Impala Public Jenkins: Verified
  Gabor Kaszab: Looks good to me, approved

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 5
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Alex Rodoni (Code Review)" <ge...@cloudera.org>.
Hello Gabor Kaszab, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/14722

to look at the new patch set (#2).

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................

IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

- The following format specifiers are documented:
- FX
- FM
- Free text

Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
---
M docs/topics/impala_conversion_functions.xml
1 file changed, 380 insertions(+), 380 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/14722/2
-- 
To view, visit http://gerrit.cloudera.org:8080/14722
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Alex Rodoni (Code Review)" <ge...@cloudera.org>.
Hello Gabor Kaszab, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/14722

to look at the new patch set (#3).

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................

IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

- The following format specifiers are documented:
- FX
- FM
- Free text

Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
---
M docs/topics/impala_conversion_functions.xml
1 file changed, 376 insertions(+), 396 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/14722/3
-- 
To view, visit http://gerrit.cloudera.org:8080/14722
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 1: Verified+1

Build Successful 

https://jenkins.impala.io/job/gerrit-docs-auto-test/533/ : Doc tests passed.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 Nov 2019 00:58:06 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 4:

Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/544/ 

Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstrong@cloudera.com or on this JIRA: IMPALA-7317


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Dec 2019 19:47:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 2: Verified+1

Build Successful 

https://jenkins.impala.io/job/gerrit-docs-auto-test/534/ : Doc tests passed.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Nov 2019 20:21:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Alex Rodoni (Code Review)" <ge...@cloudera.org>.
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 3:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/14722/3/docs/topics/impala_conversion_functions.xml
File docs/topics/impala_conversion_functions.xml:

http://gerrit.cloudera.org:8080/#/c/14722/3/docs/topics/impala_conversion_functions.xml@557
PS3, Line 557: <p>The <codeph>AM</codeph>/<codeph>PM</codeph> input
             :                         length has to match the pattern's length. For example,
             :                         the input argument of <codeph>A.M.</codeph> is not
             :                         accepted for the <varname>pattern</varname> of
             :                           <codeph>AM</codeph>.</p>
> I have just noticed that this part is valid for string to datetime path.
It was from your design doc. 
I will remove it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 03 Dec 2019 19:48:06 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 2:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@549
PS2, Line 549: The following rules apply to string:
This doesn't make sense for me.
"The following rules apply:" should be enough.


http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@560
PS2, Line 560: to have the same
             :                                 length
"to have the maximum possible length."


http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@589
PS2, Line 589: <p>Valid only for the date/time to string conversions.</p>
The FM modifier can be used in either datetime to string and string to datetime scenarios so this is not correct. What I meant with the comment is that the description you had for "FM" is true only for the datetime to string path. Please remove this line and rephrase the above like:

"In a datetime to string conversion FM suppresses blank paddig..."
"In a string to datetime conversion FM is used for omitting the effect of FX for certain tokens." (or something similar :) )


http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@633
PS2, Line 633: double quotes in the free text
             :                               token
double quotes surrounding the free text token


http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@800
PS2, Line 800:  
nit: please remove the trailing space here and one line below.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Nov 2019 10:23:11 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 04 Dec 2019 13:30:17 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 3: Code-Review+1

(1 comment)

Just one comment. If that's taken care then I'm fine to +2. Thanks for making these changes!

http://gerrit.cloudera.org:8080/#/c/14722/3/docs/topics/impala_conversion_functions.xml
File docs/topics/impala_conversion_functions.xml:

http://gerrit.cloudera.org:8080/#/c/14722/3/docs/topics/impala_conversion_functions.xml@557
PS3, Line 557: <p>The <codeph>AM</codeph>/<codeph>PM</codeph> input
             :                         length has to match the pattern's length. For example,
             :                         the input argument of <codeph>A.M.</codeph> is not
             :                         accepted for the <varname>pattern</varname> of
             :                           <codeph>AM</codeph>.</p>
I have just noticed that this part is valid for string to datetime path.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 28 Nov 2019 16:13:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Alex Rodoni (Code Review)" <ge...@cloudera.org>.
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 2:

(5 comments)

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

http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@549
PS2, Line 549: The following rules apply to string:
> This doesn't make sense for me.
Done


http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@560
PS2, Line 560: to have the same
             :                                 length
> "to have the maximum possible length."
Done


http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@589
PS2, Line 589: <p>Valid only for the date/time to string conversions.</p>
> The FM modifier can be used in either datetime to string and string to date
Done


http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@633
PS2, Line 633: double quotes in the free text
             :                               token
> double quotes surrounding the free text token
Done


http://gerrit.cloudera.org:8080/#/c/14722/2/docs/topics/impala_conversion_functions.xml@800
PS2, Line 800:  
> nit: please remove the trailing space here and one line below.
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Nov 2019 20:27:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Alex Rodoni (Code Review)" <ge...@cloudera.org>.
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 1:

(13 comments)

http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml
File docs/topics/impala_conversion_functions.xml:

http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@552
PS1, Line 552: <li>
             :                           <p>Must be specified at the beginning of the format
             :                               <varname>pattern</varname> and is valid for the
             :                             whole <varname>pattern</varname>.</p>
             :                         </li>
> This statement is true for both direction of the conversion not just for st
Done


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@558
PS1, Line 558: <p> In string to date/time conversions, this forces
             :                             strict separator matching and expects all the tokens
             :                             in the input to have the same length as their
             :                             maximum length. For example, a month has to be of
             :                             length 2 prefixed by zero if needed.</p>
> I'd split this into two items:
Done


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@564
PS1, Line 564: <li>
             :                           <p>Numeric output is left padded with zeros.</p>
             :                         </li>
> This is a valid statement but for datetime to string path.
Done


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@567
PS1, Line 567: <li>
             :                           <p>Non-numeric output except for
             :                               <codeph>AM</codeph>/<codeph>PM</codeph> is right
             :                             padded with spaces up to the expected length.</p>
             :                         </li>
> In milestone 1 and 2 there are no tokens with non-numeric output (except AM
Done


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@572
PS1, Line 572: <li>
             :                           <p>Separators must match exactly, including the
             :                             separator character.</p>
             :                         </li>
> I'd merge this item with the one at the top of the list that mentions stric
Done


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@588
PS1, Line 588: <entry>Suppresses blank padding for the element immediately
             :                       following an <codeph>FM</codeph> in the
             :                         <varname>pattern</varname> string:<ul>
             :                         <li>Trailing spaces for text output</li>
             :                         <li>Leading zeroes for numeric output</li>
             :                       </ul></entry>
> This is valid only for the datetime to string path.
Done


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@629
PS1, Line 629: datet/ime
> nit: typo
Done


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@635
PS1, Line 635: except the case may differ.
> Case also has to match.
Done


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@643
PS1, Line 643: 'YYYY-MM-DD"<b>\"text\"</b>"'
> The format part is incorrect in this example. It should be surrounded by do
Done


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@646
PS1, Line 646:  To include a literal double quote character
             :                               within the nested string, the double quote
             :                               character must be escaped with a double backslash
             :                                 (<codeph>\\"</codeph>).</p>
> I think there is a confusion in the design doc about how to include literal
Done


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@652
PS1, Line 652: <p> If the whole <varname>pattern</varname> string
             :                               is delimited by double quotes, escape with a
             :                               triple backslash: (\\\")</p>
> Let me provide the same example here that I gave above. The difference is t
Done


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@665
PS1, Line 665: an error
             :                               returns.
> an error returns as it's not trivial to find where the separator sequence e
Done


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@792
PS1, Line 792: CAST('2019-11-10text'
             :    AS DATE
             :    FORMAT 'YYYY-MM-DD"text"')
> Instead of this example I'd use something that seems more real-life e.g.:
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Nov 2019 19:34:41 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Alex Rodoni (Code Review)" <ge...@cloudera.org>.
Hello Gabor Kaszab, Impala Public Jenkins, 

I'd like you to reexamine a change. Please visit

    http://gerrit.cloudera.org:8080/14722

to look at the new patch set (#4).

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................

IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

- The following format specifiers are documented:
- FX
- FM
- Free text

Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
---
M docs/topics/impala_conversion_functions.xml
1 file changed, 371 insertions(+), 396 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/22/14722/4
-- 
To view, visit http://gerrit.cloudera.org:8080/14722
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 4
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 2:

Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/534/ 

Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstrong@cloudera.com or on this JIRA: IMPALA-7317


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Nov 2019 19:34:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 1:

Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/533/ 

Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstrong@cloudera.com or on this JIRA: IMPALA-7317


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Sat, 16 Nov 2019 00:33:40 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 3:

Build Started https://jenkins.impala.io/job/gerrit-docs-auto-test/543/ 

Testing docs change - this change appears to modify docs/ and no code. This is experimental - please report any issues to tarmstrong@cloudera.com or on this JIRA: IMPALA-7317


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 3
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 27 Nov 2019 20:27:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Alex Rodoni (Code Review)" <ge...@cloudera.org>.
Alex Rodoni has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 1:

> (13 comments)
 > 
 > Thanks for making these changes! It took some time to separate the
 > changes related to formatting from the changes related to milestone
 > 2 so I focused only on the part related to FX, FM and Free text
 > tokens, plus the examples at the end.

Sorry for all the format changes!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Thu, 21 Nov 2019 17:46:01 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2

Posted by "Gabor Kaszab (Code Review)" <ge...@cloudera.org>.
Gabor Kaszab has posted comments on this change. ( http://gerrit.cloudera.org:8080/14722 )

Change subject: IMPALA-9141: [DOCS] SQL:2016 date time patterns - Milestone 2
......................................................................


Patch Set 1:

(13 comments)

Thanks for making these changes! It took some time to separate the changes related to formatting from the changes related to milestone 2 so I focused only on the part related to FX, FM and Free text tokens, plus the examples at the end.

http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml
File docs/topics/impala_conversion_functions.xml:

http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@552
PS1, Line 552: <li>
             :                           <p>Must be specified at the beginning of the format
             :                               <varname>pattern</varname> and is valid for the
             :                             whole <varname>pattern</varname>.</p>
             :                         </li>
This statement is true for both direction of the conversion not just for string to datetime path.


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@558
PS1, Line 558: <p> In string to date/time conversions, this forces
             :                             strict separator matching and expects all the tokens
             :                             in the input to have the same length as their
             :                             maximum length. For example, a month has to be of
             :                             length 2 prefixed by zero if needed.</p>
I'd split this into two items:
- Forces strict separator matching.
- Expects all the tokens to have the same length.


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@564
PS1, Line 564: <li>
             :                           <p>Numeric output is left padded with zeros.</p>
             :                         </li>
This is a valid statement but for datetime to string path.


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@567
PS1, Line 567: <li>
             :                           <p>Non-numeric output except for
             :                               <codeph>AM</codeph>/<codeph>PM</codeph> is right
             :                             padded with spaces up to the expected length.</p>
             :                         </li>
In milestone 1 and 2 there are no tokens with non-numeric output (except AM/PM) sot this can be dropped. From milestone 3 where MONTH name and DAY names are introduced this will make sense, however, for the datetime to string path.


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@572
PS1, Line 572: <li>
             :                           <p>Separators must match exactly, including the
             :                             separator character.</p>
             :                         </li>
I'd merge this item with the one at the top of the list that mentions strict separator matching.


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@588
PS1, Line 588: <entry>Suppresses blank padding for the element immediately
             :                       following an <codeph>FM</codeph> in the
             :                         <varname>pattern</varname> string:<ul>
             :                         <li>Trailing spaces for text output</li>
             :                         <li>Leading zeroes for numeric output</li>
             :                       </ul></entry>
This is valid only for the datetime to string path.


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@629
PS1, Line 629: datet/ime
nit: typo


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@635
PS1, Line 635: except the case may differ.
Case also has to match.


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@643
PS1, Line 643: 'YYYY-MM-DD"<b>\"text\"</b>"'
The format part is incorrect in this example. It should be surrounded by double quotes and there are too many double quotes around the text token. This should be:
"YYYY-MM-DD<b>\"text\"</b>"


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@646
PS1, Line 646:  To include a literal double quote character
             :                               within the nested string, the double quote
             :                               character must be escaped with a double backslash
             :                                 (<codeph>\\"</codeph>).</p>
I think there is a confusion in the design doc about how to include literal double quotes within the free text token.
If the format itself is surrounded by single quotes then the free text token can be surrounded by (non-escaped) double quotes and to include double quotes within they have to be escaped by a single backslash.
One example (might make sense to also include to the doc):
cast('"Date:" 2019-11-10' as date format '"\"Date:\" "YYYY-MM-DD')


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@652
PS1, Line 652: <p> If the whole <varname>pattern</varname> string
             :                               is delimited by double quotes, escape with a
             :                               triple backslash: (\\\")</p>
Let me provide the same example here that I gave above. The difference is that here the format is surrounded by double quotes so the double quotes for the free text boundaries has to be escaped and the double quotes within the free text has to be escaped with "\\\". Could you add this example here?
cast('"Date:" 2019-11-10' as date format "\"\\\"Date:\\\" \"YYYY-MM-DD")


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@665
PS1, Line 665: an error
             :                               returns.
an error returns as it's not trivial to find where the separator sequence ends and where the free text starts.


http://gerrit.cloudera.org:8080/#/c/14722/1/docs/topics/impala_conversion_functions.xml@792
PS1, Line 792: CAST('2019-11-10text'
             :    AS DATE
             :    FORMAT 'YYYY-MM-DD"text"')
Instead of this example I'd use something that seems more real-life e.g.:
CAST('Date: 2019-11-10' AS DATE FORMAT '"Date: "YYYY-MM-DD')



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Id31e9ab14b56afc2e0bd7332cac299243a16bb07
Gerrit-Change-Number: 14722
Gerrit-PatchSet: 1
Gerrit-Owner: Alex Rodoni <ar...@cloudera.com>
Gerrit-Reviewer: Gabor Kaszab <ga...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Wed, 20 Nov 2019 11:31:57 +0000
Gerrit-HasComments: Yes