julianhyde commented on code in PR #3646:
URL: https://github.com/apache/calcite/pull/3646#discussion_r1465535681
##########
core/src/test/java/org/apache/calcite/rel/rel2sql/RelToSqlConverterTest.java:
##########
@@ -6677,12 +6677,37 @@ private void checkLiteral2(String expression, String
expected) {
* Add BITAND_AGG, BITOR_AGG functions (enabled in Snowflake library)</a>. */
@Test void testBitOrAgg() {
final String query = "select bit_or(\"product_id\")\n"
- + "from \"product\"";
+ + "from \"product\"";
final String expectedSnowflake = "SELECT BITOR_AGG(\"product_id\")\n"
- + "FROM \"foodmart\".\"product\"";
+ + "FROM \"foodmart\".\"product\"";
sql(query).withLibrary(SqlLibrary.SNOWFLAKE).withSnowflake().ok(expectedSnowflake);
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-6220";>[CALCITE-6220]
+ * Rewrite MIN/MAX(bool) as BOOL_AND/BOOL_OR for Postgres, Redshift</a>. */
+ @Test void testMaxMinOnBooleanColumn() {
+ final String query = "select max(\"brand_name\" = 'a'), "
+ + "min(\"brand_name\" = 'a'), "
+ + "min(\"brand_name\")\n"
+ + "from \"product\"";
+ final String expected = "SELECT MAX(\"brand_name\" = 'a'), "
+ + "MIN(\"brand_name\" = 'a'), "
+ + "MIN(\"brand_name\")\n"
+ + "FROM \"foodmart\".\"product\"";
+ final String expectedPostgres = "SELECT BOOL_OR(\"brand_name\" = 'a'), "
+ + "BOOL_AND(\"brand_name\" = 'a'), "
+ + "MIN(\"brand_name\")\n"
+ + "FROM \"foodmart\".\"product\"";
+ final String expectedRedshift = "SELECT BOOL_OR(\"brand_name\" = 'a'), "
+ + "BOOL_AND(\"brand_name\" = 'a'), "
+ + "MIN(\"brand_name\")\n"
+ + "FROM \"foodmart\".\"product\"";
+ sql(query).ok(expected);
+ sql(query).withPostgresql().ok(expectedPostgres);
Review Comment:
I would chain. And add a test for a 'neutral' dialect:
```
sql(query).ok(expected)
.withPostgresql().ok(expectedPostgres)
.withRedshift().ok(expectedRedshift)
.withMySQL().ok(expected);
```
I like how you tested on a non-BOOLEAN column
##########
core/src/main/java/org/apache/calcite/sql/SqlDialect.java:
##########
@@ -872,6 +872,13 @@ public SqlNode rewriteSingleValueExpr(SqlNode aggCall,
RelDataType relDataType)
return aggCall;
}
+ /** With x as BOOLEAN column, rewrite MAX(x)/MIN(x) as BOOL_OR(x)/BOOL_AND(x)
Review Comment:
Say what the method does. Nothing to do with BOOLEAN. Declarative "Rewrites"
not imperative "rewrite".
##########
core/src/main/java/org/apache/calcite/sql/dialect/PostgresqlSqlDialect.java:
##########
@@ -173,12 +173,16 @@ public PostgresqlSqlDialect(Context context) {
timeUnitNode.getParserPosition());
SqlFloorFunction.unparseDatetimeFunction(writer, call2, "DATE_TRUNC",
false);
break;
-
default:
super.unparseCall(writer, call, leftPrec, rightPrec);
}
}
+ @Override public SqlNode rewriteMaxMinExpr(SqlNode aggCall, RelDataType
relDataType) {
Review Comment:
Don't instantatiate a dialect. Unnecessary coupling. Use a helper method.
E.g. `static void unparseFetchUsingLimit()`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
