Skip to content

Update comments on OptimizerRule about function name matching#20346

Open
alamb wants to merge 2 commits intoapache:mainfrom
alamb:alamb/documentation
Open

Update comments on OptimizerRule about function name matching#20346
alamb wants to merge 2 commits intoapache:mainfrom
alamb:alamb/documentation

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Feb 13, 2026

Which issue does this PR close?

Rationale for this change

I gave feedback to @devanshu0987 https://github.com/apache/datafusion/pull/20180/changes#r2800720037 that it was not a good idea to check for function names in optimizer rules, but then I realized that the rationale for this is not written down anywhere.

What changes are included in this PR?

Document why checking for function names in optimizer rules is not good and offer alternatives

Are these changes tested?

By CI

Are there any user-facing changes?

Just docs, no functional changes

@alamb alamb added the documentation Improvements or additions to documentation label Feb 13, 2026
@github-actions github-actions bot added optimizer Optimizer rules functions Changes to functions implementation and removed documentation Improvements or additions to documentation labels Feb 13, 2026
@alamb alamb force-pushed the alamb/documentation branch from 2137f99 to 6071a18 Compare February 13, 2026 19:59
@github-actions github-actions bot removed the functions Changes to functions implementation label Feb 13, 2026
@alamb alamb added the documentation Improvements or additions to documentation label Feb 13, 2026
@alamb alamb changed the title Alamb/documentation Update comments on OptimizerRule about function name matching Feb 13, 2026
/// DataFusion.
///
/// For example if a rule rewrites a function based on the check
/// `func.name() == 'sum'`, if rewrite the plan incorrectly if the
Copy link
Contributor

Choose a reason for hiding this comment

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

Grammar / typo

///
/// # Notes for implementations:
///
/// # Return the same plan if no changes were made
Copy link
Contributor

Choose a reason for hiding this comment

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

Two hash marks?

///
/// ## Matching on functions
///
/// The rule should avoid function specific transformations, and instead use
Copy link
Contributor

Choose a reason for hiding this comment

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

function-specific

@github-actions github-actions bot removed the documentation Improvements or additions to documentation label Feb 13, 2026
Copy link
Contributor Author

@alamb alamb left a comment

Choose a reason for hiding this comment

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

Thanks @neilconway -- I did the changes in 6759858

@alamb alamb marked this pull request as ready for review February 13, 2026 20:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

optimizer Optimizer rules

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants