Uh oh!
There was an error while loading. Please reload this page.
Allow SQL TypePlanner to plan SQL types as extension types - #20676
Merged
Conversation
paleolimbot
commented
Mar 3, 2026
Uh oh!
There was an error while loading. Please reload this page.
paleolimbotforce-pushed
the
sql-type-planner-ext
branch
from
March 3, 2026 17:22
e00cd32 to
e0c56f2Comparepaleolimbot
marked this pull request as ready for review
March 3, 2026 18:29
alamb
approved these changes
Mar 4, 2026
alamb
left a comment
Contributor
There was a problem hiding this comment.
Thanks @paleolimbot -- this looks good to me
The only question I have is if we can document the best practices for people implementing TypePlanner (it seems like maybe the best practice is always to implement both plan_type and plan_type_field?) Also perhaps we could consider deprecating plan_type 🤔
| /// | ||
| /// Returns None if not possible. The default implementation falls back | ||
| /// on plan_type and wraps it in a nullable field reference. | ||
| fn plan_type_field( |
Contributor
There was a problem hiding this comment.
Maybe we can help document how this is different from plan_type (or should we be deprecating plan_type in favor of plan_type_field?
paleolimbot
commented
Mar 4, 2026
MemberAuthor
I added the deprecation here and a note on the reason for using the new member. Thanks! |
This was referenced Mar 6, 2026
alamb
commented
Mar 9, 2026
Contributor
Thanks @paleolimbot |
Uh oh!
There was an error while loading. Please reload this page.
de-bgunter pushed a commit
to de-bgunter/datafusion
that referenced
this pull request
Mar 24, 2026
…20676) ## Which issue does this PR close? - Closesapache#20675 ## Rationale for this change The existing enum `SQLDataType` has a number of existing members that have canonical Arrow extension type equivalents; however, the `TypePlanner` trait only supports returning `DataType` (which cannot represent an Arrow extension type). This will be substantially more useful after apache#18136, as the SQL planner inserts casts in a number of places (and currently the extension metadata of those casts is dropped when the logical cast is created). ## What changes are included in this PR? This PR adds a `fn plan_type_field()` member to the `TypePlanner` trait. The only place that the previously existing `plan_type()` member was called was already wrapping it in a `FieldRef` and so few other changes were needed. ## Are these changes tested? Yes ## Are there any user-facing changes? Existing `TypePlanner`s will continue to work and an example was added for supporting the UUID type.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
TypePlannerto plan SQL types as extension types #20675Rationale for this change
The existing enum
SQLDataTypehas a number of existing members that have canonical Arrow extension type equivalents; however, theTypePlannertrait only supports returningDataType(which cannot represent an Arrow extension type).This will be substantially more useful after #18136, as the SQL planner inserts casts in a number of places (and currently the extension metadata of those casts is dropped when the logical cast is created).
What changes are included in this PR?
This PR adds a
fn plan_type_field()member to theTypePlannertrait. The only place that the previously existingplan_type()member was called was already wrapping it in aFieldRefand so few other changes were needed.Are these changes tested?
Yes
Are there any user-facing changes?
Existing
TypePlanners will continue to work and an example was added for supporting the UUID type.