ccciudatu commented on PR #13772: URL: https://github.com/apache/datafusion/pull/13772#issuecomment-2578841705
@vbarua @Blizzara I finally got back to figure out whether it still makes sense with the new APIs. With the latest changes, supporting extension tables no longer requires a fork of datafusion, but merely a custom implementation for the new traits. However, there's a bit of boilerplate/delegation/copy-paste required for leveraging the default functionality. So I decided to give this another try with a few changes in a fixup commit: - I changed the `SerializerRegistry` trait to return a string qualifier alongside the bytes on serialize. This way, the implementor can make proper use of the protobuf `type_url` or can use any string qualifier as a discriminator for the actual TableProvider implementation. - Since the ExtensionTable::detail::type_url is now supposed to be the proper protobuf one, I chose to store the original table name as `rel.common.hint.alias`, hoping to convince @vbarua that a custom TableProvider implementation is hardly aware of the names under which users choose to register the corresponding tables (especially for UDTFs), so it's not trivial to make the name part of the extension payload and it imposes unnecessary restrictions for the supported table providers. The alias property is loosely specified; the canonical example seems to be the SubqueryAlias, but it's not limited to that and it feels "close enough". A default name will be used if the hint is missing. -- 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]
