Skip to content

Add support for varbinary filters in Pinot#9180

Merged
hashhar merged 3 commits intotrinodb:masterfrom
elonazoulay:pinot_varbinary
Sep 10, 2021
Merged

Add support for varbinary filters in Pinot#9180
hashhar merged 3 commits intotrinodb:masterfrom
elonazoulay:pinot_varbinary

Conversation

@elonazoulay
Copy link
Copy Markdown
Member

Also includes fixes to real and double decoding for broker requests.

@hashhar
Copy link
Copy Markdown
Member

hashhar commented Sep 9, 2021

Related test failure:

Error:  io.trino.plugin.pinot.TestPinotIntegrationSmokeTest.testNullBehavior  Time elapsed: 9.491 s  <<< FAILURE!
java.lang.AssertionError: 
Plan does not match, expected [

- anyTree
    - node(ExchangeNode)
        - node(ProjectNode)
            - node(TableScanNode)

] but found [

Output[_col0]
│   Layout: [element_at:varchar]
│   _col0 := element_at
└─ RemoteExchange[GATHER]
   │   Layout: [element_at:varchar]
   └─ ScanFilterProject[table = pinot:PinotTableHandle{schemaName=default, tableName=alltypes, constraint={PinotColumnHandle{columnName=bytes_col, dataType=varbinary, returnNullOnEmptyGroup=true}=[ SortedRangeSet[type=varbinary, ranges=1, {[Slice{address=0, length=0}]}] ]}, limit=OptionalLong.empty, query=Optional.empty}, filterPredicate = (element_at("bool_array_col", BIGINT '1') = VARCHAR 'null')]
          Layout: [element_at:varchar]
          element_at := element_at("string_array_col", BIGINT '1')
          bool_array_col := PinotColumnHandle{columnName=bool_array_col, dataType=array(varchar), returnNullOnEmptyGroup=true}
          string_array_col := PinotColumnHandle{columnName=string_array_col, dataType=array(varchar), returnNullOnEmptyGroup=true}

]
	at io.trino.sql.planner.assertions.PlanAssert.assertPlan(PlanAssert.java:56)
	at io.trino.sql.planner.assertions.PlanAssert.assertPlan(PlanAssert.java:47)
	at io.trino.sql.planner.assertions.PlanAssert.assertPlan(PlanAssert.java:41)
	at io.trino.sql.query.QueryAssertions$QueryAssert.lambda$isNotFullyPushedDown$9(QueryAssertions.java:466)
	at io.trino.transaction.TransactionBuilder.lambda$execute$1(TransactionBuilder.java:122)
	at io.trino.transaction.TransactionBuilder.execute(TransactionBuilder.java:150)
	at io.trino.transaction.TransactionBuilder.execute(TransactionBuilder.java:121)
	at io.trino.sql.query.QueryAssertions$QueryAssert.isNotFullyPushedDown(QueryAssertions.java:464)
	at io.trino.sql.query.QueryAssertions$QueryAssert.isNotFullyPushedDown(QueryAssertions.java:450)
	at io.trino.plugin.pinot.TestPinotIntegrationSmokeTest.testNullBehavior(TestPinotIntegrationSmokeTest.java:753)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at org.testng.internal.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:104)
	at org.testng.internal.Invoker.invokeMethod(Invoker.java:645)
	at org.testng.internal.Invoker.invokeTestMethod(Invoker.java:851)
	at org.testng.internal.Invoker.invokeTestMethods(Invoker.java:1177)
	at org.testng.internal.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:129)
	at org.testng.internal.TestMethodWorker.run(TestMethodWorker.java:112)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:829)

@elonazoulay elonazoulay force-pushed the pinot_varbinary branch 2 times, most recently from 1e648a3 to 92eac7e Compare September 9, 2021 20:21
@findepi findepi changed the title Add support for varbinary filters Add support for varbinary filters in Pinot Sep 9, 2021
@findepi
Copy link
Copy Markdown
Member

findepi commented Sep 9, 2021

Can you consider adding "in Pinot" in commit messages?
they are short enough that this can be added without going over the limit

@elonazoulay elonazoulay force-pushed the pinot_varbinary branch 2 times, most recently from 1b9c105 to 5baabc6 Compare September 10, 2021 04:32
@elonazoulay elonazoulay requested a review from hashhar September 10, 2021 04:35
Copy link
Copy Markdown
Member

@hashhar hashhar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM % comments.

@elonazoulay elonazoulay requested a review from hashhar September 10, 2021 05:09
@hashhar hashhar merged commit 1676545 into trinodb:master Sep 10, 2021
@hashhar hashhar added this to the 362 milestone Sep 10, 2021
@hashhar hashhar mentioned this pull request Sep 10, 2021
13 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

3 participants