feat: support time range rolling on Series. #1590

sycai · 2025-04-03T23:33:03Z

No description provided.

TrevorBergeron

I think you will also want to modify the ordering pull-up logic to not pull ordering into range windows? Should be some similar logic already there.

TrevorBergeron · 2025-04-07T21:26:17Z

bigframes/core/compile/compiled.py

@@ -579,6 +593,19 @@ def _convert_ordering_to_table_values(
    return ordering_values


+def _convert_range_ordering_to_table_value(


what makes this one different? is it that it doesn't allow NULLS FIRST/LAST overrides and only allows one column?

Yeah. If we have to deal with NULLs, there will be multiple expressions after "ORDER BY", but it is not allowed by SQL window syntax.

Added doc that explains the reasons for future references.

TrevorBergeron · 2025-04-07T21:27:25Z

bigframes/core/compile/compiled.py

+            start=_to_ibis_boundary(bounds.start_micros),
+            end=_to_ibis_boundary(bounds.end_micros),


Why define as micros? Sure, now its always micros, but not really a necessary property.

It has something to do with dealing with nullness. Property removed.

TrevorBergeron · 2025-04-07T21:30:02Z

bigframes/core/rewrite/windows.py

+from bigframes.core import nodes
+
+
+def rewrite_range_rolling(node: nodes.BigFrameNode) -> nodes.BigFrameNode:


I know I always propose compile-time rewrites, but I think validations need to happen earlier, in the API. The tree should always be valid, rewrites just transform from one valid tree to another.

Are you talking about the length checking for ordering?

I will check the rolling window key on the API level too. Here this is simply to make sure we fail fast if some other error sneaks in.

TrevorBergeron · 2025-04-07T21:31:39Z

bigframes/core/window/rolling.py

+    spec = window_spec.WindowSpec(
+        bounds=window_spec.RangeWindowBounds.from_timedelta_window(window, closed),
+        min_periods=1 if min_periods is None else min_periods,
+        ordering=(
+            ordering.OrderingExpression(
+                ex.deref(block.index_columns[0]), order_direction
+            ),
+        ),
+    )


I do like this where we are picking the ordering column immediately and validating

TrevorBergeron · 2025-04-07T21:31:58Z

bigframes/core/window/rolling.py

+@singledispatch
+def _find_order_direction(


nit: probably should have tree stuff elsewhere, but whatever

Should be simple to refactor, though. I moved them to a separated file

TrevorBergeron · 2025-04-07T21:33:04Z

bigframes/core/window/rolling.py

+    if len(root.by) == 0:
+        return None


in this case, the node is a no-op so we can just call child

TIL. code updated.

yeah, all re-orderings are stable re-orderings, so it prepends to the existing ordering key rather than replaceing

TrevorBergeron · 2025-04-07T21:39:09Z

bigframes/core/window/rolling.py

+
+@_find_order_direction.register
+def _(root: nodes.FilterNode, column_id: str):
+    return _find_order_direction(root.child, column_id)


can probably also extend to the additive nodes (projection, window, isin), for which you can just ignore the contents and call child (or you can try to identify strictly increasing functions if you want to get fancy).

Added windowNode and inNode. The ProjectNode is left out because some operations may invalidate ordering (e.g. multiplication with a negative value)

ProjetNode is always additive, it doesn't mutate values, so it should be safe. If anything, it can provide alternative ordering keys if you know an operation is strictly increasing.

TrevorBergeron · 2025-04-08T00:22:50Z

bigframes/core/rewrite/windows.py

+
+    return dataclasses.replace(
+        node,
+        window_spec=dataclasses.replace(node.window_spec, ordering=(new_ordering,)),


it would be consistent to redefine the window spec bounds to integers rather than timestamp to be consistent with the underlying value now being an integer.

I think I still prefer the timedeltas because it describes best what the range windows are for

sycai added 3 commits April 3, 2025 21:27

[WIP] implement range rolling with mods at expression level

da0372e

Merge branch 'main' into sycai_rolling_window

87884ba

implement range rolling for series

d253ca7

product-auto-label bot added size: m Pull request size is medium. api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. labels Apr 3, 2025

fix lint

e873c07

product-auto-label bot added size: l Pull request size is large. and removed size: m Pull request size is medium. labels Apr 3, 2025

Merge branch 'main' into sycai_rolling_window

87f265b

sycai force-pushed the sycai_rolling_window branch from 81811e2 to 87f265b Compare April 4, 2025 00:20

update rewrite logic

6222c5e

sycai force-pushed the sycai_rolling_window branch from a4cd9dc to 58e0764 Compare April 4, 2025 01:27

fix mypy

850ea41

sycai force-pushed the sycai_rolling_window branch from 58e0764 to 850ea41 Compare April 4, 2025 01:29

differentiate range bound and unbounded window

db6a353

sycai force-pushed the sycai_rolling_window branch from 5239077 to db6a353 Compare April 4, 2025 03:22

fix mypy

1928992

sycai marked this pull request as ready for review April 4, 2025 16:25

sycai requested review from a team as code owners April 4, 2025 16:25

sycai requested a review from drylks-work April 4, 2025 16:25

blunderbuss-gcf bot assigned rey-esp Apr 4, 2025

sycai requested review from TrevorBergeron and chelsea-lin and removed request for drylks-work April 4, 2025 16:25

sycai and others added 5 commits April 4, 2025 14:32

Merge branch 'main' into sycai_rolling_window

6967e24

Merge branch 'main' into sycai_rolling_window

91af2c5

Merge branch 'main' into sycai_rolling_window

f5113c3

relax sorting key search algo

ad53e3d

check whether window key is prefix

a242824

TrevorBergeron reviewed Apr 7, 2025

View reviewed changes

address comments

2e86a47

sycai force-pushed the sycai_rolling_window branch from 31447ea to 2e86a47 Compare April 7, 2025 23:04

sycai and others added 4 commits April 7, 2025 23:07

fix type hint

9a835cb

add comment on skipping range window during order pull up

ea59217

Merge branch 'main' into sycai_rolling_window

91d349e

fix lint

0bdaf17

sycai force-pushed the sycai_rolling_window branch from 5f87995 to 0bdaf17 Compare April 7, 2025 23:17

sycai added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 7, 2025

sycai requested a review from TrevorBergeron April 7, 2025 23:19

bigframes-bot removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Apr 7, 2025

check order for all AdditiveNode

cd6a666

TrevorBergeron approved these changes Apr 8, 2025

View reviewed changes

explicitly dispatch additive nodes

4b4c379

sycai merged commit 6e98a2c into main Apr 8, 2025
24 checks passed

sycai deleted the sycai_rolling_window branch April 8, 2025 02:02

release-please bot mentioned this pull request Apr 7, 2025

chore(main): release 2.0.0 #1552

Merged

		@@ -579,6 +593,19 @@ def _convert_ordering_to_table_values(
		return ordering_values


		def _convert_range_ordering_to_table_value(

		start=_to_ibis_boundary(bounds.start_micros),
		end=_to_ibis_boundary(bounds.end_micros),

		from bigframes.core import nodes


		def rewrite_range_rolling(node: nodes.BigFrameNode) -> nodes.BigFrameNode:

feat: support time range rolling on Series. #1590

feat: support time range rolling on Series. #1590

Uh oh!

Conversation

sycai commented Apr 3, 2025

Uh oh!

TrevorBergeron 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!

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!

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!

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!

Uh oh!