Jim Apple has posted comments on this change.

Change subject: IMPALA-889: Add support for ISO-SQL trim()
......................................................................


Patch Set 1:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/4474/1//COMMIT_MSG
Commit Message:

Line 10: Form #1: Impala UDF calling
What are the pros and cons of having two different forms?


Line 11:   Syntax: SELECT TRIM(where, characters, string_to_be_trimmed);
"select" isn't part of the syntax


PS1, Line 12:   where: enumerate values, available choices are: 
left/leading/right/trailing/both:
Please wrap lines at the red vertical line. I believe it's 70 characters.


PS1, Line 12: enumerate values
What is the purpose of this phrase in the sentence?


PS1, Line 16: option
What is "option"? Do you mean "where"?


PS1, Line 17: is
are, not is


Line 19:   Examples:
Please don't give examples until you explain, in prose, what it means to trim 
characters - does the order of the characters in "characters" matter? Does 
their multiplicity?

And don't just answer those particular questions - try to describe the function 
precisely.


PS1, Line 30:   Note: left and right are Impala keywords, they are not 
available in this syntax.
Does this match the standard?


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I4753c608b0b00569bf8c5e95b132df6df358e602
Gerrit-PatchSet: 1
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Youwei Wang <youwei.a.w...@intel.com>
Gerrit-Reviewer: Jim Apple <jbap...@cloudera.com>
Gerrit-HasComments: Yes

Reply via email to