Skip to content

Fix type cast issue with dateTimeConvert scalar function #11839

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.

Sign up for GitHub

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

Jump to bottom
Merged
xiangfu0 merged 1 commit into from
Oct 20, 2023
Merged

Fix type cast issue with dateTimeConvert scalar function #11839

xiangfu0 merged 1 commit into from
Oct 20, 2023

Conversation

@xiangfu0
Copy link
Contributor

@xiangfu0 xiangfu0 commented Oct 20, 2023 •
edited
Loading

Right now dateTimeConvert is always cast for STRING in scalar function.
The return type should be LONG(BIGINT) for EPOCH or TIMESTAMP type and STRING for SIMPLE_DATE_FORMAT

This is discovered when running a query in v2:
SELECT DATETIMECONVERT(AGO('P32D'),'1:MILLISECONDS:EPOCH', '1:SECONDS:EPOCH', '1:SECONDS') from activity_full

Error:

ProcessingException(errorCode:150, message:SQLParsingError:
java.lang.Exception: Unable to find table for this query
	at org.apache.pinot.controller.api.resources.PinotQueryResource.getMultiStageQueryResponse(PinotQueryResource.java:210)
	at org.apache.pinot.controller.api.resources.PinotQueryResource.executeSqlQuery(PinotQueryResource.java:173)
	at org.apache.pinot.controller.api.resources.PinotQueryResource.handlePostSql(PinotQueryResource.java:121)
	at jdk.internal.reflect.GeneratedMethodAccessor366.invoke(Unknown Source)
...
Caused by: java.lang.RuntimeException: Error composing query plan for: SELECT DATETIMECONVERT(AGO('P32D'),'1:MILLISECONDS:EPOCH', '1:SECONDS:EPOCH', '1:SECONDS') from activity_full
	at org.apache.pinot.query.QueryEnvironment.getTableNamesForQuery(QueryEnvironment.java:241)
	at org.apache.pinot.controller.api.resources.PinotQueryResource.getMultiStageQueryResponse(PinotQueryResource.java:208)
	... 27 more
Caused by: java.lang.UnsupportedOperationException: Cannot generate a valid execution plan for the given query: LogicalProject(EXPR$0=[DATETIMECONVERT(AGO('P32D'), '1:MILLISECONDS:EPOCH', '1:SECONDS:EPOCH', '1:SECONDS')])
  LogicalTableScan(table=[[activity_full]])
	at org.apache.pinot.query.QueryEnvironment.optimize(QueryEnvironment.java:342)
	at org.apache.pinot.query.QueryEnvironment.compileQuery(QueryEnvironment.java:284)
	at org.apache.pinot.query.QueryEnvironment.getTableNamesForQuery(QueryEnvironment.java:237)
...
Caused by: org.apache.pinot.sql.parsers.SqlCompilationException: Caught exception while making literal with value: 1695011538 and type: BIGINT
	at org.apache.calcite.rel.rules.PinotEvaluateLiteralRule.evaluateLiteralOnlyFunction(PinotEvaluateLiteralRule.java:176)
	at org.apache.calcite.rel.rules.PinotEvaluateLiteralRule$EvaluateLiteralShuttle.visitCall(PinotEvaluateLiteralRule.java:131)
	at org.apache.calcite.rel.rules.PinotEvaluateLiteralRule$EvaluateLiteralShuttle.visitCall(PinotEvaluateLiteralRule.java:119)
	at org.apache.calcite.rex.RexCall.accept(RexCall.java:189)
...
Caused by: java.lang.ClassCastException: class java.lang.String cannot be cast to class java.lang.Number (java.lang.String and java.lang.Number are in module java.base of loader 'bootstrap')
	at org.apache.calcite.rex.RexBuilder.clean(RexBuilder.java:1721)
	at org.apache.calcite.rex.RexBuilder.makeLiteral(RexBuilder.java:1558)
	at org.apache.calcite.rex.RexBuilder.makeLiteral(RexBuilder.java:1520)
	at org.apache.calcite.rel.rules.PinotEvaluateLiteralRule.evaluateLiteralOnlyFunction(PinotEvaluateLiteralRule.java:174))

After fix:
image

@xiangfu0 xiangfu0 force-pushed the allow-cast-for-type branch from 3194fc8 to fa1e475 Compare October 20, 2023 04:22
@xiangfu0 xiangfu0 added the multi-stage Related to the multi-stage query engine label Oct 20, 2023
@xiangfu0 xiangfu0 force-pushed the allow-cast-for-type branch from fa1e475 to 9bfdc93 Compare October 20, 2023 04:43
@codecov-commenter
Copy link

codecov-commenter commented Oct 20, 2023 •
edited
Loading

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 66.38%. Comparing base (86d3c44) to head (0658af2).
⚠️ Report is 3271 commits behind head on master.

Files with missing lines Patch % Lines
.../pinot/common/function/scalar/DateTimeConvert.java 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files 
@@             Coverage Diff              @@
##             master   #11839      +/-   ##
============================================
+ Coverage     66.35%   66.38%   +0.02%     
  Complexity      207      207              
============================================
  Files          2350     2350              
  Lines        127281   127284       +3     
  Branches      19603    19604       +1     
============================================
+ Hits          84463    84496      +33     
+ Misses        36924    36895      -29     
+ Partials       5894     5893       -1
Flag Coverage Δ
custom-integration1 <0.01% <0.00%> (ø)
integration <0.01% <0.00%> (ø)
integration1 <0.01% <0.00%> (ø)
integration2 0.00% <0.00%> (ø)
java-11 66.34% <66.66%> (+0.01%) ⬆️
java-21 66.22% <66.66%> (+<0.01%) ⬆️
skip-bytebuffers-false 66.37% <66.66%> (+0.02%) ⬆️
skip-bytebuffers-true 66.19% <66.66%> (+48.38%) ⬆️
temurin 66.38% <66.66%> (+0.02%) ⬆️
unittests 66.38% <66.66%> (+0.02%) ⬆️
unittests1 67.15% <66.66%> (+0.03%) ⬆️
unittests2 17.83% <0.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@xiangfu0 xiangfu0 force-pushed the allow-cast-for-type branch from 9bfdc93 to 0658af2 Compare October 20, 2023 07:17
@xiangfu0 xiangfu0 requested a review from walterddr October 20, 2023 20:35
walterddr
walterddr approved these changes Oct 20, 2023
View reviewed changes
Copy link
Contributor

@walterddr walterddr left a comment

Choose a reason for hiding this comment

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

lgtm

@xiangfu0 xiangfu0 merged commit 1108036 into apache:master Oct 20, 2023
@xiangfu0 xiangfu0 deleted the allow-cast-for-type branch October 20, 2023 21:57
@xiangfu0 xiangfu0 mentioned this pull request Nov 8, 2023
Merged
@xiangfu0 xiangfu0 added the backward-incompat Referenced by PRs that introduce or fix backward compat issues label Dec 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@walterddr walterddr walterddr approved these changes

@Jackie-Jiang Jackie-Jiang Awaiting requested review from Jackie-Jiang

Assignees

No one assigned

Labels

backward-incompat Referenced by PRs that introduce or fix backward compat issues bugfix multi-stage Related to the multi-stage query engine

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@xiangfu0 @codecov-commenter @walterddr