Skip to content

refactor: Define CTE-related SQL nodes for emitter#2495

Open
TrevorBergeron wants to merge 17 commits intomainfrom
cte_extract2
Open

refactor: Define CTE-related SQL nodes for emitter#2495
TrevorBergeron wants to merge 17 commits intomainfrom
cte_extract2

Conversation

@TrevorBergeron
Copy link
Contributor

Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

  • Make sure to open an issue as a bug/issue before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
  • Ensure the tests and linter pass
  • Code coverage does not decrease (if any source code was changed)
  • Appropriate docs were updated (if necessary)

Fixes #<issue_number_goes_here> 🦕

@TrevorBergeron TrevorBergeron requested review from a team as code owners March 6, 2026 00:49
@TrevorBergeron TrevorBergeron requested a review from tswast March 6, 2026 00:49
@product-auto-label product-auto-label bot added the size: l Pull request size is large. label Mar 6, 2026
@product-auto-label product-auto-label bot added the api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. label Mar 6, 2026
Copy link
Contributor

@chelsea-lin chelsea-lin left a comment

Choose a reason for hiding this comment

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

It would be great if you can check all generated SQL are executable in the bigframes-dev project.

INNER JOIN `bfcte_1`
ON COALESCE(CAST(`bfcol_1` AS STRING), '0') = COALESCE(CAST(`bfcol_5` AS STRING), '0')
AND COALESCE(CAST(`bfcol_1` AS STRING), '1') = COALESCE(CAST(`bfcol_5` AS STRING), '1')
), `bfcte_3` AS (
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you please double check why this SQL is longer? Both bfcte_2 and bfcte_3 are not useful.

STRUCT(COALESCE(`bfcol_3`, 0) AS `bfpart1_0`, COALESCE(`bfcol_3`, 1) AS `bfpart2_0`) IN (
(
SELECT
STRUCT(COALESCE(`bfcol_4`, 0) AS `bfpart1_0`, COALESCE(`bfcol_4`, 1) AS `bfpart2_0`)
Copy link
Contributor

Choose a reason for hiding this comment

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

This query has a syntax error: Unrecognized name: bfcol_4

FROM (
SELECT
*,
STRUCT(COALESCE(`bfcol_3`, 0) AS `bfpart1_0`, COALESCE(`bfcol_3`, 1) AS `bfpart2_0`) IN (
Copy link
Contributor

Choose a reason for hiding this comment

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

The bfpart1_0 and bfpart2_0 looks redundancy. bfpart_0 and bfpart_1 would be better. However, these are local alias so that it may not need to generate a global UID. If so, we can use bfpart1/bfpart or other more readable variable naming.

@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Mar 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. size: xl Pull request size is extra large.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants