Skip to content

Conversation

chenkovsky
Copy link
Contributor

bigquery supports

SELECT DATETIME_TRUNC(CURRENT_DATETIME, WEEK(MONDAY))

WEEK(MONDAY) should be parsed to datetime_field

Comment on lines +2572 to +2577
let stmt = bigquery().verified_stmt(concat!(
"SELECT ",
"DATE_TRUNC(CURRENT_DATE, DAY), ",
"DATE_TRUNC(CURRENT_DATE, WEEK(MONDAY)) ",
"FROM my_table",
));
Copy link
Contributor

Choose a reason for hiding this comment

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

double checking the intent of the changes: my understanding is that currently given SELECT DATE_TRUNC(CURRENT_DATE, WEEK(MONDAY), WEEK(MONDAY) should be parsed as a function with name WEEK and having a single identifier MONDAY as argument? Is that indeed the case (vs e.g the parser not being able to handle such a statement)

Copy link
Contributor Author

@chenkovsky chenkovsky Jul 23, 2025

Choose a reason for hiding this comment

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

currently, DAY will be parsed as ident, week will be parsed as function. I think this is flawed.

  1. when transpiling bigquery sql to another dialect, DATE_TRUNC(CURRENT_DATE, DAY) should be transpiled to DATE_TRUNC(CURRENT_DATE,'day'), but currently, it will be transpiled into DATE_TRUNC(CURRENT_DATE, DAY).
  2. when compiling sql to logical plan, planner will try to find column called DAY, MONDAY, and function called WEEK. we should tell planner this is not function or column. I think we should convert it into str when planning.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah I see, I think the current behavior of having it parsed as a function is preferable since the construct is syntatically a function - there's quite a lot of such special scenarios across dialects and it would likely be unwieldy to cover them consistently and I think out of scope for the parser as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thanks. if it's out of scope, i will close this pr. it can also be solved with a postprocessor.

@chenkovsky chenkovsky closed this Jul 25, 2025
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.

2 participants