Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions babel/src/test/resources/sql/big-query.iq
Original file line number Diff line number Diff line change
Expand Up @@ -1012,9 +1012,9 @@ FROM

SELECT
Comment thread
tanclary marked this conversation as resolved.
email,
REGEXP_CONTAINS(email, '^([\w.+-]+@foo\.com|[\w.+-]+@bar\.org)$')
REGEXP_CONTAINS(email, '^([\w.+-]+@foo\.com|[\w.+-]+@bar\.org)\s+$')
AS valid_email_address,
REGEXP_CONTAINS(email, '^[\w.+-]+@foo\.com|[\w.+-]+@bar\.org$')
REGEXP_CONTAINS(email, '^[\w.+-]+@foo\.com|[\w.+-]+@bar\.org\s+$')
AS without_parentheses
FROM
(SELECT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlOperatorBinding;
import org.apache.calcite.sql.type.SqlTypeUtil;
import org.apache.calcite.sql.validate.SqlValidatorUtil;

import static java.util.Objects.requireNonNull;

Expand All @@ -38,6 +39,10 @@ public SqlArrayValueConstructor() {
opBinding.getTypeFactory(),
opBinding.collectOperandTypes());
requireNonNull(type, "inferred array element type");

// explicit cast elements to component type if they are not same
SqlValidatorUtil.adjustTypeForArrayConstructor(type, opBinding);

return SqlTypeUtil.createArrayType(
opBinding.getTypeFactory(), type, false);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import org.apache.calcite.sql.type.SqlTypeTransforms;
import org.apache.calcite.sql.type.SqlTypeUtil;
import org.apache.calcite.sql.validate.SqlValidator;
import org.apache.calcite.sql.validate.SqlValidatorUtil;
import org.apache.calcite.util.Litmus;
import org.apache.calcite.util.Optionality;
import org.apache.calcite.util.Static;
Expand Down Expand Up @@ -1060,6 +1061,10 @@ private static RelDataType arrayReturnType(SqlOperatorBinding opBinding) {
: opBinding.getTypeFactory().createUnknownType();
}
requireNonNull(type, "inferred array element type");

// explicit cast elements to component type if they are not same
SqlValidatorUtil.adjustTypeForArrayConstructor(type, opBinding);

return SqlTypeUtil.createArrayType(opBinding.getTypeFactory(), type, false);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import org.apache.calcite.sql.SqlKind;
import org.apache.calcite.sql.SqlOperatorBinding;
import org.apache.calcite.sql.type.SqlTypeUtil;
import org.apache.calcite.sql.validate.SqlValidatorUtil;
import org.apache.calcite.util.Pair;
import org.apache.calcite.util.Util;

Expand All @@ -44,10 +45,15 @@ public SqlMapValueConstructor() {
super("MAP", SqlKind.MAP_VALUE_CONSTRUCTOR);
}

@SuppressWarnings("argument.type.incompatible")
@Override public RelDataType inferReturnType(SqlOperatorBinding opBinding) {
Pair<@Nullable RelDataType, @Nullable RelDataType> type =
getComponentTypes(
opBinding.getTypeFactory(), opBinding.collectOperandTypes());

// explicit cast elements to component type if they are not same
SqlValidatorUtil.adjustTypeForMapConstructor(type, opBinding);

return SqlTypeUtil.createMapType(
opBinding.getTypeFactory(),
requireNonNull(type.left, "inferred key type"),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import org.apache.calcite.schema.impl.AbstractSchema;
import org.apache.calcite.schema.impl.AbstractTable;
import org.apache.calcite.sql.SqlCall;
import org.apache.calcite.sql.SqlCallBinding;
import org.apache.calcite.sql.SqlDataTypeSpec;
import org.apache.calcite.sql.SqlDynamicParam;
import org.apache.calcite.sql.SqlFunctionCategory;
Expand All @@ -48,6 +49,7 @@
import org.apache.calcite.sql.SqlNode;
import org.apache.calcite.sql.SqlNodeList;
import org.apache.calcite.sql.SqlOperator;
import org.apache.calcite.sql.SqlOperatorBinding;
import org.apache.calcite.sql.SqlOperatorTable;
import org.apache.calcite.sql.SqlSelect;
import org.apache.calcite.sql.SqlSyntax;
Expand Down Expand Up @@ -1309,6 +1311,93 @@ public static boolean isMeasure(SqlNode selectItem) {
return null;
}

/**
* When the array element does not equal the array component type, make explicit casting.
*
* @param componentType derived array component type
* @param opBinding description of call
*/
public static void adjustTypeForArrayConstructor(
RelDataType componentType, SqlOperatorBinding opBinding) {
if (opBinding instanceof SqlCallBinding) {
requireNonNull(componentType, "array component type");
adjustTypeForMultisetConstructor(
componentType, componentType, (SqlCallBinding) opBinding);
}
}

/**
* When the map key or value does not equal the map component key type or value type,
* make explicit casting.
*
* @param componentType derived map pair component type
* @param opBinding description of call
*/
public static void adjustTypeForMapConstructor(
Pair<RelDataType, RelDataType> componentType, SqlOperatorBinding opBinding) {
if (opBinding instanceof SqlCallBinding) {
requireNonNull(componentType.getKey(), "map key type");
requireNonNull(componentType.getValue(), "map value type");
adjustTypeForMultisetConstructor(
componentType.getKey(), componentType.getValue(), (SqlCallBinding) opBinding);
}
}

/**
* Adjusts the types for operands in a SqlCallBinding during the construction of a sql collection
* type such as Array or Map. This method iterates from the operands of a {@link SqlCall}
* obtained from the provided {@link SqlCallBinding}.
* It modifies each operand to match the specified 'evenType' or 'oddType' depending on whether
* the operand's position is even or odd, respectively. The type adjustment is performed by
* explicitly casting each operand to the desired type if the existing operand type does not
* match the desired type without considering field names.
* If we adjust array, we should set 'evenType' and 'oddType' to same desired type,
* if we adjust map, we should set 'evenType' as map key type and 'oddType' as map value type.
*
* <p>For example, if the operand types are {@code [INT, STRING, INT, STRING]}
* with {@code evenType} as {@code BOOLEAN} and {@code oddType} as {@code DOUBLE},
* after executing this method, the types should be {@code [BOOLEAN, DOUBLE, BOOLEAN, DOUBLE]},
* then the corresponding operands are cast to these types.
*
* @param evenType the {@link RelDataType} to which the operands at even positions should be cast
* @param oddType the {@link RelDataType} to which the operands at odd positions should be cast
* @param sqlCallBinding the {@link SqlCallBinding} containing the operands to be adjusted
*/
private static void adjustTypeForMultisetConstructor(
RelDataType evenType, RelDataType oddType, SqlCallBinding sqlCallBinding) {
SqlCall call = sqlCallBinding.getCall();
List<RelDataType> operandTypes = sqlCallBinding.collectOperandTypes();
List<SqlNode> operands = call.getOperandList();
RelDataType elementType;
for (int i = 0; i < operands.size(); i++) {
if (i % 2 == 0) {
elementType = evenType;
Comment thread
tanclary marked this conversation as resolved.
} else {
elementType = oddType;
}
if (!operandTypes.get(i).equalsSansFieldNames(elementType)) {
call.setOperand(i, castTo(operands.get(i), elementType));
}
}
}

/**
* Creates a CAST operation to cast a given {@link SqlNode} to a specified {@link RelDataType}.
* This method uses the {@link SqlStdOperatorTable#CAST} operator to create a new {@link SqlCall}
* node representing a CAST operation. The original 'node' is cast to the desired 'type',
* preserving the nullability of the 'type'.
*
* @param node the {@link SqlNode} which is to be cast
* @param type the target {@link RelDataType} to which 'node' should be cast
* @return a new {@link SqlNode} representing the CAST operation
*/
private static SqlNode castTo(SqlNode node, RelDataType type) {
Comment thread
tanclary marked this conversation as resolved.
return SqlStdOperatorTable.CAST.createCall(
SqlParserPos.ZERO,
node,
SqlTypeUtil.convertTypeToSpec(type).withNullable(type.isNullable()));
}

//~ Inner Classes ----------------------------------------------------------

/**
Expand Down
3 changes: 1 addition & 2 deletions core/src/test/java/org/apache/calcite/test/JdbcTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -8144,10 +8144,9 @@ private void checkGetTimestamp(Connection con) throws SQLException {
* ClassCastException retrieving from ARRAY that has mixed INTEGER and DECIMAL
* elements</a>. */
@Test void testIntAndBigDecimalInArray() {
// Result should be "EXPR$0=[1, 1.1]\n"; [CALCITE-4850] logged.
CalciteAssert.that()
.query("select array[1, 1.1]")
.returns("EXPR$0=[0E+1, 1.1]\n");

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@tanclary thanks for patient reviewing! btw, I think we can safely close CALCITE-4850. The result has been corrected.

.returns("EXPR$0=[1, 1.1]\n");
}

/** Test case for
Expand Down
12 changes: 6 additions & 6 deletions core/src/test/resources/sql/misc.iq
Original file line number Diff line number Diff line change
Expand Up @@ -2179,12 +2179,12 @@ select array[1,null,2] as a from (values (1));

values array['a',null,'bcd'],
array['efgh'];
+----------------+
| EXPR$0 |
+----------------+
| [a, null, bcd] |
| [efgh] |
+----------------+
+------------------+
| EXPR$0 |
+------------------+
| [a , null, bcd] |

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is there an extra space?

@taoran92 taoran92 Sep 18, 2023 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JiajunBernoulli thanks for reviewing. this is a import change in this PR.

if we have array('A', 'AB'), the derived component type is char(2), the old behavior not cast 'A' to char(2) in runtime to match derived component type. The correct behavior let 'A' match the char(2), 'A' cast to char(2), then it will fill with extra spaces by default in calcite.

This is calcite and sql standard limitation. Of course, this does look weird, actually calcite has other ways to remove spaces. We can activate SqlConformanceEnum.PRAGMATIC_2003 to let shouldConvertRaggedUnionTypesToVarying be true, it will change array/map char type to varchar type to skip these spaces.

More details can be found in javadoc https://github.com/apache/calcite/blob/d9dd3ac8a9f695e111a0a5e77f45b61b90f4b5b6/core/src/main/java/org/apache/calcite/sql/validate/SqlConformance.java#L465C7-L465C7

I also have added some cases in the PR to show this difference.

@taoran92 taoran92 Sep 19, 2023 •

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JiajunBernoulli hi, Jiajun. If you have time, could you help to review it again? thanks

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, Thank you for your clear explanation.

| [efgh] |
+------------------+
(2 rows)

!ok
Expand Down
Loading