IGNITE-29031 SQL Calcite: Support byte[] in UDF and UDTF parameters and results

[tkalkirill]

https://issues.apache.org/jira/browse/IGNITE-29031

[zstan]

First of all you need to follow the common process [1] i.e. make a PR from your own ignite mirror, not from origin = https://github.com/apache but from: https://github.com/tkalkirill/ignite

[1] https://cwiki.apache.org/confluence/spaces/IGNITE/pages/177047163/How+to+Contribute#HowtoContribute-GITworkflow

[tkalkirill]

@zstan Okay, subsequent tickets will do as described.

[zstan]

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.

[tkalkirill]

Tried to fix them.

[zstan]

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

[tkalkirill]

This was needed because the previous implementation was only for a byte[], now it is not needed, I got rid of it.

[zstan]

looks like a hack as for me

[tkalkirill]

This change is only needed when a byte[] table-function argument is a binary literal, for example binaryTableLength(x'010203'). Calcite tries to materialize the literal as byte[] during row-type inference, before the runtime converter is involved. Dynamic parameters and other non-literal expressions work without this workaround.

Would you prefer that we drop binary-literal support from this fix and address it separately if needed, or keep the workaround with an explanatory comment and a dedicated test?

[zstan]

I prefer to revise this approach, if it`s hard to implement now - let`s fill an additional issue, thanks !

[tkalkirill]

When testing with temporal types, it turned out that this is currently necessary and does not work without it.

[zstan]

I disagree that this PR need to cover only byte[], what about temporal types ? BO, appropriate tests need to be appended.

[tkalkirill]

Added quite a lot of tests.

[zstan]

        @QuerySqlFunction
        public static java.util.Date kill1() {
            return java.sql.Date.valueOf("2020-01-01");
        }

and further call:

assertQuery("SELECT kill1()")
            .resultSize(1)
            .check();

will raise ClassCastException, while

        @QuerySqlFunction
        public static java.sql.Date kill2() {
            return java.sql.Date.valueOf("2020-01-01");
        }

passes, wdyt ?

[zstan]

failed:

        assertQuery("SELECT EXTRACT(YEAR FROM D) FROM f()")
            .resultSize(1)
            .check();

        @QuerySqlTableFunction(columnTypes =
            {java.util.Date.class}, columnNames = {"D"})
        public static Iterable<Object[]> f() { // kill
            return Collections.singletonList(new Object[] {java.sql.Date.valueOf("2020-01-01")});
        }

[zstan]

UDF like : static boolean f(char c)
and further SELECT f('') ->
literal CHAR(0) passes SqlTypeAssignmentRule (CHAR assignable from CHAR,
precision ignored) so no CAST is added;
TypeUtils.fromInternal(root, "", char.class) ->
StringIndexOutOfBoundsException

[zstan]

dead code

[zstan]

dead code

[zstan]

need to be moved into abstract class cause IgniteScalarFunction#getParameters do the same as i can see

[zstan]

Also near passed on master and failed in current branch, seems regression:

    @Test
    public void testCollectionColumnTableFunctions() {
        client.getOrCreateCache(new
            CacheConfiguration<>("collection-column-table-functions")
            .setSqlSchema("PUBLIC")
            .setSqlFunctionClasses(CollectionColumnFunctionsLibrary.class));

        // ArrayList column -> ARRAY.
        assertQuery("SELECT ID, CARDINALITY(VALS), VALS[1] FROM " +
            "listColumn()")
            .returns(1, 2, 10)
            .returns(2, 3, 30)
            .check();

        assertQuery("SELECT t.ID, v.VAL FROM listColumn() t, " +
            " UNNEST(t.VALS) v(VAL) ORDER BY t.ID, v.VAL")
            .returns(1, 10)
            .returns(1, 20)
            .returns(2, 30)
            .returns(2, 40)
            .returns(2, 50)
            .check();

        // String[] column -> MULTISET.
        assertQuery("SELECT ID, CARDINALITY(NAMES) FROM arrayColumn()")
            .returns(1, 1)
            .returns(2, 2)
            .check();

        assertQuery("SELECT t.ID, n.NAME FROM arrayColumn() t, " +
            " UNNEST(t.NAMES) n(NAME) ORDER BY t.ID, n.NAME")
            .returns(1, "a")
            .returns(2, "b")
            .returns(2, "c")
            .check();
    }

    /** */
    public static final class CollectionColumnFunctionsLibrary {
        /** */
        private CollectionColumnFunctionsLibrary() {
            // No-op.
        }

        /** Returns rows with a {@link List}-typed column. */
        @QuerySqlTableFunction(columnTypes = {int.class, ArrayList.class},
            columnNames = {"ID", "VALS"})
        public static Iterable<Collection<?>> listColumn() {
            return Arrays.asList(
                Arrays.asList(1, new ArrayList<>(Arrays.asList(10, 20))),
                Arrays.asList(2, new ArrayList<>(Arrays.asList(30, 40,
                    50)))
            );
        }

        /** Returns rows with an array-typed column. */
        @QuerySqlTableFunction(columnTypes = {int.class, String[].class},
            columnNames = {"ID", "NAMES"})
        public static Iterable<Collection<?>> arrayColumn() {
            return Arrays.asList(
                Arrays.asList(1, Arrays.asList("a")),
                Arrays.asList(2, Arrays.asList("b", "c"))
            );
        }
    }