Skip to content
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

BUG(duckdb): Compiling STRUCTs throws away field name info, which is sometimes needed #4365

Closed
NickCrews opened this issue Nov 9, 2024 · 1 comment · Fixed by #4366
Closed
Assignees

Comments

@NickCrews
Copy link
Contributor

NickCrews commented Nov 9, 2024

Fully reproducible code snippet

import sqlglot
print(sqlglot.__version__)
# 25.28.1.dev12

inp = """SELECT CAST({'i':1, 's':'foo'} AS STRUCT("s" TEXT, "i" INT));"""
e = sqlglot.parse_one(inp, "duckdb")
actual = e.sql("duckdb")
print(actual)
# SELECT CAST(ROW(1, 'foo') AS STRUCT("s" TEXT, "i" INT));

If you run actual on https://shell.duckdb.org/, you get "Conversion Error: Could not convert string 'foo' to INT32".
This is because the names of the fields is important, and the simple (ROW(1, 'foo') that is currently given as output isn't adequate for this usage.

I would expect that actual == inp. I think this should work even if the struct values are columns instead of literals, eg {'i':my_int_col, 's':my_str_col} should roundtrip exactly.

I originally found this in this ibis issue, but I think this should be fixed at the sqlglot level. Curious if you will agree.

@VaggelisD
Copy link
Collaborator

Hey @NickCrews, thanks for reporting this.

The STRUCT -> ROW transformation was added to solve a BQ -> DuckDB transpilation path, but it does seem that it's not as flexible under casts as pure structs / struct_pack. On DuckDB's docs they're listed as synonymous ways of creating structs so I'd expect that they have similar semantics under the hood.

For now, I reverted a part of the transpilation path in order to restore DuckDB's STRUCT cast roundtrip.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants