Skip to content

fix: insert placeholder type inference showing wrong type when there is function wrapped placeholder (unknown type)#20744

Merged
berkaysynnada merged 2 commits into
apache:mainfrom
buraksenn:fix-placeholder-type-infernece-because-of-unknown-type
Apr 19, 2026
Merged

fix: insert placeholder type inference showing wrong type when there is function wrapped placeholder (unknown type)#20744
berkaysynnada merged 2 commits into
apache:mainfrom
buraksenn:fix-placeholder-type-infernece-because-of-unknown-type

Conversation

@buraksenn
Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Rationale for this change

Please see #20544 for details. I wanted to open a PR to discuss if API change is ok for this fix

What changes are included in this PR?

This PR changes FieldRef to Option for placeholders and adds test case for this.

Are these changes tested?

Yes. Existing tests and added test case for the reproduced error

Are there any user-facing changes?

Yes. this changes prepare_param_data_types field and its callers to use Option wrapped FieldRef instead of FieldRef.

@github-actions github-actions Bot added the sql SQL Planner label Mar 6, 2026
@buraksenn
Copy link
Copy Markdown
Contributor Author

@berkaysynnada can you review this when you've bandwidth?

@berkaysynnada
Copy link
Copy Markdown
Contributor

I'll take a look ASAP

Copy link
Copy Markdown
Contributor

@berkaysynnada berkaysynnada left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Since this involves a pub API change, I'd prefer we wait a few days after this approval before merging, unless there's a reason to rush

Comment thread datafusion/sql/src/statement.rs Outdated
@berkaysynnada berkaysynnada added this pull request to the merge queue Apr 19, 2026
Merged via the queue into apache:main with commit 675881d Apr 19, 2026
35 checks passed
@berkaysynnada berkaysynnada deleted the fix-placeholder-type-infernece-because-of-unknown-type branch April 19, 2026 22:06
Rich-T-kid pushed a commit to Rich-T-kid/datafusion that referenced this pull request Apr 21, 2026
…is function wrapped placeholder (unknown type) (apache#20744)

## Which issue does this PR close?

- Closes apache#20544.

## Rationale for this change
Please see apache#20544 for details. I wanted to open a PR to discuss if API
change is ok for this fix

## What changes are included in this PR?
This PR changes FieldRef to Option<FieldRef> for placeholders and adds
test case for this.

## Are these changes tested?
Yes. Existing tests and added test case for the reproduced error

## Are there any user-facing changes?
Yes. this changes `prepare_param_data_types` field and its callers to
use Option wrapped FieldRef instead of FieldRef.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

sql SQL Planner

Projects

None yet

Development

Successfully merging this pull request may close these issues.

INSERT placeholder type inference corrupted when VALUES contains function-wrapped placeholders

2 participants