-
Notifications
You must be signed in to change notification settings - Fork 50
fix: support JSON and STRUCT for bbq.sql_scalar #1754
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.
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
Conversation
cdd2edd
to
7244c33
Compare
literals = [ | ||
bigframes.dtypes.bigframes_dtype_to_literal(column.dtype) for column in columns | ||
literals_sql = [ | ||
sqlglot_ir._literal(column.values[0], column.dtype).sql(dialect="bigquery") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
column.values
can be quite expensive. That's going to download everything as a numpy array. Also, it might return NaN if the first row contains null.
Can we keep using bigframes_dtype_to_literal to get representative values for the type?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch! Yes, I've implemented it so that None
value generates literal SQL. This simplifies the logic, and it's confirmed by the test_sql_scalar_w_all_scalar_output
test. Would you mind double-checking this?
7244c33
to
84fd8fa
Compare
8384f2d
to
97b49e0
Compare
Fixes internal issue 416015997🦕