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/11/22 21:36:35 UTC
[GitHub] [calcite] olivrlee opened a new pull request, #2980: [CALCITE-5389] Add STARTS_WITH() for BIG_QUERY
olivrlee opened a new pull request, #2980:
URL: https://github.com/apache/calcite/pull/2980
Adding `STARTS_WITH()` for `BIG_QUERY`
--
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] asfgit closed pull request #2980: [CALCITE-5389] Add STARTS_WITH() and ENDS_WITH() for BIG_QUERY
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #2980: [CALCITE-5389] Add STARTS_WITH() and ENDS_WITH() for BIG_QUERY
URL: https://github.com/apache/calcite/pull/2980
--
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] wnob commented on a diff in pull request #2980: [CALCITE-5389] Add STARTS_WITH() for BIG_QUERY
Posted by GitBox <gi...@apache.org>.
wnob commented on code in PR #2980:
URL: https://github.com/apache/calcite/pull/2980#discussion_r1029900371
##########
core/src/main/java/org/apache/calcite/sql/fun/SqlLibraryOperators.java:
##########
@@ -206,6 +206,14 @@ private SqlLibraryOperators() {
.andThen(SqlTypeTransforms.TO_VARYING), null,
OperandTypes.STRING, SqlFunctionCategory.STRING);
+ /** BigQuery's "SUBSTR(value1, value2)" function. */
Review Comment:
typo?
##########
babel/src/main/codegen/includes/parserImpls.ftl:
##########
@@ -178,6 +199,7 @@ SqlCreate SqlCreateTable(Span s, boolean replace) :
| < DATEPART: "DATEPART" >
| < NEGATE: "!" >
| < TILDE: "~" >
+| < STARTS_WITH: "STARTS_WITH" >
Review Comment:
nit: I believe this should be alphabetically sorted
##########
babel/src/test/resources/sql/big-query.iq:
##########
@@ -2519,4 +2519,31 @@ SELECT PARSE_TIMESTAMP("%c", "Thu Dec 25 07:30:00 2008") AS parsed;
!ok
!}
+#####################################################################
+#
+# STARTS_WITH(value1, value2)
+#
+# Takes two STRING or BYTES values. Returns TRUE if the second value is a prefix of the first.
+#
+# This function supports specifying collation.
+WITH items AS
+ (SELECT 'foo' as item
Review Comment:
These are all strings, but your function accepts `OperandTypes.STRING_STRING` which includes bytes objects as well. We should test those too (I think you need another overload in `SqlFunctions`).
I also think you may need to specify that both arguments must have the same type, and I'm not sure if `OperandTypes.STRING_STRING` accomplishes that. There should be a test to make sure you can't call `STARTS_WITH` passing a string and a bytes together. Double check that this is consistent with BigQuery by running a sample query in the BQ console, since their documentation isn't 100% clear.
--
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 #2980: [CALCITE-5389] Add STARTS_WITH() and ENDS_WITH() for BIG_QUERY
Posted by GitBox <gi...@apache.org>.
julianhyde commented on PR #2980:
URL: https://github.com/apache/calcite/pull/2980#issuecomment-1340012823
I'm reviewing now (as of 8b92384761).
I saw you merged with main. Best not to do that - or to squash/rebase. Just append commits, and you or the reviewer can squash/rebase after the reviews are done.
Also best to create a new branch for each PR. My personal preference is to start with the bug number, e.g. 5389-starts-with. Then you can use tab-completion in the shell to complete 'git fetch'.
--
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] olivrlee commented on a diff in pull request #2980: [CALCITE-5389] Add STARTS_WITH() for BIG_QUERY
Posted by GitBox <gi...@apache.org>.
olivrlee commented on code in PR #2980:
URL: https://github.com/apache/calcite/pull/2980#discussion_r1033956641
##########
babel/src/test/resources/sql/big-query.iq:
##########
@@ -2519,4 +2519,31 @@ SELECT PARSE_TIMESTAMP("%c", "Thu Dec 25 07:30:00 2008") AS parsed;
!ok
!}
+#####################################################################
+#
+# STARTS_WITH(value1, value2)
+#
+# Takes two STRING or BYTES values. Returns TRUE if the second value is a prefix of the first.
+#
+# This function supports specifying collation.
+WITH items AS
+ (SELECT 'foo' as item
Review Comment:
Update: BQ Console will throw an error if attempting to use string+bytes, so I've updated this PR to use `STRING_SAME_SAME`
--
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