FrankChen021 commented on PR #16592: URL: https://github.com/apache/druid/pull/16592#issuecomment-5741673087
> I think that getting rid of the bindables mode might just make this and possibly other issues go away... > > I was looking around a bit and I think you will get similar issues for all the functions inside `NestedDataOperatorConversions` > > to address that I think the following might be considered: > > * fix these `SqlFunction`-s in [NestedDataOperatorConversions](https://github.com/apache/druid/blob/bf2be938a93aa3819901bfcc65adac66ce2bc75a/sql/src/main/java/org/apache/druid/sql/calcite/expression/builtin/NestedDataOperatorConversions.java#L727-L745) ; to be usable bindable functions > > * make the builder return a [SqlUserDefinedFunction](https://github.com/apache/druid/blob/bf2be938a93aa3819901bfcc65adac66ce2bc75a/sql/src/main/java/org/apache/druid/sql/calcite/expression/OperatorConversions.java#L555) > * which will need an [ImplementableFunction](https://github.com/apache/calcite/blob/47a2597bb20d6d9f006f60330499323f98bf0c2f/core/src/main/java/org/apache/calcite/schema/ImplementableFunction.java#L33) > > * preferably that function should utilize the the original udf implementatiion behind the scenes to process the data thru the [CallImplementor](https://github.com/apache/calcite/blob/47a2597bb20d6d9f006f60330499323f98bf0c2f/core/src/main/java/org/apache/calcite/adapter/enumerable/CallImplementor.java#L38) > * make the `bindables` go away > > * the reason these are around is due to the fact that some tables are only provided in a way the `bindable` wants them [here](https://github.com/apache/druid/blob/bf2be938a93aa3819901bfcc65adac66ce2bc75a/sql/src/main/java/org/apache/druid/sql/calcite/planner/QueryHandler.java#L208-L221) > * fixing that would be to make sure that all tables can be unwrapped to [DruidTable](https://github.com/apache/druid/blob/bf2be938a93aa3819901bfcc65adac66ce2bc75a/sql/src/main/java/org/apache/druid/sql/calcite/planner/QueryHandler.java#L288) > * easiest option would be to provide them as an `InlineTable` ; however that would mean all those values will appear in the plans - so a new subclass of `DruidTable` would be needed - which could access `information_schema` stuff > > I would preffer (2) as I have a feeling that (1) might be quite tricky.... Removing bindables mode have further benefits; it will make those queries work the same as regular tables with joins/etc; and it would reduce the complexity a bit. > > I'll try to think about further alternatives/ideas/etc > > cc: @gianm All druid native functions do not work on system table and information_schema tables, this has been a problem when these tables were first introduced, and has never been addressed. In #20183 , I add the native table support for system tables, and based on that , information_schema table can also be migrated to the native query path. I hope we can review and merge it soon( I expect it can be merged in 39 release). For this PR, I will close it. @gianm -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
