Pushdown DistinctLimitNode in Pinot Connector#14863
Conversation
|
@agrawaldevesh Could you help review this PR, thanks! |
...o-pinot-toolkit/src/test/java/com/facebook/presto/pinot/query/TestPinotPlanOptimizerSql.java
Outdated
Show resolved
Hide resolved
381af86 to
9576843
Compare
agrawaldevesh
left a comment
There was a problem hiding this comment.
Still trying to understand the implementation better -- some very high level questions inline.
There was a problem hiding this comment.
I think I have forgotten your old Pinot-SQL PR :-) -- but didn't we have a special derived class for the SQL generation ? Instead of using if useSqlSyntax
There was a problem hiding this comment.
This is for pushing down a new PlanNode: DistinctLimitPlanNode.
Query SELECT DISTINCT colA will be parsed to an AggregationPlanNode,
Query SELECT DISTINCT colA LIMIT X will be parsed to a DistinctLimitPlanNode.
For PQL side, since we still need to use the hidden column count(*), so I use withAggregation(...) for setting.
For SQL side, use withDistinctLimit to set groupbycolumns and limit.
There was a problem hiding this comment.
PinotQueryGeneratorContext.withAggregation has some special handling for the case when distinctCount is already pushed down. (Search for PINOT_DISTINCT_COUNT_FUNCTION_NAME). Can you make sure that this doesn't conflict with that. Thanks
presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotQueryGenerator.java
Outdated
Show resolved
Hide resolved
presto-pinot-toolkit/src/test/java/com/facebook/presto/pinot/query/TestPinotQueryGenerator.java
Outdated
Show resolved
Hide resolved
...-pinot-toolkit/src/test/java/com/facebook/presto/pinot/query/TestPinotQueryGeneratorSql.java
Outdated
Show resolved
Hide resolved
2f904ba to
f26ce01
Compare
agrawaldevesh
left a comment
There was a problem hiding this comment.
Getting very close
There was a problem hiding this comment.
PinotQueryGeneratorContext.withAggregation has some special handling for the case when distinctCount is already pushed down. (Search for PINOT_DISTINCT_COUNT_FUNCTION_NAME). Can you make sure that this doesn't conflict with that. Thanks
presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotQueryGenerator.java
Outdated
Show resolved
Hide resolved
...-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotQueryGeneratorContext.java
Outdated
Show resolved
Hide resolved
...-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotQueryGeneratorContext.java
Outdated
Show resolved
Hide resolved
f26ce01 to
81e5af0
Compare
|
distinctCount pushdown won't be triggered by this. It's on top of count on a column with distinct mark. |
presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotQueryGenerator.java
Outdated
Show resolved
Hide resolved
...o-pinot-toolkit/src/test/java/com/facebook/presto/pinot/query/TestPinotPlanOptimizerSql.java
Outdated
Show resolved
Hide resolved
...o-pinot-toolkit/src/test/java/com/facebook/presto/pinot/query/TestPinotPlanOptimizerSql.java
Outdated
Show resolved
Hide resolved
presto-pinot-toolkit/src/test/java/com/facebook/presto/pinot/query/TestPinotQueryGenerator.java
Outdated
Show resolved
Hide resolved
...-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotQueryGeneratorContext.java
Outdated
Show resolved
Hide resolved
81e5af0 to
02431aa
Compare
Thanks @agrawaldevesh |
02431aa to
bc25ce1
Compare
presto-pinot-toolkit/src/main/java/com/facebook/presto/pinot/query/PinotQueryGenerator.java
Outdated
Show resolved
Hide resolved
29047db to
cd30cbc
Compare
cd30cbc to
4d05ea7
Compare
Push down DistinctLimitNode to Pinot Query.
For Presto query:
SELECT DISTINCT flightnum FROM airlinestats LIMIT 10,We will pushdown below query to Pinot:
SELECT FlightNum FROM airlineStats GROUP BY FlightNum LIMIT 10.SELECT count(*) FROM airlineStats GROUP BY FlightNum TOP 10.Below is the generated query plan for SQL mode.
Below is the generated query plan for PQL mode.