Lars Volker has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/10893 )

Change subject: IMPALA-6677: [DOCS] Document the next_day function
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/10893/1/docs/topics/impala_datetime_functions.xml
File docs/topics/impala_datetime_functions.xml:

http://gerrit.cloudera.org:8080/#/c/10893/1/docs/topics/impala_datetime_functions.xml@1941
PS1, Line 1941:
> Did you see my comment above? I would like to use upper case for SQL. And h
Sorry, I had missed your comment.

I think it'd be good to keep the style consistent, and the previous few 
examples that I looked at used lowercase for SQL keywords and function names. 
Other pages of our docs use uppercase or lowercase for SQL keywords and 
lowercase for function names. Most notably, our function pages also use 
lowercase for function names: 
https://impala.apache.org/docs/build3x/html/topics/impala_math_functions.html#math_functions

If we want to change this going forward, I suggest to change it in a single 
commit (since the files are rather large). I don't feel strongly about which 
way we should go as long as it's consistent.


Space before parentheses usually expresses a logical grouping, whereas function 
arguments are usually not separated from the function name.

This example from our docs contains both:

  SELECT x FROM t1 WHERE y = (SELECT max(z) FROM t2);

I think that this convention provides the best readability. If you feel 
strongly about it, I suggest to start a thread on dev@ and ask for more 
feedback.

We might also want to check what other projects do. I briefly looked at the 
docs of Hive and Phoenix and they seem to follow our current style.


http://gerrit.cloudera.org:8080/#/c/10893/2/docs/topics/impala_datetime_functions.xml
File docs/topics/impala_datetime_functions.xml:

http://gerrit.cloudera.org:8080/#/c/10893/2/docs/topics/impala_datetime_functions.xml@1941
PS2, Line 1941: Saturday December 25
"first Saturday after December..."?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I2dacc86ff69a1016b1863d9db66dd29fd832b715
Gerrit-Change-Number: 10893
Gerrit-PatchSet: 2
Gerrit-Owner: Alex Rodoni <[email protected]>
Gerrit-Reviewer: Alex Rodoni <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Lars Volker <[email protected]>
Gerrit-Comment-Date: Fri, 13 Jul 2018 03:33:04 +0000
Gerrit-HasComments: Yes

Reply via email to