twalthr commented on code in PR #26862:
URL: https://github.com/apache/flink/pull/26862#discussion_r2254674971
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/BinaryToBinaryCastRule.java:
##########
@@ -43,37 +43,38 @@ private BinaryToBinaryCastRule() {
.build());
}
- /* Example generated code for BINARY(2):
-
- // legacy behavior
- ((byte[])(inputValue))
-
- // new behavior
- ((((byte[])(inputValue)).length == 2) ? (((byte[])(inputValue))) :
(java.util.Arrays.copyOf(((byte[])(inputValue)), 2)))
-
- */
-
+ /**
+ * Generates code for casting between BINARY and VARBINARY types.
+ *
+ * <p>For VARBINARY targets: preserves original length if it fits within
the target constraint,
+ * otherwise truncates to target length.
+ *
+ * <p>For BINARY targets: pads shorter inputs to exact target length,
truncates longer inputs.
+ *
+ * <p>Example generated code for {@code CAST(input AS VARBINARY(4))}:
+ *
+ * <pre>
+ * ((input.length <= 4) ? input : java.util.Arrays.copyOf(input, 4))
Review Comment:
the down side of this approach is that we always perform a length check
during even if it is not necessary
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/BinaryToBinaryCastRule.java:
##########
@@ -43,37 +43,38 @@ private BinaryToBinaryCastRule() {
.build());
}
- /* Example generated code for BINARY(2):
-
- // legacy behavior
Review Comment:
still document the legacy behavior
##########
flink-table/flink-table-planner/src/main/java/org/apache/flink/table/planner/functions/casting/BinaryToBinaryCastRule.java:
##########
@@ -43,37 +43,38 @@ private BinaryToBinaryCastRule() {
.build());
}
- /* Example generated code for BINARY(2):
-
- // legacy behavior
- ((byte[])(inputValue))
-
- // new behavior
- ((((byte[])(inputValue)).length == 2) ? (((byte[])(inputValue))) :
(java.util.Arrays.copyOf(((byte[])(inputValue)), 2)))
-
- */
-
+ /**
+ * Generates code for casting between BINARY and VARBINARY types.
+ *
+ * <p>For VARBINARY targets: preserves original length if it fits within
the target constraint,
+ * otherwise truncates to target length.
+ *
+ * <p>For BINARY targets: pads shorter inputs to exact target length,
truncates longer inputs.
+ *
+ * <p>Example generated code for {@code CAST(input AS VARBINARY(4))}:
+ *
+ * <pre>
+ * ((input.length <= 4) ? input : java.util.Arrays.copyOf(input, 4))
+ * </pre>
+ */
@Override
public String generateExpression(
CodeGeneratorCastRule.Context context,
String inputTerm,
LogicalType inputLogicalType,
LogicalType targetLogicalType) {
- int inputLength = LogicalTypeChecks.getLength(inputLogicalType);
- int targetLength = LogicalTypeChecks.getLength(targetLogicalType);
+ final int targetLength =
LogicalTypeChecks.getLength(targetLogicalType);
Review Comment:
Is this rule now in sync with character string rule?
##########
flink-table/flink-table-planner/src/test/java/org/apache/flink/table/planner/functions/CastFunctionITCase.java:
##########
@@ -649,9 +649,9 @@ private static List<TestSetSpec> allTypesBasic() {
BIGINT(), DEFAULT_POSITIVE_BIGINT, (double)
DEFAULT_POSITIVE_BIGINT)
.fromCase(
BIGINT(), DEFAULT_NEGATIVE_BIGINT, (double)
DEFAULT_NEGATIVE_BIGINT)
- .fromCase(FLOAT(), DEFAULT_POSITIVE_FLOAT, 123.456d)
- .fromCase(FLOAT(), DEFAULT_NEGATIVE_FLOAT, -123.456)
- .fromCase(FLOAT(), 9234567891.12, 9234567891.12d)
+ .fromCase(FLOAT(), DEFAULT_POSITIVE_FLOAT,
123.45600128173828)
+ .fromCase(FLOAT(), DEFAULT_NEGATIVE_FLOAT,
-123.45600128173828)
+ .fromCase(FLOAT(), 9234567891.12, 9.234568192E9)
.fromCase(DOUBLE(), DEFAULT_POSITIVE_DOUBLE,
DEFAULT_POSITIVE_DOUBLE)
Review Comment:
I was just about to ask the same.
--
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]
