Skip to content

fix(redshift): fix INTERVAL quoting#5545

Merged
max-sixty merged 4 commits intoPRQL:mainfrom
lukapeschke:fix-interval-quoting-redshift
Nov 8, 2025
Merged

fix(redshift): fix INTERVAL quoting#5545
max-sixty merged 4 commits intoPRQL:mainfrom
lukapeschke:fix-interval-quoting-redshift

Conversation

@lukapeschke
Copy link
Copy Markdown
Contributor

Continuing the work in #5540 for intervals.
Unfortunately, there is no "one size fits all" quoting that works for redshift:

INTERVAL '1 year' causes the following error: Interval values with month or year parts are not supported

SELECT CAST((DATE_TRUNC('week', added_date) - INTERVAL '1' WEEK) AS timestamp) AS firstdayofpreviousweek causes a syntax error: Error occurred while trying to execute a query: [SQLState 42601] ERROR: syntax error at or near \"WEEK\" in context \") However, replacing '1' WEEK with '1 WEEK' here works.

Signed-off-by: Luka Peschke <mail@lukapeschke.com>
@lukapeschke
Copy link
Copy Markdown
Contributor Author

@priithaamer @eitsupi I'd be happy to have your opinion on this one 🙂

@priithaamer
Copy link
Copy Markdown
Contributor

It seems only week is special in Redshift where it needs '1 week' as ValueAndUnitQuoted. Perhaps every other unit could use ValueQuoted because '1' year etc seem to work fine (i'm no expert when it comes to Redshift intervals and it seems there may be nuances)?

@lukapeschke
Copy link
Copy Markdown
Contributor Author

@priithaamer You're right, I set the quoting style to ValueAndUnitQuoted for weeks only in f0fa895 and all our tests passed.

The redshift docs does not mention weeks at all 😕 https://docs.aws.amazon.com/redshift/latest/dg/r_interval_data_types.html

@priithaamer
Copy link
Copy Markdown
Contributor

@priithaamer You're right, I set the quoting style to ValueAndUnitQuoted for weeks only in f0fa895 and all our tests passed.

The redshift docs does not mention weeks at all 😕 https://docs.aws.amazon.com/redshift/latest/dg/r_interval_data_types.html

Thanks! I can confirm this works and can be merged. We could also consider dropping direct week support and if possible, in Redshift dialect compile 1week to INTERVAL '7' DAY?

@max-sixty
Copy link
Copy Markdown
Member

sorry, I merged the other; would you mind resolving conflicts and then merging?

@lukapeschke
Copy link
Copy Markdown
Contributor Author

@max-sixty done :) Can I let you merge and create a new release ?

@eitsupi
Copy link
Copy Markdown
Member

eitsupi commented Nov 8, 2025

Sorry for the delay, I'm not currently in a position to test Redshift so I can't be sure this is correct, but the changes seem reasonable.

@max-sixty max-sixty merged commit 72ef486 into PRQL:main Nov 8, 2025
36 checks passed
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