Copilot commented on code in PR #4855:
URL: https://github.com/apache/calcite/pull/4855#discussion_r3006313732
##########
testkit/src/main/java/org/apache/calcite/util/Smalls.java:
##########
@@ -1493,6 +1494,23 @@ public static ByteString eval(String s) {
}
}
+ /** User-defined function with return type byte[]. */
+ public static class ByteArrayFunction {
+ public static byte[] eval(String s) {
+ if (s == null) {
+ return null;
+ }
+ return s.getBytes(StandardCharsets.UTF_8);
+ }
+ }
+
+ /** User-defined function with parameter type byte[]. */
+ public static class ByteArrayLengthFunction {
+ public static int eval(byte[] bytes) {
Review Comment:
ByteArrayLengthFunction.eval does not handle null input; if a SQL VARBINARY
argument is NULL (or conversion yields null), this will throw a
NullPointerException. Consider changing the signature to return a boxed Integer
and return null when bytes is null (or otherwise define/implement the desired
NULL semantics).
```suggestion
public static @Nullable Integer eval(@Nullable byte[] bytes) {
if (bytes == null) {
return null;
}
```
##########
core/src/test/java/org/apache/calcite/test/UdfTest.java:
##########
@@ -1107,6 +1119,29 @@ private static CalciteAssert.AssertThat
withBadUdf(Class<?> clazz) {
withUdf().query(sql2).returns("C=true\n");
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7187";>[CALCITE-7187]
+ * Java UDF byte/Byte array can not mapping to VARBINARY</a>. */
+ @Test void testByteArrayDirectComparison() {
+ final String testString = "test";
+ final String testHex = "74657374";
+
+ final String sql = "values \"adhoc\".bytearray('" + testString + "')";
+ withUdf().query(sql).typeIs("[EXPR$0 VARBINARY]");
+
+ final String sql2 = "select \"adhoc\".bytearray(cast('" + testString
+ + "' as varchar)) = x'" + testHex + "' as C\n";
+ withUdf().query(sql2).returns("C=true\n");
+ }
+
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7187";>[CALCITE-7187]
+ * Java UDF byte/Byte array can not mapping to VARBINARY</a>. */
Review Comment:
Same grammar issue in the linked issue description text ("can not mapping");
please change to "cannot be mapped" for clarity.
##########
core/src/test/java/org/apache/calcite/test/UdfTest.java:
##########
@@ -1107,6 +1119,29 @@ private static CalciteAssert.AssertThat
withBadUdf(Class<?> clazz) {
withUdf().query(sql2).returns("C=true\n");
}
+ /** Test case for
+ * <a
href="https://issues.apache.org/jira/browse/CALCITE-7187";>[CALCITE-7187]
+ * Java UDF byte/Byte array can not mapping to VARBINARY</a>. */
Review Comment:
The linked issue description text has grammatical errors ("can not
mapping"). Please update it (e.g., "cannot be mapped") to keep test
documentation clear and searchable.
##########
site/_docs/reference.md:
##########
@@ -3426,6 +3426,10 @@ can specify the name and optionality of each parameter
using the
[Parameter]({{ site.apiRoot
}}/org/apache/calcite/linq4j/function/Parameter.html)
annotation.
+For Java UDFs, `byte[]` and `ByteString` are supported Java representations of
+SQL `VARBINARY` values for parameters and return types. Boxed byte arrays
+(`Byte[]`) are not supported.
Review Comment:
The doc mentions `ByteString` without qualification; since there are
multiple ByteString types in the Java ecosystem, consider using the fully
qualified name (org.apache.calcite.avatica.util.ByteString) or linking to the
API docs to avoid ambiguity. Also, the sentence about `Byte[]` could be misread
as "not supported at all"; if the intent is "not supported as a VARBINARY
representation", consider stating that explicitly.
```suggestion
For Java UDFs, `byte[]` and [`ByteString`]({{ site.apiRoot
}}/org/apache/calcite/avatica/util/ByteString.html) are supported Java
representations of
SQL `VARBINARY` values for parameters and return types. Boxed byte arrays
(`Byte[]`) are not supported as Java representations of SQL `VARBINARY`
parameters or return types.
```
--
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]
