feat: Auto-plan complex reduction expressions by TrevorBergeron · Pull Request #2298 · googleapis/python-bigquery-dataframes

TrevorBergeron · 2025-11-26T01:03:16Z

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> 🦕

tswast

Neat! My main concern is around making sure we're all on the same page regarding terminology and intention for some of these internal classes and methods.

tswast · 2025-12-09T23:13:23Z

bigframes/core/array_value.py

+        *,
+        dropna: bool = False,
+    ):
+        # Warning: this function does not check if the expression is a valid reduction, and may fail spectacularly on invalid inputs


I wonder if there is anything we can do to get the type system to help out here?

I don't think the python system is sufficiently powerful to do this, instead, will just have to settle for runtime checks. In this case, will get a ValueError if expression is not an aggregating one.

bigframes/core/array_value.py

tswast · 2025-12-09T23:17:38Z

bigframes/core/blocks.py

            index_labels=self._index_labels,
        )

+    # This is a new experimental version of the aggregate that supports mixing analytic and scalar expressions\


Would be good to clarify how experimental. Are there particular combinations of inputs that aren't as well tested?

Eh, just going to remove this line. Its tested well over the domains its been applied to so far.

bigframes/core/blocks.py

tswast · 2025-12-09T23:23:45Z

bigframes/core/expression_factoring.py

 _MAX_INLINE_COMPLEXITY = 10


+def plan_general_col_exprs(


Like with compute, it might be good to document somewhere what we mean by "plan". Perhaps we should start a bigframes glossary?

Changing name to apply_agg_exprs_to_plan, where the plan is the BFET and the exprs are the column expressions describing the full aggregation.

bigframes/core/expression_factoring.py

bigframes/core/array_value.py

tswast · 2025-12-12T20:14:28Z

bigframes/core/array_value.py

+        """
+        Applies arbitrary aggregation expressions to the block, optionally grouped by keys.
+
+        This method handles reduction operations (e.g., sum, mean, count) that collapse


How do we handle methods like APPROX_TOP_COUNT that can't be used in windows?

SELECT pickup_datetime, trip_distance, APPROX_TOP_COUNT(trip_distance, 10) OVER (ORDER BY pickup_datetime ROWS BETWEEN 3 PRECEDING AND CURRENT ROW ) AS rolling_sum_trip_distance_4_trips FROM `bigquery-public-data`.`new_york`.`tlc_green_trips_2014` ORDER BY pickup_datetime;

Likewise, what about methods like PERCENTILE_CONT that require a window with an order by clause?

Presumably, these ops are handled at a diiferent layer to avoid such cases? It'd be good to be explicit in the docs here that ops provided need to work as both analytical and aggregate functions. Or if we can use such functions here, what to do to avoid windows / force group by.

yeah, best to catch these early. adding a windowizable prop to ops, and will throw if try to windowize non-windowizable op

tswast · 2025-12-12T20:26:21Z

bigframes/core/block_transforms.py


-    block, agg_ids = block.aggregate(
-        by_column_ids=grouping_column_ids, aggregations=aggregations
+    block, _ = block.reduce_general(


It's be worth a comment explaining why it's safe to ignore the second element of the returned tuple.

The second element is mostly not used, so just removing it from the method entirely

tswast · 2025-12-12T20:26:45Z

bigframes/core/block_transforms.py

+        kurt_exprs.append(kurt_expr)

-    block = block.select_columns(kurt_ids).with_column_labels(column_labels)
+    block, _ = block.reduce_general(


bigframes/core/block_transforms.py

tswast · 2025-12-12T21:18:53Z

bigframes/core/blocks.py

+        dropna: bool = True,
+    ) -> typing.Tuple[Block, typing.Sequence[str]]:
+        """
+        Version of the aggregate that supports mixing analytic and scalar expressions.


Since we still have aggregate, aggregate_size, and aggregate_all_and_stack let's say when to use them over this.

Alternatively, put an internal deprecated comment in the docstrings of those and file an issue for ourselves to cleanup uses of them.

good point, removing aggregate and aggregate_size in this PR now.

bigframes/core/blocks.py

tswast

Thanks!

TrevorBergeron added 3 commits November 25, 2025 22:17

feat: Auto-plan complex reduction expressions

cba77b8

port kurt, skew to new framework

4abc26c

fix many issues

c9a9921

product-auto-label bot added size: l Pull request size is large. api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. labels Nov 26, 2025

TrevorBergeron changed the title ~~Agg factoring~~ feat: Auto-plan complex reduction expressions Nov 26, 2025

factor out data structures to classes

b2e4efa

TrevorBergeron requested a review from tswast December 1, 2025 17:43

TrevorBergeron marked this pull request as ready for review December 1, 2025 17:43

TrevorBergeron requested review from a team as code owners December 1, 2025 17:43

blunderbuss-gcf bot assigned liujiashang Dec 1, 2025

tswast reviewed Dec 9, 2025

View reviewed changes

TrevorBergeron added 2 commits December 12, 2025 01:36

add documentation

1e53fa9

Merge remote-tracking branch 'github/main' into agg_factoring

bf3c6bb

TrevorBergeron requested a review from tswast December 12, 2025 20:27

tswast reviewed Dec 12, 2025

View reviewed changes

TrevorBergeron added 5 commits December 15, 2025 19:52

Make old aggregates use new block method

0b040c0

remove aggregate_size block method

60ba975

add windowizable prop

37869a7

fix windowizable predicate

1d0514b

add comment for unpartitioned agg index assignemtn

a7a0399

TrevorBergeron requested a review from tswast December 15, 2025 20:21

fix usage of amended method

6e76219

tswast approved these changes Dec 15, 2025

View reviewed changes

TrevorBergeron merged commit 4d5de14 into main Dec 15, 2025
24 of 25 checks passed

TrevorBergeron deleted the agg_factoring branch December 15, 2025 22:13

ldetmer mentioned this pull request Jan 6, 2026

chore: create a release #2363

Merged

Conversation

TrevorBergeron commented Nov 26, 2025

Uh oh!

tswast left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tswast left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants