refactor: Define sql nodes and transform #2438
Conversation
product-auto-label
bot
added
size: xl
| """Compiles a BigFrameNode according to the request into SQL using SQLGlot.""" | ||
|
|
||
| output_names = tuple((expression.DerefOp(id), id.sql) for id in request.node.ids) | ||
| result_node = nodes.ResultNode( |
There was a problem hiding this comment.
IIUC, the ResultNode shares some functionality and properties with the new SelectNode. Can we refactor them together to reduce code complexity?
There was a problem hiding this comment.
ehh, they are similar, but are used at different layers of compilation, so I am hesitant to merge them. Maybe later on when code becomes more stable when can refactor commonalities more confidently
...it/core/compile/sqlglot/aggregations/snapshots/test_unary_compiler/test_any_w_window/out.sql
Outdated
Show resolved
Hide resolved
...unit/core/compile/sqlglot/aggregations/snapshots/test_unary_compiler/test_max/window_out.sql
Outdated
Show resolved
Hide resolved
| SELECT | ||
| *, | ||
| `int64_col`, | ||
| `int64_col` AS `bfcol_6`, |
There was a problem hiding this comment.
Maybe it's not related to this PR. The alias renaming seems to unnecessary here.
There was a problem hiding this comment.
Yeah, I think we can do more with aggregations, but (non-windowed) aggregations aren't in scope for this PR
| `time_col`, | ||
| `timestamp_col` | ||
| FROM `bigframes-dev`.`sqlglot_test`.`scalar_types` FOR SYSTEM_TIME AS OF '2025-11-09T03:04:05.678901+00:00' | ||
| ) | ||
| SELECT | ||
| `bool_col`, |
There was a problem hiding this comment.
we can still apply is_star_selection optimization here.
There was a problem hiding this comment.
yeah, I think we probably can, will leave further optimizations for later PRs
3 participants
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:
Fixes #<issue_number_goes_here> 🦕