Skip to content

[CALCITE-5929] Improve LogicalWindow print plan to add the constant value#5015

Merged
xuzifu666 merged 1 commit into
apache:mainfrom
xuzifu666:calcite-5929
Jun 13, 2026
Merged

[CALCITE-5929] Improve LogicalWindow print plan to add the constant value#5015
xuzifu666 merged 1 commit into
apache:mainfrom
xuzifu666:calcite-5929

Conversation

@xuzifu666

Copy link
Copy Markdown
Member

return buf.toString();
}

private static String expandBound(RexWindowBound bound,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please note that bounds may not be literals, but constant expressions. Please add a test case for that to make sure that this toString() does the right thing.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Yes, thanks for the reminder, I added tests to cover this.

@xuzifu666 xuzifu666 requested a review from mihaibudiu June 12, 2026 23:59
* to their values. Unlike {@link #toString()}, this is for display
* only and does not affect {@link #equals} or {@link #hashCode}.
* Constants can be literals or expressions (e.g., 5+5), and are expanded
* to their string representation for readability. */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

they are not expanded for readability, they are expanded because they have to be shown.
You can remove the text after the last comma.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Make sense, done.

@sonarqubecloud

Copy link
Copy Markdown

@xuzifu666 xuzifu666 added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Jun 13, 2026
<Resource name="sql">
<![CDATA[select COUNT(*) over (
ORDER BY empno
ROWS BETWEEN 5 + 5 PRECEDING AND 1 PRECEDING) AS w_count from emp

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I see the SQL and the execution plan here; this is what I wanted to see. I don't have any other questions for now.

@xuzifu666 xuzifu666 merged commit f541bf3 into apache:main Jun 13, 2026
35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

LGTM-will-merge-soon Overall PR looks OK. Only minor things left.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants