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

Fix timestamp literal handling in the multi-stage query engine #14502

Merged
merged 1 commit into from
Nov 20, 2024

Conversation

yashmayya
Copy link
Collaborator

@yashmayya yashmayya commented Nov 20, 2024

  • There are some gaps in timestamp literal handling in the multi-stage query engine - particularly, when converting from Calcite's internal representation to a long value representing milliseconds since epoch (which is what Pinot uses internally).
  • The existing logic only handles the case where the representation is an instance of java.util.Calendar. However, the internal representation used by Calcite for timestamp types is org.apache.calcite.util.TimestampString (see here). External callers usually convert this to Calendar or Long (see here), but this doesn't always appear to be the case.
  • One example is range sets in a Sarg (search argument) - this issue was detected by a query with a filter predicate like WHERE ts BETWEEN '2016-01-01 00:00:00' AND '2016-01-01 10:00:00' where the class cast exception class org.apache.calcite.util.TimestampString cannot be cast to class java.util.Calendar was thrown when converting the bounds of a range here during conversion of SEARCH to an OR of ranges. This wasn't a major issue until recently because search nodes were usually reduced before the final query plan was generated (which changed in Apply filter reduce expressions Calcite rule at the end to prevent literal-only filter pushdown to leaf stage #14448).
  • Simply replacing the Calendar based cast logic to TimestampString based cast logic is also incorrect and a query with a filter predicate like WHERE ts >= CAST(1454284798000 AS TIMESTAMP) will fail with a class cast exception class java.util.GregorianCalendar cannot be cast to class org.apache.calcite.util.TimestampString.
  • This patch fixes the timestamp literal handling logic to cover all the cases. A couple of query compilation tests based on the above two filter predicates have also been added.

@yashmayya yashmayya added bugfix multi-stage Related to the multi-stage query engine labels Nov 20, 2024
@yashmayya yashmayya merged commit a914940 into apache:master Nov 20, 2024
21 checks passed
davecromberge pushed a commit to davecromberge/pinot that referenced this pull request Nov 22, 2024
yashmayya added a commit to yashmayya/pinot that referenced this pull request Jan 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix multi-stage Related to the multi-stage query engine
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants