zstan commented on code in PR #13543:
URL: https://github.com/apache/ignite/pull/13543#discussion_r3921681002


##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/TableFunctionScan.java:
##########
@@ -66,6 +68,22 @@ private Row convertToRow(Object rowContainer) {
                 + "] doesn't match defined columns number [" + 
rowType.getFieldCount() + "].");
         }
 
-        return rowFactory.create(rowArr);
+        return rowFactory.create(convertBinaryColumns(rowArr));
+    }
+
+    /** Converts binary column values to the internal representation. */
+    private Object[] convertBinaryColumns(Object[] row) {
+        Object[] convertedRow = row;
+
+        for (int i = 0; i < row.length; i++) {
+            if (row[i] instanceof byte[] && 
SqlTypeUtil.isBinary(rowType.getFieldList().get(i).getType())) {

Review Comment:
   1. we already have common converter : TypeUtils#toInternal - it need to be 
used
   2. explain why do you need row.clone() ? .toArray() - already produced a new 
array.



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/util/TypeUtils.java:
##########
@@ -309,7 +309,7 @@ public static <Row> Function<Row, Row> 
resultTypeConverter(ExecutionContext<Row>
 
     /** */
     private static Function<Object, Object> fieldConverter(ExecutionContext<?> 
ectx, RelDataType fieldType) {
-        Type storageType = ectx.getTypeFactory().getJavaClass(fieldType);
+        Type storageType = SqlTypeUtil.isBinary(fieldType) ? byte[].class : 
ectx.getTypeFactory().getJavaClass(fieldType);

Review Comment:
   plz explain why do we need changes in this class ? All changes need to be 
covered by tests, i miss them.



##########
modules/calcite/src/main/java/org/apache/ignite/internal/processors/query/calcite/exec/exp/IgniteTableFunction.java:
##########
@@ -132,4 +145,43 @@ private static void raiseValidationError(Method method, 
String errPostfix) {
 
         throw new IgniteSQLException("Unable to create table function for 
method '" + mtdSign + "'. " + errPostfix);
     }
+
+    /** Returns function parameters represented as SQL types where required. */
+    private static List<FunctionParameter> sqlParameters(Method method, 
List<FunctionParameter> functionParameters) {
+        var res = new ArrayList<>(functionParameters);
+
+        for (int i = 0; i < method.getParameterTypes().length; i++) {
+            if (method.getParameterTypes()[i] == byte[].class)
+                res.set(i, sqlBinaryParameter(res.get(i)));
+        }
+
+        return Collections.unmodifiableList(res);
+    }
+
+    /** Prevents Calcite from evaluating binary literals while deriving a 
table function row type. */
+    private static FunctionParameter sqlBinaryParameter(FunctionParameter 
delegate) {

Review Comment:
   looks like a hack as for me



-- 
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]

Reply via email to