Skip to content

bugfix: Invalid QuestDB timestamp floor year unit #7474

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

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

peterklingelhofer
Copy link
Contributor

@peterklingelhofer peterklingelhofer commented Nov 29, 2023

Check List

  • Tests has been run in packages where changes made if available
  • Linter has been run for changed code
  • Tests for the changes have been added if not covered yet
  • Docs have been added / updated if required

Issue Reference this PR resolves

When aggregating data on the year level, we see this error in QuestDB in the generated SQL:

SELECT timestamp_floor(‘Y’, to_timezone("data_hourly".timestamp, 'UTC')) "data_hourly__timestamp_year",
FROM "data_hourly"
LIMIT 5000;

invalid unit 'Y'

Description of Changes Made (if issue reference is not provided)

As you can see in the QuestDB documentation, Y is not a valid option.

Adjusting to use the lowercase y yields results as expected:

SELECT timestamp_floor('y', to_timezone("data_hourly".timestamp, 'UTC')) "data_hourly__timestamp_year",
FROM "data_hourly"
LIMIT 5000;

Note: This only seems to be a problem if we try to grab timestamp__year in the GraphQL query.

query CubeQuery  { 
  cube(where: {dataHourly: {timestamp: {inDateRange: "This year" } }}) {
    dataHourly(orderBy: {timestamp: asc}) {
      data_column_1
      data_column_2
      timestamp {
        year
      }
    }
  }
}

Yields the above invalid unit 'Y' error.

Whereas this query works fine:

query CubeQuery  { 
  cube(where: {dataHourly: {timestamp: {inDateRange: "This year" } }}) {
    dataHourly(orderBy: {timestamp: asc}) {
      data_column_1
      data_column_2
    }
  }
}

@peterklingelhofer peterklingelhofer requested a review from a team as a code owner November 29, 2023 12:51
Copy link

vercel bot commented Nov 29, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

8 Ignored Deployments
Name Status Preview Comments Updated (UTC)
examples-angular-dashboard ⬜️ Ignored (Inspect) Visit Preview Dec 3, 2023 6:32pm
examples-react-d3 ⬜️ Ignored (Inspect) Visit Preview Dec 3, 2023 6:32pm
examples-react-dashboard ⬜️ Ignored (Inspect) Visit Preview Dec 3, 2023 6:32pm
examples-react-data-table ⬜️ Ignored (Inspect) Visit Preview Dec 3, 2023 6:32pm
examples-react-highcharts ⬜️ Ignored (Inspect) Visit Preview Dec 3, 2023 6:32pm
examples-react-material-ui ⬜️ Ignored (Inspect) Visit Preview Dec 3, 2023 6:32pm
examples-react-pivot-table ⬜️ Ignored (Inspect) Visit Preview Dec 3, 2023 6:32pm
examples-vue-query-builder ⬜️ Ignored (Inspect) Visit Preview Dec 3, 2023 6:32pm

@github-actions github-actions bot added the pr:community Contribution from Cube.js community members. label Nov 29, 2023
Copy link

codecov bot commented Nov 29, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (d64fc85) 48.02% compared to head (56e96e7) 48.02%.
Report is 45 commits behind head on master.

❗ Current head 56e96e7 differs from pull request most recent head a3238ad. Consider uploading reports for the commit a3238ad to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #7474   +/-   ##
=======================================
  Coverage   48.02%   48.02%           
=======================================
  Files         155      155           
  Lines       20868    20868           
  Branches     5370     5370           
=======================================
  Hits        10021    10021           
  Misses      10101    10101           
  Partials      746      746           
Flag Coverage Δ
cube-backend 48.02% <ø> (ø)

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.

@paveltiunov
Copy link
Member

@peterklingelhofer Thanks for contributing! Could you please add a test for it?

@paveltiunov paveltiunov self-assigned this Nov 30, 2023
@peterklingelhofer peterklingelhofer requested a review from a team as a code owner December 3, 2023 18:31
@peterklingelhofer
Copy link
Contributor Author

@peterklingelhofer Thanks for contributing! Could you please add a test for it?

Added base and base snake case GraphQL queries to test for this: a3238ad

@peterklingelhofer
Copy link
Contributor Author

@paveltiunov not sure if the tests are just broken with respect to testing year (I noticed there were no year tests before I added this one) or if it's my change. Regardless, adding a GraphQL query to test is not specifically testing the QuestDB driver from what I can tell.

Unfortunately I don't have enough time to dig further - I can close this out and simply create an issue linking to this MR for context. It's not really that big of a deal, if you want data for "This year" it's probably not that hard to figure out what year this year is, and I don't need to for my use-case, just was a bug I noticed in passing that I figured I'd make a quick attempt on.

@paveltiunov
Copy link
Member

@peterklingelhofer I guess snapshots weren't updated.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:community Contribution from Cube.js community members.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants