You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by GitBox <gi...@apache.org> on 2022/12/21 18:53:53 UTC
[GitHub] [calcite] tanclary opened a new pull request, #3011: [CALCITE-5452] Add BigQuery LENGTH() as synonym for CHAR_LENGTH()
tanclary opened a new pull request, #3011:
URL: https://github.com/apache/calcite/pull/3011
Add LENGTH() as a library function as an alias for the standard CHAR_LENGTH(). Some dependencies for soon-to-be deprecated standard functions refactored to avoid null pointer exceptions as a result of circular dependencies between the standard and library operators. This decision was made with the help of @mkou .
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] julianhyde commented on pull request #3011: [CALCITE-5452] Add BigQuery LENGTH() as synonym for CHAR_LENGTH()
Posted by GitBox <gi...@apache.org>.
julianhyde commented on PR #3011:
URL: https://github.com/apache/calcite/pull/3011#issuecomment-1373070249
Can you change the commit message to start with the Jira case. Just like the PR subject.
The comments about deprecated functions above are useful, but would be better as part of the commit message.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] julianhyde closed pull request #3011: [CALCITE-5452] Add BigQuery LENGTH() as synonym for CHAR_LENGTH()
Posted by GitBox <gi...@apache.org>.
julianhyde closed pull request #3011: [CALCITE-5452] Add BigQuery LENGTH() as synonym for CHAR_LENGTH()
URL: https://github.com/apache/calcite/pull/3011
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [calcite] julianhyde commented on a diff in pull request #3011: [CALCITE-5452] Add BigQuery LENGTH() as synonym for CHAR_LENGTH()
Posted by GitBox <gi...@apache.org>.
julianhyde commented on code in PR #3011:
URL: https://github.com/apache/calcite/pull/3011#discussion_r1063052461
##########
babel/src/test/resources/sql/big-query.iq:
##########
@@ -2596,4 +2596,18 @@ FROM items;
!ok
+#####################################################################
Review Comment:
Is there a reason that you added at the end? If not, find a rationale to add it at a different position. Appends cause merge conflicts.
##########
babel/src/test/resources/sql/big-query.iq:
##########
@@ -2596,4 +2596,18 @@ FROM items;
!ok
+#####################################################################
Review Comment:
Yeah, it's my mistake. I knew this file was going to be large, so I should have created it in alphabetical order.
##########
site/_docs/reference.md:
##########
@@ -2634,6 +2635,7 @@ semantics.
| m | JSON_STORAGE_SIZE(jsonValue) | Returns the number of bytes used to store the binary representation of *jsonValue*
| b o | LEAST(expr [, expr ]* ) | Returns the least of the expressions
| b m p | LEFT(string, length) | Returns the leftmost *length* characters from the *string*
+| b | LENGTH(string) | Returns the number of characters in the *string*
Review Comment:
Do what CHARACTER_LENGTH did: "As CHAR_LENGTH(string)"
##########
testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java:
##########
@@ -6231,6 +6231,28 @@ void assertSubFunReturns(boolean binary, String s, int start,
f0.forEachLibrary(list(SqlLibrary.BIG_QUERY, SqlLibrary.ORACLE), consumer);
}
+ @Test void testLengthFunc() {
Review Comment:
I'd move this test next to `testCharLengthFunc` because it is so similar.
Use the `f0.forEachLibrary(list(SqlLibrary.BIG_QUERY), consumer);` pattern so that the test can be extended to other libraries in future.
##########
site/_docs/reference.md:
##########
@@ -1771,6 +1771,7 @@ period:
| {fn LENGTH(string)} | Returns the number of characters in a string
| {fn LOCATE(string1, string2 [, integer])} | Returns the position in *string2* of the first occurrence of *string1*. Searches from the beginning of *string2*, unless *integer* is specified.
| {fn LEFT(string, length)} | Returns the leftmost *length* characters from *string*
+| {fn LENGTH(string)} | Returns the number of characters in *string*
Review Comment:
I don't believe that you have added (or want to add) JDBC function syntax for LENGTH. remove this line
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@calcite.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org