Skip to content

Conversation

@alamb
Copy link
Contributor

@alamb alamb commented Apr 8, 2024

Which issue does this PR close?

Part of #9994

Rationale for this change

These methods were introduced in #9913 by @peter-toth but I think they were part of a previous implementation and unused.

What changes are included in this PR?

  1. Mark apply_subqueries and map_subueries pub
  2. Remove the (unused) transform_* functions

Are these changes tested?

CI

Are there any user-facing changes?

No (as the removed functions have not been released)

@peter-toth
Copy link
Contributor

peter-toth commented Apr 8, 2024

1. looks good to me, but regarding 2. I'm not sure we need to remove those APIs: #9913 (comment). The transform_..._with_subquery() versions are indeed not in use yet, but can be useful in the future.

@alamb alamb changed the title Minor: remove unused LogicalPlan::transform code, adjust visibility Minor: adjust visibility of LogicalPlan:: apply_subqueries and LogicalPlan::map_subueries Apr 8, 2024
@alamb
Copy link
Contributor Author

alamb commented Apr 8, 2024

  1. looks good to me, but regarding 2. I'm not sure we need to remove those APIs: Consistent LogicalPlan subquery handling in TreeNode::apply and TreeNode::visit  #9913 (comment). The transform_..._with_subquery() versions are indeed not in use yet, but can be useful in the future.

Makes sense -- I will make a new PR that makes them pub / adds docs

@alamb alamb closed this Apr 8, 2024
@alamb
Copy link
Contributor Author

alamb commented Apr 8, 2024

#9998 for making them pub. I will document them as a follow on PR

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.

2 participants