Skip to content

Commit

Permalink
[CALCITE-6220] Rewrite MIN/MAX(bool) as BOOL_AND/BOOL_OR for Postgres…
Browse files Browse the repository at this point in the history
…, Redshift
  • Loading branch information
tanclary committed Jan 24, 2024
1 parent 91a8ab8 commit d920051
Show file tree
Hide file tree
Showing 5 changed files with 61 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
import org.apache.calcite.sql.SqlUpdate;
import org.apache.calcite.sql.SqlUtil;
import org.apache.calcite.sql.fun.SqlInternalOperators;
import org.apache.calcite.sql.fun.SqlMinMaxAggFunction;
import org.apache.calcite.sql.fun.SqlSingleValueAggFunction;
import org.apache.calcite.sql.fun.SqlStdOperatorTable;
import org.apache.calcite.sql.parser.SqlParserPos;
Expand Down Expand Up @@ -577,6 +578,8 @@ protected Builder buildAggregate(Aggregate e, Builder builder,
RelDataType aggCallRelDataType = aggCall.getType();
if (aggCall.getAggregation() instanceof SqlSingleValueAggFunction) {
aggCallSqlNode = dialect.rewriteSingleValueExpr(aggCallSqlNode, aggCallRelDataType);
} else if (aggCall.getAggregation() instanceof SqlMinMaxAggFunction) {
aggCallSqlNode = dialect.rewriteMaxMinExpr(aggCallSqlNode, aggCallRelDataType);
}
addSelect(selectList, aggCallSqlNode, e.getRowType());
}
Expand Down
7 changes: 7 additions & 0 deletions core/src/main/java/org/apache/calcite/sql/SqlDialect.java
Original file line number Diff line number Diff line change
Expand Up @@ -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)
* for certain database variants (Postgres and Redshift, currently).
*/
public SqlNode rewriteMaxMinExpr(SqlNode aggCall, RelDataType relDataType) {
return aggCall;
}

/**
* Returns the SqlNode for emulating the null direction for the given field
* or <code>null</code> if no emulation needs to be done.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
RedshiftSqlDialect redshiftSqlDialect = new RedshiftSqlDialect(DEFAULT_CONTEXT);
return redshiftSqlDialect.rewriteMaxMinExpr(aggCall, relDataType);
}

@Override public boolean supportsGroupByLiteral() {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,15 @@
import org.apache.calcite.rel.type.RelDataType;
import org.apache.calcite.rel.type.RelDataTypeSystem;
import org.apache.calcite.rel.type.RelDataTypeSystemImpl;
import org.apache.calcite.sql.SqlBasicCall;
import org.apache.calcite.sql.SqlDataTypeSpec;
import org.apache.calcite.sql.SqlDialect;
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.SqlUserDefinedTypeNameSpec;
import org.apache.calcite.sql.SqlWriter;
import org.apache.calcite.sql.fun.SqlLibraryOperators;
import org.apache.calcite.sql.parser.SqlParserPos;
import org.apache.calcite.sql.type.SqlTypeName;

Expand Down Expand Up @@ -107,6 +111,21 @@ public RedshiftSqlDialect(Context context) {
SqlParserPos.ZERO);
}

@Override public SqlNode rewriteMaxMinExpr(SqlNode aggCall, RelDataType relDataType) {
// The behavior of this method depends on the argument type,
// and whether it is MIN/MAX
final SqlTypeName type = relDataType.getSqlTypeName();
final boolean isMax = aggCall.getKind() == SqlKind.MAX;
// If the type is BOOLEAN, create a new call to the correct operator
if (type == SqlTypeName.BOOLEAN) {
final SqlOperator op = isMax ? SqlLibraryOperators.BOOL_OR : SqlLibraryOperators.BOOL_AND;
final SqlNode operand = ((SqlBasicCall) aggCall).operand(0);
return op.createCall(SqlParserPos.ZERO, operand);
}
// Otherwise, just return as it arrived
return aggCall;
}

@Override public boolean supportsGroupByLiteral() {
return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
sql(query).withRedshift().ok(expectedRedshift);
}

/** 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>. */
Expand Down

0 comments on commit d920051

Please sign in to comment.