Skip to content

[CALCITE-7810] Write Rex string literal digests directly - #5284

Open
FrankChen021 wants to merge 1 commit into
apache:mainfrom
FrankChen021:codex/direct-rex-literal-formatting
Open

FrankChen021 wants to merge 1 commit into
apache:mainfrom
FrankChen021:codex/direct-rex-literal-formatting

Conversation

@FrankChen021

@FrankChen021 FrankChen021 commented Sep 22, 2026 •

Copy link
Copy Markdown
Member

Fixes CALCITE-7810.

Why

String RexLiteral digest generation asks NlsString to build a complete SQL string and then copies that temporary string into the destination builder. Large literal ARRAYs repeat this allocation for every element.

In Druid's string-IN planning benchmark, writing directly to the destination reduced allocation by 2.46% at 100,000 literals and 1.31% at 1,000,000 literals. Timing confidence intervals overlapped, so no latency improvement is claimed.

What

Add NlsString.asSql overloads that append the escaped SQL representation to a caller-provided StringBuilder. The existing string-returning method delegates to the same implementation, and RexLiteral uses the direct-write overload.

ANSI dialect selection, charset prefixes, quote escaping, and collation suffix handling remain encapsulated in NlsString.

Verification

  • UtilTest.testNlsStringAsSqlStringBuilder covers appending to existing content, charset prefixes, and quote escaping.
  • RexBuilderTest.testStringLiteral verifies unchanged RexLiteral output.
  • ./gradlew :core:test --tests org.apache.calcite.rex.RexBuilderTest.testStringLiteral --tests org.apache.calcite.util.UtilTest.testNlsStringAsSqlStringBuilder
  • ./gradlew :core:autostyleJavaCheck :core:checkstyleMain :core:checkstyleTest
  • Druid InPlanningBenchmark.queryStringInSqlPlanOnly with -prof gc
Druid benchmark results
String literals Allocation before Allocation after Reduction
100,000 1,454.93 MiB/op 1,419.20 MiB/op 35.73 MiB/op (2.46%)
1,000,000 14,617.18 MiB/op 14,425.79 MiB/op 191.39 MiB/op (1.31%)

Configuration: inSubQueryThreshold=2147483647, rowsPerSegment=500000, 2 forks, 2 one-second warmup iterations, and 5 one-second measurement iterations. Allocation is cumulative bytes per operation, not retained or peak heap.

The performance measurement currently comes from Druid; a Calcite-local ubenchmark is not yet included.

Scope

This change affects string RexLiteral digest construction only; it does not change the produced SQL representation.

Copilot AI lite review requested due to automatic review settings September 22, 2026 10:24

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@sonarqubecloud

Copy link
Copy Markdown

@xuzifu666

Copy link
Copy Markdown
Member

Why are you submitting multiple PRs using the same Jira ticket number? Please consult the contributor's guide first.

@FrankChen021

FrankChen021 commented Sep 22, 2026 •

Copy link
Copy Markdown
Member Author

Why are you submitting multiple PRs using the same Jira ticket number? Please consult the contributor's guide first.

they're for the same purpose, but changes are splitted so that it's easier for reviewing and track the benefit of each change

@FrankChen021

Copy link
Copy Markdown
Member Author

Why are you submitting multiple PRs using the same Jira ticket number? Please consult the contributor's guide first.

I would like to point out that based on the document, I don't see any statement that a JIRA ticket number can only be used by one PR. If I miss sth, you're welcome to point out.

@rubenada

Copy link
Copy Markdown
Contributor

@FrankChen021 thanks for the contribution.
In this case, I'd say it's recommendable to create sub-tasks under the main Jira ticket, and attach each PR to each one of the sub-tasks.

@FrankChen021 FrankChen021 changed the title [CALCITE-7805] Write Rex string literal digests directly [CALCITE-7810] Write Rex string literal digests directly Sep 22, 2026
@FrankChen021

Copy link
Copy Markdown
Member Author

@FrankChen021 thanks for the contribution. In this case, I'd say it's recommendable to create sub-tasks under the main Jira ticket, and attach each PR to each one of the sub-tasks.

This sounds good to me.
Updated as you suggested. Thanks.

@julianhyde julianhyde left a comment

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.

-1 slop

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants