You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@calcite.apache.org by Alessandro Solimando <al...@gmail.com> on 2020/10/01 22:01:57 UTC
Re: [CassandraAdapter] selecting tuple elements always returns "null"
Hi Ruben,
I have totally missed SqlFunctions#structAccess, which is exactly what I
was supposed to be calling for accessing struct fields.
I wanted the DOT operator, but I have mistakenly used the ITEM one (that
is, "column[index]" instead of "column.index").
The corrected version of the UT is passing fine:
@Test void testTupleInnerValues() {
> CalciteAssert.that()
> .with(DTCASSANDRA)
> .query("select \"test_collections\".\"f_tuple\".\"1\","
> + "\"test_collections\".\"f_tuple\".\"2\","
> + "\"test_collections\".\"f_tuple\".\"3\""
> + " from \"test_collections\"")
> .returns("1=3000000000"
> + "; 2=30ff87"
> + "; 3=2015-05-03 11:30:54\n");
> }
>
For this reason I have resolved CALCITE-4293
<https://issues.apache.org/jira/browse/CALCITE-4293> as "Not a bug", but I
have filed CALCITE-4301
<https://issues.apache.org/jira/browse/CALCITE-4301> for
fixing the test that was disabled in the original PR I wrote for adding
support for complex types in Cassandra adapter.
Thanks a lot for the tip Ruben, it has put me back on the right track.
Best regards,
Alessandro
On Tue, 29 Sep 2020 at 00:08, Ruben Q L <ru...@gmail.com> wrote:
> Hi Alessandro,
>
> I have zero experience with CassandraAdapter, but just a naive comment from
> what you describe: could the issue be that it ends up calling
> SqlFunctions#item instead of SqlFunctions#structAccess [1] ?
>
> Best,
> Ruben
>
> [1]
>
> https://github.com/apache/calcite/blob/00f2dbacb7675b65fa19fe7cfe6f446687f92f8f/core/src/main/java/org/apache/calcite/runtime/SqlFunctions.java#L2939
>
> Le lun. 28 sept. 2020 à 21:25, Julian Hyde <jh...@gmail.com> a
> écrit :
>
> > Sorry my suggestion did not work.
> >
> > > On Sep 28, 2020, at 1:12 PM, Alessandro Solimando <
> > alessandro.solimando@gmail.com> wrote:
> > >
> > > 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 <jh...@apache.org> 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
> > >> <al...@gmail.com> 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
> > >>
> >
> >
>