[CALCITE-6156] Add ENDSWITH, STARTSWITH functions (enabled in Postgres, Snowflake libraries) #3565
Conversation
tanclary
force-pushed
the
6156-startswith
branch
from
8624e51
to
78d0c9c
Compare
tanclary
changed the title
|
Going to fix the formatting issues soon. |
core/src/main/java/org/apache/calcite/sql/dialect/SnowflakeSqlDialect.java
Outdated
Show resolved
Hide resolved
| SPARK("s", "spark"); | ||
| SPARK("s", "spark"), | ||
| /** A collection of operators that are in Snowflake but not in standard SQL. */ | ||
| SNOWFLAKE("s", "snowflake"); |
There was a problem hiding this comment.
I could be wrong, but do these single chars point to the symbol we use in reference.md to indicate the supported libraries?
If so should they be distinct symbols for each or its okay to use the same, I see s is used for Spark, Snowflake and Spatial but 'MSSQL' = q and 'MYSQL' = m
There was a problem hiding this comment.
Just saw reference.md defines its own symbols for each library, just wanted to confirm what these chars are intended for.
There was a problem hiding this comment.
List should be alphabetical.
There was a problem hiding this comment.
Changed to f and re-ordered list.
| final String expected = "SELECT STARTSWITH(\"brand_name\" 'a')\n" | ||
| + "FROM \"foodmart\".\"product\""; | ||
| sql(bigQuerySql).withLibrary(SqlLibrary.BIG_QUERY).withSnowflake().ok(expected); | ||
| sql(snowflakeSql).withLibrary(SqlLibrary.SNOWFLAKE).withSnowflake().ok(expected); |
There was a problem hiding this comment.
It's not very important to have SQL in more than one source dialect. But it's worth testing what SQL is generated in Postgres, BigQuery and Snowflake dialects.
| * comply with Snowflake syntax. | ||
| */ | ||
| private static void unparseEndsStartsWith(final SqlWriter writer, final SqlCall call) { | ||
| final String name = call.getKind() == SqlKind.ENDS_WITH ? "ENDSWITH" : "STARTSWITH"; |
There was a problem hiding this comment.
This works, but it's a bit janky.
I would be better to map functions. If there is a call to ENDS_WITH, map it to a call to ENDSWITH. Then let the operator unparse itself.
A principled approach will allow us to scale the many functions x many dialects.
There was a problem hiding this comment.
I rewrote this bit in my latest commit.
tanclary
force-pushed
the
6156-startswith
branch
7 times, most recently
from
a2ca748
to
d46dc30
Compare
| f.checkBoolean("starts_with(x'1234', x'')", true); | ||
| f.checkBoolean("starts_with(x'', x'123456')", false); | ||
| f.checkBoolean("starts_with(x'', x'')", true); | ||
| final SqlOperatorFixture f0 = fixture(); |
There was a problem hiding this comment.
I wish there was a way to create tests for functions that are aliases (e.g. starts_with and startswith, length and char_length) that did not use copy-paste. Any ideas?
There was a problem hiding this comment.
Yeah I was thinking this too.
I filed https://issues.apache.org/jira/browse/CALCITE-6172 in the meantime while I think about it.
…s, Snowflake libraries)
tanclary
force-pushed
the
6156-startswith
branch
from
d46dc30
to
6dd3779
Compare
|
No description provided.