linhr commented on PR #20548: URL: https://github.com/apache/datafusion/pull/20548#issuecomment-3976394110
@goldmedal Thanks for the review! > One design question: would it be better to try the standard `source_as_provider` path first, and only fall back to extension planners when the `TableSource` is not a `DefaultTableSource`? Good points! Yeah I agree that having extension planner as a fallback is indeed better, for the reasons you've mentioned. I have made the change accordingly. > Also, a minor note: the existing plan_extension path validates that the returned ExecutionPlan schema matches the LogicalPlan schema (around line 1649). It might be worth adding a similar check here to catch mismatches early. Nice catch! I have added the validation (with a minor refactor to extract the validation logic as a separate function). -- 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]
