-
Notifications
You must be signed in to change notification settings - Fork 3.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Druid __time
filter is not pushed down
#8404
Comments
The result of "explain":
seems the filter is pushed down, but from what we observed, the scan rows are still full table, not sure if there is issue in |
Looks like only trino/plugin/trino-druid/src/main/java/io/trino/plugin/druid/DruidJdbcClient.java Lines 140 to 153 in 60f66d8
I am not an expert in Druid. @findepi @martint Please take a look. |
@sumannewton Correct. There are no explicit type mappings defined for the write path. There should be type mappings defined explicitly for data-types in both the read ( |
Hi @hashhar so is there any plan to improve this part? |
@jerryleooo i don't know of anyone actively working on this at the moment but we can guide anyone who wants to work on this through the changes. Looking at the mentioned methods in the PostgreSQL connector should be all that's needed. cc: @dheerajkulakarni thought you might be interested. |
@hashhar understand and we have the interest to improve this part, will see the methods you mentioned. tks |
Hi @hashhar @sumannewton I am trying to fix this as you guided: #8474 trino/plugin/trino-base-jdbc/src/main/java/io/trino/plugin/jdbc/StandardColumnMappings.java Line 499 in c647c31
getObject(columnIndex, LocalDateTime.class) , I paste the full error log here:
I guess we may need to report this to Avatica or have a workaround here. |
Seems https://issues.apache.org/jira/browse/CALCITE-1630 is related |
Take a look at |
Thanks @hashhar, Since Druid doesn't support |
The |
Lack of pushdown is because Druid column mapping was never implemented yet: trino/plugin/trino-druid/src/main/java/io/trino/plugin/druid/DruidJdbcClient.java Line 152 in 825e6d8
hence we end up calling the We should impl proper type mapping for Druid. See other connectors extending from |
Thanks, @findepi , I am working on this as you suggested. |
The problem is, inside I guess I may need to implement |
@findepi Adding above code for type mapping similar to other JDBC clients resulted in the following error:
@jerryleooo did you face similar issue? |
@jibinpt can see my latest commit |
@jibinpt did you try to add that module? |
@findepi error message say |
This fix is sorely needed! |
Created the pull request for @jerryleooo here: jerryleooo#2 |
#13335 fixes this issue. |
In Druid queries,
__time
is a very important filter since applying it narrows down the scan range, otherwise, Druid will do a full table scan, which is very slow and resource-consuming.Currently, I found the
__time
is not pushed down, not sure if I use it wrongly or the pushdown is not supported yet:Trino SQL:
select sum(col1) from <table name> where __time between date'2021-06-01' and date'2021-06-30';
SQLs sent to Druid:
SELECT "__time", "col1" FROM "druid"."<table name>"
I noticed there are ongoing works (#4109 or #4313) of implementing Druid aggregation pushdown, not sure if this is part work of it
The text was updated successfully, but these errors were encountered: