Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IGNITE-22480 Sql. Avoid compiling literal-only expressions when assembling a row for insert #3922

Merged
merged 7 commits into from
Jun 19, 2024

Conversation

xtern
Copy link
Contributor

@xtern xtern commented Jun 14, 2024

@xtern xtern marked this pull request as ready for review June 17, 2024 11:47
@xtern xtern changed the title IGNITE-22480 IGNITE-22480 Sql. Avoid compiling literal-only expressions when assembling a row for insert Jun 17, 2024
Object val = literal.getValueAs(type);

// Literal was parsed as UTC timestamp, now we need to adjust it to the client's time zone.
if (val != null && literal.getTypeName() == SqlTypeName.TIMESTAMP_WITH_LOCAL_TIME_ZONE) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we adjust TIME_WITH_LOCAL_TIME_ZONE as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we have separate ticket for this https://issues.apache.org/jira/browse/IGNITE-21555

@@ -704,8 +701,36 @@ public void testRowSource() {
RexNode val10 = rexBuilder.makeExactLiteral(new BigDecimal("1"), intType);
RexNode val11 = rexBuilder.makeExactLiteral(new BigDecimal("2"), bigIntType);
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. I think we need to ensure that DECIMAL values preserve precision/scale after this transformation.
  2. Ideally, we need to check literals of all types here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, the test has been improved, please take a look again.

@xtern xtern merged commit 9961bc8 into apache:main Jun 19, 2024
1 check passed
@xtern xtern deleted the ignite-22480 branch June 19, 2024 11:18
// Avoiding compilation when all expressions are constants.
for (int i = 0; i < values.size(); i++) {
if (!(values.get(i) instanceof RexLiteral)) {
return new ValuesImpl(scalar(values, null), ctx.rowHandler().factory(rowSchema));
Copy link
Member

Choose a reason for hiding this comment

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

Factory could be cached in local variable.

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.

4 participants