Skip to content

fix(bigframes): Fix bugs compiling ambiguous ids and in subqueries #16617

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sycai merged 6 commits into from
Apr 13, 2026
Merged

fix(bigframes): Fix bugs compiling ambiguous ids and in subqueries #16617

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9741f50
fix(bigframes): Fix bugs compiling ambiguous ids and in subqueries
TrevorBergeron Apr 10, 2026
6b28e7e
reformat
TrevorBergeron Apr 10, 2026
375f162
update snapshots
TrevorBergeron Apr 10, 2026
564194a
update snapshots again
TrevorBergeron Apr 10, 2026
1708e33
update to_query tests
TrevorBergeron Apr 10, 2026
83f3119
fix isin behavior with left null
TrevorBergeron Apr 11, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions packages/bigframes/bigframes/core/compile/compiled.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -381,6 +381,7 @@ def isin_join(
new_column = (
(left_table[conditions[0]])
.isin((right_table[conditions[1]]))
.fillna(False)
.name(indicator_col)
)

Expand Down
8 changes: 4 additions & 4 deletions packages/bigframes/bigframes/session/_io/bigquery/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -516,19 +516,19 @@ def to_query(
) -> str:
"""Compile query_or_table with conditions(filters, wildcards) to query."""
if is_query(query_or_table):
sub_query = f"({query_or_table})"
from_item = f"({query_or_table})"
else:
# Table ID can have 1, 2, 3, or 4 parts. Quoting all parts to be safe.
# See: https://cloud.google.com/bigquery/docs/reference/standard-sql/lexical#identifiers
parts = query_or_table.split(".")
sub_query = ".".join(f"`{part}`" for part in parts)
from_item = ".".join(f"`{part}`" for part in parts)

# TODO(b/338111344): Generate an index based on DefaultIndexKind if we
# don't have index columns specified.
if columns:
# We only reduce the selection if columns is set, but we always
# want to make sure index_cols is also included.
select_clause = "SELECT " + ", ".join(f"`{column}`" for column in columns)
select_clause = "SELECT " + ", ".join(f"`_bf_source`.`{column}`" for column in columns)
else:
select_clause = "SELECT *"
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 10, 2026

Choose a reason for hiding this comment

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

medium

To maintain consistency with the qualified column selection in the if block and to further prevent ambiguity when this query is used as a subquery, consider qualifying the wildcard selector as well.

Suggested change
select_clause = "SELECT *"
select_clause = "SELECT _bf_source.*"


Expand All @@ -545,7 +545,7 @@ def to_query(

return (
f"{select_clause} "
f"FROM {sub_query}"
f"FROM {from_item} AS _bf_source"
f"{time_travel_clause}{where_clause}{limit_clause}"
)

Expand Down
2 changes: 1 addition & 1 deletion packages/bigframes/bigframes/session/polars_executor.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ def _is_node_polars_executable(node: nodes.BigFrameNode):
return False
for expr in node._node_expressions:
if isinstance(expr, agg_expressions.Aggregation):
if not type(expr.op) in _COMPATIBLE_AGG_OPS:
if type(expr.op) not in _COMPATIBLE_AGG_OPS:
return False
if isinstance(expr, expression.Expression):
if not set(map(type, _get_expr_ops(expr))).issubset(_COMPATIBLE_SCALAR_OPS):
Expand Down
13 changes: 8 additions & 5 deletions packages/bigframes/third_party/bigframes_vendored/sqlglot/parser.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4469,6 +4469,14 @@ def _parse_table(
if schema:
return self._parse_schema(this=this)

# see: https://docs.cloud.google.com/bigquery/docs/reference/standard-sql/query-syntax#from_clause
# from_item, then alias, then time travel, then sample.
alias = self._parse_table_alias(
alias_tokens=alias_tokens or self.TABLE_ALIAS_TOKENS
)
if alias:
this.set("alias", alias)
Comment on lines +4480 to +4484
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Apr 10, 2026

Choose a reason for hiding this comment

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

medium

Moving the alias parsing to the top of _parse_table correctly addresses BigQuery's syntax requirements (where the alias precedes the FOR SYSTEM_TIME AS OF clause). However, this change unconditionally parses the alias before the table sample, which may break dialects that set ALIAS_POST_TABLESAMPLE = True. Since this is a vendored parser, if it's intended to support multiple dialects, consider making this move conditional on the dialect's settings or ensuring that the alias can still be parsed after the sample if needed.


version = self._parse_version()

if version:
Expand All @@ -4477,11 +4485,6 @@ def _parse_table(
if self.dialect.ALIAS_POST_TABLESAMPLE:
this.set("sample", self._parse_table_sample())

alias = self._parse_table_alias(
alias_tokens=alias_tokens or self.TABLE_ALIAS_TOKENS
)
if alias:
this.set("alias", alias)

if self._match(TokenType.INDEXED_BY):
this.set("indexed", self._parse_table_parts())
Expand Down
Loading