Skip to content

Conversation

TrevorBergeron
Copy link
Contributor

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

@product-auto-label product-auto-label bot added size: s Pull request size is small. api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. labels Dec 14, 2024
@TrevorBergeron TrevorBergeron marked this pull request as ready for review December 16, 2024 19:13

# Remote ops are expensive so force reprojection before to ensure filters get applied first
any_remote_op = any(
ex.contains_op(expr, bigframes.operations.RemoteFunctionOp)
Copy link
Collaborator

@tswast tswast Dec 27, 2024

Choose a reason for hiding this comment

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

I'm a little bit concerned about hard coding remote function op here. Would it be better to flag each scalar operator as "expensive" / "cheap"?

Certainly there's a spectrum of costs possible, but a distinction could be made if there's an associated cost (e.g. cloud run CPU in this case) per call.

For example, LLM-style ops, BQML prediction (though that's not a scalar in BigQuery terms, though maybe could be in Bigframes eventually), could similarly have an "there's an extra cost associated here" flag.

@TrevorBergeron
Copy link
Contributor Author

Instead, removing all filter deferment in #1336

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: bigquery Issues related to the googleapis/python-bigquery-dataframes API. size: s Pull request size is small.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants