Respond to review comments

core/src/main/java/org/apache/calcite/sql/dialect/SnowflakeSqlDialect.java

@@ -19,7 +19,6 @@
 import org.apache.calcite.avatica.util.Casing;
 import org.apache.calcite.sql.SqlCall;
 import org.apache.calcite.sql.SqlDialect;
-import org.apache.calcite.sql.SqlKind;
 import org.apache.calcite.sql.SqlWriter;
 import org.apache.calcite.sql.fun.SqlLibraryOperators;
 import org.apache.calcite.sql.parser.SqlParserPos;
@@ -43,31 +42,22 @@ public SnowflakeSqlDialect(Context context) {
 
   @Override public void unparseCall(final SqlWriter writer, final SqlCall call, final int leftPrec,
       final int rightPrec) {
-    switch(call.getKind()) {
+    switch (call.getKind()) {
     case ENDS_WITH:
+      SqlCall endsWithCall = SqlLibraryOperators.ENDSWITH
+          .createCall(SqlParserPos.ZERO, call.getOperandList());
+      super.unparseCall(writer, endsWithCall, leftPrec, rightPrec);
+      break;
     case STARTS_WITH:
-      unparseEndsStartsWith(writer, call);
+      SqlCall startsWithCall = SqlLibraryOperators.STARTSWITH
+          .createCall(SqlParserPos.ZERO, call.getOperandList());
+      super.unparseCall(writer, startsWithCall, leftPrec, rightPrec);
       break;
     default:
       super.unparseCall(writer, call, leftPrec, rightPrec);
     }
   }
 
-	/** 
-	 * Remove underscore for {@code STARTS_WITH} and {@code ENDS_WITH} operators to 
-	 * comply with Snowflake syntax.
-	 */
-  private static void unparseEndsStartsWith(final SqlWriter writer, final SqlCall call) {
-    final SqlCall 
-		if (call.getKind() == SqlKind.STARTS_WITH) {
-			SqlLibraryOperators.STARTSWITH.createCall(SqlParserPos.ZERO, call.getOperandList())
-		}
-    final SqlWriter.Frame frame = writer.startFunCall(name);
-    call.operand(0).unparse(writer, 0, 0);
-    call.operand(1).unparse(writer, 0, 0);
-    writer.endFunCall(frame);
-  }
-
   @Override public boolean supportsApproxCountDistinct() {
     return true;
   }

core/src/main/java/org/apache/calcite/sql/fun/SqlLibrary.java

@@ -71,11 +71,11 @@ public enum SqlLibrary {
   /** A collection of operators that are in PostgreSQL but not in standard
    * SQL. */
   POSTGRESQL("p", "postgresql"),
+  /** A collection of operators that are in Snowflake but not in standard SQL. */
+  SNOWFLAKE("f", "snowflake"),
   /** A collection of operators that are in Apache Spark but not in standard
    * SQL. */
-  SPARK("s", "spark"),
-  /** A collection of operators that are in Snowflake but not in standard SQL. */
-  SNOWFLAKE("s", "snowflake");
+  SPARK("s", "spark");
 
   /** Map from {@link Enum#name() name} and {@link #fun} to library. */
   public static final Map<String, SqlLibrary> MAP;
@@ -99,7 +99,7 @@ public List<SqlLibrary> children() {
     switch (this) {
     case ALL:
       return ImmutableList.of(BIG_QUERY, CALCITE, HIVE, MSSQL, MYSQL, ORACLE,
-          POSTGRESQL, SPARK);
+          POSTGRESQL, SNOWFLAKE, SPARK);
     default:
       return ImmutableList.of();
     }

core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java

@@ -6561,28 +6561,34 @@ private void checkLiteral2(String expression, String expected) {
    * <a href="https://issues.apache.org/jira/browse/CALCITE-6156">[CALCITE-6156]
    * Add ENDSWITH, STARTSWITH functions (enabled in Postgres, Snowflake libraries)</a>. */
   @Test void testSnowflakeStartsWith() {
-    final String bigQuerySql = 	"select starts_with(\"brand_name\", 'a')\n"
+    final String query = "select startswith(\"brand_name\", 'a')\n"
         + "from \"product\"";
-    final String snowflakeSql = "select startswith(\"brand_name\", 'a')\n"
-        + "from \"product\"";
-    final String expected = "SELECT STARTSWITH(\"brand_name\" 'a')\n"
+    final String expectedBigQuery = "SELECT STARTS_WITH(brand_name, 'a')\n"
+        + "FROM foodmart.product";
+    final String expectedPostgres = "SELECT STARTS_WITH(\"brand_name\", 'a')\n"
+        + "FROM \"foodmart\".\"product\"";
+    final String expectedSnowflake = "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);
+    sql(query).withLibrary(SqlLibrary.SNOWFLAKE).withBigQuery().ok(expectedBigQuery);
+    sql(query).withLibrary(SqlLibrary.SNOWFLAKE).withPostgresql().ok(expectedPostgres);
+    sql(query).withLibrary(SqlLibrary.SNOWFLAKE).withSnowflake().ok(expectedSnowflake);
   }
 
   /** Test case for
    * <a href="https://issues.apache.org/jira/browse/CALCITE-6156">[CALCITE-6156]
    * Add ENDSWITH, STARTSWITH functions (enabled in Postgres, Snowflake libraries)</a>. */
   @Test void testSnowflakeEndsWith() {
-    final String bigQuerySql = 	"select ends_with(\"brand_name\", 'a')\n"
+    final String query = "select endswith(\"brand_name\", 'a')\n"
         + "from \"product\"";
-    final String snowflakeSql = "select endswith(\"brand_name\", 'a')\n"
-        + "from \"product\"";
-    final String expected = "SELECT ENDSWITH(\"brand_name\" 'a')\n"
+    final String expectedBigQuery = "SELECT ENDS_WITH(brand_name, 'a')\n"
+        + "FROM foodmart.product";
+    final String expectedPostgres = "SELECT ENDS_WITH(\"brand_name\", 'a')\n"
+        + "FROM \"foodmart\".\"product\"";
+    final String expectedSnowflake = "SELECT ENDSWITH(\"brand_name\", 'a')\n"
         + "FROM \"foodmart\".\"product\"";
-    sql(bigQuerySql).withLibrary(SqlLibrary.BIG_QUERY).withSnowflake().ok(expected);
-    sql(snowflakeSql).withLibrary(SqlLibrary.SNOWFLAKE).withSnowflake().ok(expected);
+    sql(query).withLibrary(SqlLibrary.SNOWFLAKE).withBigQuery().ok(expectedBigQuery);
+    sql(query).withLibrary(SqlLibrary.SNOWFLAKE).withPostgresql().ok(expectedPostgres);
+    sql(query).withLibrary(SqlLibrary.SNOWFLAKE).withSnowflake().ok(expectedSnowflake);
   }
 
   @Test void testSubstringInSpark() {

core/src/test/java/org/apache/calcite/util/UtilTest.java

@@ -951,16 +951,16 @@ private List<Integer> makeConsList(int start, int end) {
 
     assertThat(SqlLibrary.expand(ImmutableList.of(a)),
         hasToString("[ALL, BIG_QUERY, CALCITE, HIVE, MSSQL, MYSQL, ORACLE, "
-            + "POSTGRESQL, SPARK]"));
+            + "POSTGRESQL, SNOWFLAKE, SPARK]"));
     assertThat(SqlLibrary.expand(ImmutableList.of(a, c)),
         hasToString("[ALL, BIG_QUERY, CALCITE, HIVE, MSSQL, MYSQL, ORACLE, "
-            + "POSTGRESQL, SPARK]"));
+            + "POSTGRESQL, SNOWFLAKE, SPARK]"));
     assertThat(SqlLibrary.expand(ImmutableList.of(c, a)),
         hasToString("[CALCITE, ALL, BIG_QUERY, HIVE, MSSQL, MYSQL, ORACLE, "
-            + "POSTGRESQL, SPARK]"));
+            + "POSTGRESQL, SNOWFLAKE, SPARK]"));
     assertThat(SqlLibrary.expand(ImmutableList.of(c, o, a)),
         hasToString("[CALCITE, ORACLE, ALL, BIG_QUERY, HIVE, MSSQL, MYSQL, "
-            + "POSTGRESQL, SPARK]"));
+            + "POSTGRESQL, SNOWFLAKE, SPARK]"));
     assertThat(SqlLibrary.expand(ImmutableList.of(o, c, o)),
         hasToString("[ORACLE, CALCITE]"));
 

site/_docs/reference.md

@@ -2628,13 +2628,13 @@ The 'C' (compatibility) column contains value:
 * '*' for all libraries,
 * 'b' for Google BigQuery ('fun=bigquery' in the connect string),
 * 'c' for Apache Calcite ('fun=calcite' in the connect string),
+* 'f' for Snowflake ('fun=snowflake' in the connect string),
 * 'h' for Apache Hive ('fun=hive' in the connect string),
 * 'm' for MySQL ('fun=mysql' in the connect string),
 * 'q' for Microsoft SQL Server ('fun=mssql' in the connect string),
 * 'o' for Oracle ('fun=oracle' in the connect string),
 * 'p' for PostgreSQL ('fun=postgresql' in the connect string),
-* 's' for Apache Spark ('fun=spark' in the connect string),
-* 'f' for Snowflake ('fun=snowflake' in the connect string).
+* 's' for Apache Spark ('fun=spark' in the connect string).
 
 One operator name may correspond to multiple SQL dialects, but with different
 semantics.
@@ -2826,7 +2826,7 @@ BigQuery's type system uses confusingly different names for types and functions:
 | s | SOUNDEX(string)                                | Returns the phonetic representation of *string*; return original *string* if *string* is encoded with multi-byte encoding such as UTF-8
 | m | SPACE(integer)                                 | Returns a string of *integer* spaces; returns an empty string if *integer* is less than 1
 | b | SPLIT(string [, delimiter ])                   | Returns the string array of *string* split at *delimiter* (if omitted, default is comma).  If the *string* is empty it returns an empty array, otherwise, if the *delimiter* is empty, it returns an array containing the original *string*.
-| f | STARTSWITH(string1, string2)                   | Returns whether *string2* is a prefix of*string1*
+| f | STARTSWITH(string1, string2)                   | Returns whether *string2* is a prefix of *string1*
 | b p | STARTS_WITH(string1, string2)                | Equivalent to `STARTSWITH(string1, string2)`
 | m | STRCMP(string, string)                         | Returns 0 if both of the strings are same and returns -1 when the first argument is smaller than the second and 1 when the second one is smaller than the first one
 | b p | STRPOS(string, substring)                    | Equivalent to `POSITION(substring IN string)`

testkit/src/main/java/org/apache/calcite/test/SqlOperatorTest.java

@@ -8953,6 +8953,7 @@ private void testCurrentDateFunc(Pair<String, Hook.Closeable> pair) {
       f.checkBoolean("ends_with(x'', x'123456')", false);
       f.checkBoolean("ends_with(x'', x'')", true);
     };
+    f0.forEachLibrary(list(SqlLibrary.BIG_QUERY, SqlLibrary.POSTGRESQL), consumer);
   }
 
   @Test void testSnowflakeEndsWithFunction() {