misc: Fix RemoveCrossJoinWithConstantInput to use unknown locality

[feilong-liu]

Description

Locality of a project is set in PlanRemoteProjections optimizer. RemoveCrossJoinWithConstantInput runs before PlanRemoteProjection, and we should not set the project locality to local, it should be unknown and wait for PlanRemoteProjections to set it.

Motivation and Context

As in description

Impact

Bug fix

Test Plan

Unit tests

Contributor checklist

• Please make sure your submission complies with our contributing guide, in particular code style and commit standards.
• PR description addresses the issue accurately and concisely. If the change is non-trivial, a GitHub Issue is referenced.
• Documented new properties (with its default value), SQL syntax, functions, or other functionality.
• If release notes are required, they follow the release notes guidelines.
• Adequate tests were added if applicable.
• CI passed.
• If adding new dependencies, verified they have an OpenSSF Scorecard score of 5.0 or higher (or obtained explicit TSC approval for lower scores).

Release Notes

Please follow release notes guidelines and fill in the release notes below.

== NO RELEASE NOTE ==

Summary by Sourcery

Ensure projections created when removing cross joins with constant inputs default to unknown locality so later optimizers can determine appropriate locality.

Bug Fixes:

• Prevent RemoveCrossJoinWithConstantInput from forcefully setting project locality to LOCAL by using UNKNOWN instead, allowing PlanRemoteProjections to assign locality correctly.

Enhancements:

• Extend PlannerUtils.addProjections to accept an explicit locality parameter for greater flexibility when constructing ProjectNodes.

Tests:

• Add planner rule tests verifying that ProjectNodes produced by RemoveCrossJoinWithConstantInput (with and without join filters) have UNKNOWN locality.

[sourcery-ai]

Reviewer's Guide

Adjusts RemoveCrossJoinWithConstantInput so that the ProjectNode it introduces uses UNKNOWN locality instead of LOCAL, by extending PlannerUtils.addProjections to accept a locality parameter and adding unit tests to verify the new behavior with and without a join filter.

Sequence diagram for RemoveCrossJoinWithConstantInput using UNKNOWN locality

sequenceDiagram
    participant RuleEngine
    participant RemoveCrossJoinWithConstantInput
    participant PlannerUtils
    participant ProjectNode
    participant PlanRemoteProjections

    RuleEngine->>RemoveCrossJoinWithConstantInput: apply(joinNode, ruleContext)
    RemoveCrossJoinWithConstantInput->>PlannerUtils: addProjections(joinInput, idAllocator, mapping, UNKNOWN)
    PlannerUtils->>PlannerUtils: build Assignments from joinInput outputs
    PlannerUtils->>ProjectNode: construct(id, joinInput, assignments, UNKNOWN)
    ProjectNode-->>PlannerUtils: ProjectNode
    PlannerUtils-->>RemoveCrossJoinWithConstantInput: ProjectNode with locality UNKNOWN
    RemoveCrossJoinWithConstantInput-->>RuleEngine: Result with rewritten plan

    RuleEngine->>PlanRemoteProjections: apply(planWithProjectNode, ruleContext)
    PlanRemoteProjections->>ProjectNode: inspect locality (UNKNOWN)
    PlanRemoteProjections->>ProjectNode: set locality based on distribution (LOCAL or REMOTE)
    ProjectNode-->>PlanRemoteProjections: ProjectNode with final locality
    PlanRemoteProjections-->>RuleEngine: updated plan with planned remote projections

Loading

Class diagram for updated PlannerUtils and RemoveCrossJoinWithConstantInput

classDiagram
    class PlannerUtils {
        +addProjections(source: PlanNode, planNodeIdAllocator: PlanNodeIdAllocator, variableMap: Map~VariableReferenceExpression, RowExpression~): PlanNode
        +addProjections(source: PlanNode, planNodeIdAllocator: PlanNodeIdAllocator, variableMap: Map~VariableReferenceExpression, RowExpression~, locality: Locality): PlanNode
    }

    class RemoveCrossJoinWithConstantInput {
        +apply(node: JoinNode, context: RuleContext): Result
    }

    class ProjectNode {
        +ProjectNode(id: PlanNodeId, source: PlanNode, assignments: Assignments, locality: Locality)
        +getId(): PlanNodeId
        +getSource(): PlanNode
        +getAssignments(): Assignments
        +getLocality(): Locality
    }

    class Locality {
        <<enumeration>>
        LOCAL
        UNKNOWN
        REMOTE
    }

    PlannerUtils ..> ProjectNode : creates
    PlannerUtils ..> Locality : uses
    RemoveCrossJoinWithConstantInput ..> PlannerUtils : uses
    RemoveCrossJoinWithConstantInput ..> Locality : uses
    ProjectNode --> Locality

Loading

File-Level Changes

Change	Details	Files
Allow addProjections to specify ProjectNode locality and default to LOCAL for existing callers.	Introduce an overloaded addProjections method that takes a Locality parameter in addition to the existing arguments. Retain the original addProjections signature as a convenience wrapper that delegates to the new method using LOCAL locality. Use the supplied locality when constructing the ProjectNode instead of always using LOCAL.	presto-main-base/src/main/java/com/facebook/presto/sql/planner/PlannerUtils.java
Ensure RemoveCrossJoinWithConstantInput creates ProjectNodes with UNKNOWN locality so later optimizers can determine locality.	Change RemoveCrossJoinWithConstantInput to call the new addProjections overload with UNKNOWN locality when rewriting cross joins with constant input. Keep the existing FilterNode wrapping logic, only changing the locality of the underlying ProjectNode.	presto-main-base/src/main/java/com/facebook/presto/sql/planner/iterative/rule/RemoveCrossJoinWithConstantInput.java
Add tests verifying that ProjectNodes produced by RemoveCrossJoinWithConstantInput have UNKNOWN locality, including when a join filter is present.	Add a unit test that builds a simple INNER join with a constant values input and asserts the rewritten root node is a ProjectNode with UNKNOWN locality. Add a unit test for an INNER join with a join filter that verifies the root is a FilterNode whose source is a ProjectNode with UNKNOWN locality. Use existing testing infrastructure (tester(), variables, tableScan, values) to construct the plans and assertions.	presto-main-base/src/test/java/com/facebook/presto/sql/planner/iterative/rule/TestRemoveCrossJoinWithConstantInput.java

---

Tips and commands

Interacting with Sourcery

• Trigger a new review: Comment @sourcery-ai review on the pull request.
• Continue discussions: Reply directly to Sourcery's review comments.
• Generate a GitHub issue from a review comment: Ask Sourcery to create an
  issue from a review comment by replying to it. You can also reply to a
  review comment with @sourcery-ai issue to create an issue from it.
• Generate a pull request title: Write @sourcery-ai anywhere in the pull
  request title to generate a title at any time. You can also comment
  @sourcery-ai title on the pull request to (re-)generate the title at any time.
• Generate a pull request summary: Write @sourcery-ai summary anywhere in
  the pull request body to generate a PR summary at any time exactly where you
  want it. You can also comment @sourcery-ai summary on the pull request to
  (re-)generate the summary at any time.
• Generate reviewer's guide: Comment @sourcery-ai guide on the pull
  request to (re-)generate the reviewer's guide at any time.
• Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
  pull request to resolve all Sourcery comments. Useful if you've already
  addressed all the comments and don't want to see them anymore.
• Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
  request to dismiss all existing Sourcery reviews. Especially useful if you
  want to start fresh with a new review - don't forget to comment
  @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

• Enable or disable review features such as the Sourcery-generated pull request
  summary, the reviewer's guide, and others.
• Change the review language.
• Add, remove or edit custom review instructions.
• Adjust other review settings.

Getting Help

• Contact our support team for questions or feedback.
• Visit our documentation for detailed guides and information.
• Keep in touch with the Sourcery team by following us on X/Twitter, LinkedIn or GitHub.

[sourcery-ai]

Hey - I've reviewed your changes and they look great!

---

Sourcery is free for open source - if you like our reviews please consider sharing them ✨

• X
• Mastodon
• LinkedIn
• Facebook

_{Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.}