Skip to content

Conversation

@emgeee
Copy link
Contributor

@emgeee emgeee commented Aug 16, 2024

Which issue does this PR close?

Closes #12020

Rationale for this change

A getter method seems to be the preferred way over making the internal struct public

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

@github-actions github-actions bot added the logical-expr Logical plan and expressions label Aug 16, 2024
Copy link
Contributor

@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.

Seems like a good idea to me -- thank ou @emgeee

Note there is a PR with a proposal to change this structure a bit here too #12040

Copy link
Contributor

@metegenez metegenez left a comment

Choose a reason for hiding this comment

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

LGTM.

@alamb alamb merged commit 950dc73 into apache:main Aug 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

logical-expr Logical plan and expressions

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to access LogicalPlanBuilder.plan inside a new trail implementation

3 participants