Skip to content

Commit

Permalink
fix: remove pre-caching of remote function results (#1028)
Browse files Browse the repository at this point in the history
This will save redundant remote function execution as reported in
b/370088754. The trade-off is that any remote function integration
issues will be caughts only at a later point through a usage triggered
sql execution. Why the caching is not working as indended in the
reported use case in the bug? as per TrevorBergeron@ "the cached execution is useless when it is implicitly joined back to the base dataframe"
  • Loading branch information
shobsi authored Oct 1, 2024
1 parent 4af5bbb commit 0359bc8
Show file tree
Hide file tree
Showing 2 changed files with 3 additions and 13 deletions.
6 changes: 1 addition & 5 deletions bigframes/dataframe.py
Original file line number Diff line number Diff line change
Expand Up @@ -3567,11 +3567,7 @@ def apply(self, func, *, axis=0, args: typing.Tuple = (), **kwargs):
ops.NaryRemoteFunctionOp(func=func), series_list[1:]
)
result_series.name = None

# Return Series with materialized result so that any error in the remote
# function is caught early
materialized_series = result_series.cache()
return materialized_series
return result_series

# Per-column apply
results = {name: func(col, *args, **kwargs) for name, col in self.items()}
Expand Down
10 changes: 2 additions & 8 deletions bigframes/series.py
Original file line number Diff line number Diff line change
Expand Up @@ -1475,10 +1475,7 @@ def apply(
ops.RemoteFunctionOp(func=func, apply_on_null=True)
)

# return Series with materialized result so that any error in the remote
# function is caught early
materialized_series = result_series._cached(session_aware=False)
return materialized_series
return result_series

def combine(
self,
Expand Down Expand Up @@ -1506,10 +1503,7 @@ def combine(
other, ops.BinaryRemoteFunctionOp(func=func)
)

# return Series with materialized result so that any error in the remote
# function is caught early
materialized_series = result_series._cached()
return materialized_series
return result_series

@validations.requires_index
def add_prefix(self, prefix: str, axis: int | str | None = None) -> Series:
Expand Down

0 comments on commit 0359bc8

Please sign in to comment.