Skip to content

[CALCITE-7726] Improve identifier validation on MemberExpression and ParameterExpression - #5189

Merged
rubenada merged 1 commit into
apache:mainfrom
rubenada:CALCITE-7726
Aug 20, 2026
Merged

[CALCITE-7726] Improve identifier validation on MemberExpression and ParameterExpression#5189
rubenada merged 1 commit into
apache:mainfrom
rubenada:CALCITE-7726

Conversation

@rubenada

Copy link
Copy Markdown
Contributor

Jira Link

CALCITE-7726

Changes Proposed

Improve identifier validation on MemberExpression and ParameterExpression. Fail early with a meaningful exception if a wrong identifier is detected.

String tmpName = result.accumulator().isEmpty()
? "ar"
: (result.accumulator().get(0) + "$Res");
String tmpName = result.accumulator().isEmpty() ? "ar" : "acc$Res";

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This was actually a latent bug unveiled by this patch.
For instance TpchTests would fail now on the new checkArgument with "field name should be a valid java identifier" since the old code could include a dot on the identifier (which is incorrect).
The problem was (by chance) hidden because Janino code generator inlined the expression directly, without actually using the problematic identifier; but the bug was there, latent.

@rubenada rubenada added the LGTM-will-merge-soon Overall PR looks OK. Only minor things left. label Aug 20, 2026
@sonarqubecloud

Copy link
Copy Markdown

@rubenada
rubenada merged commit 17db027 into apache:main Aug 20, 2026
19 checks passed
@rubenada

Copy link
Copy Markdown
Contributor Author

Thanks @mihaibudiu for the review!

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.

2 participants