Skip to content

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
@vercel
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
@codecov
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

data source driver pr:community Contribution from Cube.js community members.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants