Skip to content

fix(planner): Fix size estimate used in TextRenderer#27080

Merged
aaneja merged 1 commit intoprestodb:masterfrom
aaneja:fixTextRepresentationSizeEstimate
Feb 17, 2026
Merged

fix(planner): Fix size estimate used in TextRenderer#27080
aaneja merged 1 commit intoprestodb:masterfrom
aaneja:fixTextRepresentationSizeEstimate

Conversation

@aaneja
Copy link
Copy Markdown
Contributor

@aaneja aaneja commented Feb 4, 2026

Description, Motivation and Context

The size estimate on text plans (EXPLAIN plans e.g) was incorrectly changed to use the plan root instead of the actual node's outputs in 7f5f219

We fix this bug

Test Plan

CI + new test to assert on Estimates strings in the text plan

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

== NO RELEASE NOTE ==

Summary by Sourcery

Correct the size estimate used when rendering text query plans to rely on each node's own outputs instead of the plan root.

Bug Fixes:

  • Fix incorrect output size estimation in TextRenderer by basing it on the node representation outputs rather than the plan root.

Enhancements:

  • Expose a size-estimation helper on PlanNodeStatsEstimate that operates on NodeRepresentation outputs.

Summary by Sourcery

Correct size estimation used when rendering textual query plans so estimates are based on each node representation rather than the plan root.

Bug Fixes:

  • Fix incorrect output size calculation in TextRenderer by using the node representation outputs instead of the plan root when computing size estimates.

Enhancements:

  • Add a NodeRepresentation-based output size helper to PlanNodeStatsEstimate for reuse by plan printers and related components.

@prestodb-ci prestodb-ci added the from:IBM PR from IBM label Feb 4, 2026
@sourcery-ai
Copy link
Copy Markdown
Contributor

sourcery-ai bot commented Feb 4, 2026

Reviewer's guide (collapsed on small PRs)

Reviewer's Guide

Fixes text plan size estimation by introducing a NodeRepresentation-based output size helper in PlanNodeStatsEstimate and using it in TextRenderer instead of the plan root, so estimates reflect each node’s own outputs.

Sequence diagram for size estimation in TextRenderer printEstimates

sequenceDiagram
    participant TR as TextRenderer
    participant PR as PlanRepresentation
    participant NR as NodeRepresentation
    participant SE as PlanNodeStatsEstimate

    TR->>PR: getPlanNodeRoot()
    PR-->>TR: PlanNode (unused for size in new behavior)

    TR->>SE: getSourceInfo()
    SE-->>TR: SourceInfo

    TR->>SE: getOutputSizeInBytes(NR)
    alt sizeUsingVariables is false and totalSize is not NaN
        SE-->>TR: totalSize
    else sizeUsingVariables is true or totalSize is NaN
        SE->>NR: getOutputs()
        NR-->>SE: List outputs
        SE->>SE: getOutputSizeForVariables(outputs)
        SE-->>TR: computedSize
    end

    TR->>TR: formatEstimateAsDataSize(size)
    TR-->>TR: include size in printed estimates
Loading

Class diagram for updated PlanNodeStatsEstimate and TextRenderer interaction

classDiagram
    class PlanNodeStatsEstimate {
        - SourceInfo sourceInfo
        - double totalSize
        + double getOutputSizeInBytes(PlanNode planNode)
        + double getOutputSizeInBytes(NodeRepresentation nodeRepresentation)
        - double getOutputSizeForVariables(List~VariableReferenceExpression~ variables)
        + SourceInfo getSourceInfo()
    }

    class SourceInfo {
        + boolean estimateSizeUsingVariables()
    }

    class PlanNode {
        + List~VariableReferenceExpression~ getOutputVariables()
    }

    class NodeRepresentation {
        + List~VariableReferenceExpression~ getOutputs()
    }

    class TextRenderer {
        - String printEstimates(PlanRepresentation plan, NodeRepresentation node)
    }

    class PlanRepresentation {
        + PlanNode getPlanNodeRoot()
    }

    class VariableReferenceExpression

    TextRenderer ..> PlanNodeStatsEstimate : uses
    TextRenderer ..> PlanRepresentation : uses
    TextRenderer ..> NodeRepresentation : uses

    PlanNodeStatsEstimate ..> SourceInfo : uses
    PlanNodeStatsEstimate ..> PlanNode : uses
    PlanNodeStatsEstimate ..> NodeRepresentation : uses
    PlanNodeStatsEstimate ..> VariableReferenceExpression : uses

    PlanRepresentation --> PlanNode : root
    PlanNode --> VariableReferenceExpression : outputVariables
    NodeRepresentation --> VariableReferenceExpression : outputs
Loading

File-Level Changes

Change Details Files
Add size-estimation helper that works on NodeRepresentation outputs.
  • Introduce getOutputSizeInBytes(NodeRepresentation) overload on PlanNodeStatsEstimate.
  • Guard against null NodeRepresentation and reuse existing logic for variable-based size estimation.
  • Respect sourceInfo.estimateSizeUsingVariables() and precomputed totalSize before falling back to computing size from node outputs.
presto-main-base/src/main/java/com/facebook/presto/cost/PlanNodeStatsEstimate.java
Use node-specific size estimates when rendering text query plans.
  • Update TextRenderer.printEstimates to call the new PlanNodeStatsEstimate.getOutputSizeInBytes(NodeRepresentation) overload.
  • Stop using the plan root when formatting data size estimates for individual nodes in EXPLAIN output.
presto-main-base/src/main/java/com/facebook/presto/sql/planner/planPrinter/TextRenderer.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

@aaneja aaneja marked this pull request as ready for review February 4, 2026 08:26
@aaneja aaneja requested review from a team, feilong-liu and jaystarshot as code owners February 4, 2026 08:26
@prestodb-ci prestodb-ci requested review from a team, Mariamalmesfer and pramodsatya and removed request for a team February 4, 2026 08:26
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

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 ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@tdcmeehan tdcmeehan self-assigned this Feb 6, 2026
@aaneja aaneja force-pushed the fixTextRepresentationSizeEstimate branch 2 times, most recently from ed1e04d to e3dce8a Compare February 11, 2026 07:01
@aaneja aaneja force-pushed the fixTextRepresentationSizeEstimate branch from e3dce8a to 355e507 Compare February 11, 2026 07:17
@aaneja aaneja merged commit 418e326 into prestodb:master Feb 17, 2026
86 of 87 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

from:IBM PR from IBM

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants