Hi Julian,
thanks for your input.

I have quickly tried to return a "List" when building the cassandra
enumerator, the same test now fails with the following exception:

java.util.ArrayList cannot be cast to [Ljava.lang.Object;
> java.lang.ClassCastException: java.util.ArrayList cannot be cast to
> [Ljava.lang.Object;
> at Baz$1$1.current(Unknown Source)
> at
> org.apache.calcite.linq4j.Linq4j$EnumeratorIterator.next(Linq4j.java:684)
> at
> org.apache.calcite.avatica.util.IteratorCursor.next(IteratorCursor.java:46)
> at
> org.apache.calcite.avatica.AvaticaResultSet.next(AvaticaResultSet.java:217)
> at
> org.apache.calcite.test.CalciteAssert$ResultSetFormatter.resultSet(CalciteAssert.java:2082)
> at
> org.apache.calcite.test.CalciteAssert.lambda$checkResult$2(CalciteAssert.java:305)
> at
> org.apache.calcite.test.CalciteAssert.assertQuery(CalciteAssert.java:550)
> at
> org.apache.calcite.test.CalciteAssert$AssertQuery.lambda$returns$1(CalciteAssert.java:1535)
> at
> org.apache.calcite.test.CalciteAssert$AssertQuery.withConnection(CalciteAssert.java:1474)
> at
> org.apache.calcite.test.CalciteAssert$AssertQuery.returns(CalciteAssert.java:1533)
> at
> org.apache.calcite.test.CalciteAssert$AssertQuery.returns(CalciteAssert.java:1523)
> at
> org.apache.calcite.test.CalciteAssert$AssertQuery.returns(CalciteAssert.java:1486)
> at
> org.apache.calcite.test.CassandraAdapterDataTypesTest.testTupleInnerValues(CassandraAdapterDataTypesTest.java:210)
> at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
> at
> sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
> at
> sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
> at java.lang.reflect.Method.invoke(Method.java:498)
> at
> org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:675)
> at
> org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:125)
> at
> org.junit.jupiter.engine.extension.TimeoutInvocation.proceed(TimeoutInvocation.java:46)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:139)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:131)
> at
> org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestMethod(TimeoutExtension.java:81)
> at
> org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
> at
> org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:104)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:62)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:43)
> at
> org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:35)
> at
> org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
> at
> org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
> at
> org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeTestMethod$6(TestMethodTestDescriptor.java:202)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeTestMethod(TestMethodTestDescriptor.java:198)
> at
> org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:135)
> at
> org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:69)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:135)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
> at
> org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
> at
> org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:171)
> at
> org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.invokeAll(ForkJoinPoolHierarchicalTestExecutorService.java:115)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
> at
> org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
> at
> org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:171)
> at
> org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService.invokeAll(ForkJoinPoolHierarchicalTestExecutorService.java:115)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$5(NodeTestTask.java:139)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$7(NodeTestTask.java:125)
> at
> org.junit.platform.engine.support.hierarchical.Node.around(Node.java:135)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:123)
> at
> org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:122)
> at
> org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:80)
> at
> org.junit.platform.engine.support.hierarchical.ForkJoinPoolHierarchicalTestExecutorService$ExclusiveTask.compute(ForkJoinPoolHierarchicalTestExecutorService.java:171)
> at java.util.concurrent.RecursiveAction.exec(RecursiveAction.java:189)
> at java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:289)
> at
> java.util.concurrent.ForkJoinPool$WorkQueue.runTask(ForkJoinPool.java:1056)
> at java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1692)
> at
> java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:157)
> Suppressed: org.apache.calcite.util.TestUtil$ExtraInformation: With
> materializationsEnabled=false, limit=0, sql=select x['1'], x['2'], x['3']
> from (select "f_tuple" from "test_collections") as T(x)
> at org.apache.calcite.util.TestUtil.rethrow(TestUtil.java:252)
> at
> org.apache.calcite.test.CalciteAssert.assertQuery(CalciteAssert.java:566)
> ... 64 more


Somehow, the generated code still expects an "Object[]" (instead of a
"List"), so most probably the real cause is elsewhere, and what I am
observing (the call to "item" function with an array) is just a symptom.

In the meantime I have opened CALCITE-4293
<https://issues.apache.org/jira/browse/CALCITE-4293> to report the issue
caused by the current behaviour, and I will keep digging.

Best regards,
Alessandro

On Mon, 28 Sep 2020 at 02:36, Julian Hyde <[email protected]> wrote:

> I may be wrong, but I think that Calcite's Enumerable convention
> requires that SQL STRUCT and ARRAY types are mapped to a Java List.
> The item function expects that, and the adapter should comply. If the
> adapter is providing an array, that is wrong.
>
> On Sun, Sep 27, 2020 at 12:23 PM Alessandro Solimando
> <[email protected]> wrote:
> >
> > Hello all,
> > at the time I wrote the unit tests for the extended support for the
> missing
> > Cassandra data types, I have disabled the one at
> > CassandraAdapterDataTypesTest.java:192
> > <
> https://github.com/apache/calcite/blob/master/cassandra/src/test/java/org/apache/calcite/test/CassandraAdapterDataTypesTest.java#L192
> > > since
> > accessing a tuple element was returning a null value in place of the
> actual
> > non-null element.
> >
> > I recently had a chance to dig a bit to see why that's happening, and I
> > found what I consider the immediate cause for that (from
> > SqlFunctions.java:2511
> > <
> https://github.com/apache/calcite/blob/master/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java#L2511
> >
> > ):
> >
> >   /** Implements the {@code [ ... ]} operator on an object whose type is
> not
> > >    * known until runtime.
> > >    */
> >
> > public static Object item(Object object, Object index) {
> > >     if (object instanceof Map) {
> > >       return mapItem((Map) object, index);
> > >     }
> > >     if (object instanceof List && index instanceof Number) {
> > >       return arrayItem((List) object, ((Number) index).intValue());
> > >     }
> > >     return null;
> > >   }
> >
> >
> > This method ends up being called, and since the backing structure for
> > struct is an array (see CassandraEnumerator.java:114
> > <
> https://github.com/apache/calcite/blob/master/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraEnumerator.java#L114
> >),
> > none of the two if conditions match, and null is returned.
> >
> > This of course can be easily fixed, in what follows an example:
> >
> > public static Object item(Object object, Object index) {
> > >   if (object instanceof List && index instanceof Number) {
> > >      return arrayItem((List) object, ((Number) index).intValue());
> > >    }
> > >    if (object instanceof Object[]) { // it guarantees also that object
> !=
> > > null
> > >      if (index instanceof Number) {
> > >        return arrayItem(Arrays.asList(object), ((Number)
> > > index).intValue());
> > >      } else if (index instanceof String) {
> > >        Object[] array = (Object[]) object;
> > >        return mapItem(IntStream.range(0, array.length).boxed()
> > >           .collect(Collectors.toMap(i -> Integer.toString(i + 1), i ->
> > > array[i])), index);
> > >      }
> > >    }
> > >    return null;
> > >   }
> >
> >
> > Question is, is there anything which should be done differently on the
> > adapter end to prevent this from "item" to be called in the first place?
> Or
> > this is a legitimate situation and the "fix" is actually covering an
> > unhandled legal case?
> >
> > I have tried to look up in the codebase for something similar, but
> without
> > much luck, and I'd appreciate some guidance here.
> >
> > In what follows my analysis and a bit of context behind the status quo
> for
> > tuple/struct handling in Cassandra adapter:
> >
> > 1) I am accessing the struct elements via the "item" operator as this
> seems
> > the right way to do so according to the codebase examples I have seen
> > (JdbcTest for instance). Given that the "SqlTypeName" of the column of
> > "f_field" is "STRUCTURED", it ends up treated like a "MAP", rather than
> an
> > "ARRAY".
> >
> > "MAP" requires a "string" identifier and does not accept an integer to be
> > used with the "item" operator ("f_tuple"['1'] for instance). The
> identifier
> > it's the 1-based index of the element inside the structure (as dictated
> by
> > CassandraSchema.java:225
> > <
> https://github.com/apache/calcite/blob/master/cassandra/src/main/java/org/apache/calcite/adapter/cassandra/CassandraSchema.java#L225
> >)
> > since, unlike other structs, there is no field name here coming from the
> > datastax driver, hence we are left with the index inside the structure,
> > which seems reasonable.
> >
> > At first I tried to use an integer index, that's the error returned:
> >
> > java.sql.SQLException: Error while executing SQL "select x[1], x['2'],
> > > x['3'] from (select "f_tuple" from "test_collections") as T(x)": From
> line
> > > 1, column 8 to line 1, column 11: Cannot apply 'ITEM' to arguments of
> type
> > > 'ITEM(<RECORDTYPE(BIGINT 1, VARBINARY 2, TIMESTAMP(0) 3)>, <INTEGER>)'.
> > > Supported form(s): <ARRAY>[<INTEGER>]
> > > <MAP>[<VALUE>]
> >
> > (omitting the full stack trace as I don't think it's adding much value).
> >
> > 2) I also had second thoughts about having mapped Cassandra tuples to
> > struct, but there seems to be no alternatives allowing for an indexed
> > collection with heterogeneous types. What's your opinion, is it correct
> or
> > there is another way I could take?
> >
> > Finally, I'd either open a JIRA ticket for adding the extra behavior on
> > "item", or one for fixing the Cassandra adapter for tuples if in the end
> > that's the root cause of the issue.
> >
> > <https://github.com/asolimando/calcite/actions/runs/274415659>
> > In the first case I would already have a branch
> > <https://github.com/asolimando/calcite/tree/struct-values-index-access>
> which
> > might be ready to become a PR, for which tests are passing
> > <https://github.com/asolimando/calcite/actions/runs/274415659> already
> (CI
> > on forks are really great, thanks for introducing that!).
> >
> > Looking forward to hearing your thoughts.
> >
> > Best regards,
> > Alessandro
>

Reply via email to