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(binder): wrong type inference for struct with multiple fields in CASE expression #7189

Closed
kwannoel opened this issue Jan 4, 2023 · 3 comments · Fixed by #7202
Closed
Assignees
Labels
component/frontend Protocol, parsing, binder. found-by-sqlsmith type/bug Something isn't working

Comments

@kwannoel
Copy link
Contributor

kwannoel commented Jan 4, 2023

Describe the bug

Single field struct

dev=> select CASE WHEN false THEN ROW(1.1) ELSE ROW(0) END;
 case
------
 (0)
(1 row)

dev=> EXPLAIN select CASE WHEN false THEN ROW(1.1) ELSE ROW(0) END;
                                       QUERY PLAN
----------------------------------------------------------------------------------------
 BatchProject { exprs: [Case(false:Boolean, Row(1.1:Decimal), Row(0:Int32::Decimal))] }
 └─BatchValues { rows: [[]] }
(2 rows)

Multiple field

dev=> select CASE WHEN false THEN ROW(1.1, INTERVAL '1') ELSE ROW(0, INTERVAL '1') END;
ERROR:  QueryError: Bind error: types Struct(StructType { fields: [Decimal, Interval], field_names: [] }) and Struct(StructType { fields: [Int32, Interval], field_names: [] }) cannot be matched
dev=>

To Reproduce

select CASE WHEN false THEN ROW(1.1, INTERVAL '1') ELSE ROW(0, INTERVAL '1') END;

Expected behavior

postgres=# select CASE WHEN false THEN ROW(1.1, INTERVAL '1') ELSE ROW(0, INTERVAL '1') END;
     row
--------------
 (0,00:00:01)
(1 row)

Additional context

No response

@kwannoel kwannoel added the type/bug Something isn't working label Jan 4, 2023
@github-actions github-actions bot added this to the release-0.1.16 milestone Jan 4, 2023
@kwannoel kwannoel added component/frontend Protocol, parsing, binder. found-by-sqlsmith labels Jan 4, 2023
@kwannoel kwannoel changed the title bug(binder): wrong type inference for struct with 2 or more fields bug(binder): wrong type inference for struct with 2 or more fields in CASE expression Jan 4, 2023
@kwannoel kwannoel changed the title bug(binder): wrong type inference for struct with 2 or more fields in CASE expression bug(binder): wrong type inference for struct with multiple fields in CASE expression Jan 4, 2023
@kwannoel kwannoel changed the title bug(binder): wrong type inference for struct with multiple fields in CASE expression bug(binder): wrong type inference for struct with multiple fields in CASE expression Jan 4, 2023
@kwannoel kwannoel self-assigned this Jan 4, 2023
mergify bot pushed a commit that referenced this issue Jan 4, 2023
Add more input types to sqlsmith (`struct`, `list`).

### Motivation for refactoring `DataTypeName`

- Previously we use `DataTypeName` internally to indicate which expressions to generate.
- This is insufficient, since it elides `struct` and `list` internal types.
- Now we use `DataType` directly.
- In this PR we support generating these new variants as scalar values, and columns.
- Other PR may support generating functions, see: #7132.
- This will likely work by generating some variants of structs and lists we can choose from,
- During setup: insert function signatures which can work with these structs and lists.
- During setup: Define relations with these variants as columns.

### Misc

- Disable struct scalar due to #7189
- Disable timestamptz due to #5826

Approved-By: lmatz

Co-Authored-By: Tao Wu <wutao@singularity-data.com>
Co-Authored-By: Noel Kwan <noelkwan1998@gmail.com>
Co-Authored-By: Noel Kwan <47273164+kwannoel@users.noreply.github.com>
@xiangjinwu
Copy link
Contributor

It is due to the fact that we do not allow casting between the same type, so the 2nd field fails to match interval to interval 😂

.all(|(src, dst)| cast_ok(src, dst, allows))

Maybe changing this to src == dst || cast_ok(src, dst, allows) would help.

@mergify mergify bot closed this as completed in #7202 Jan 5, 2023
mergify bot pushed a commit that referenced this issue Jan 5, 2023
Fix row type alignment. As mentioned by @xiangjinwu [here](#7189 (comment)):
> It is due to the fact that we do not allow casting between the same type, so the 2nd field fails to match interval to interval 😂

Thanks to @xiangjinwu for pointing out the quick fix.

Approved-By: xiangjinwu
@kwannoel

This comment was marked as outdated.

@kwannoel
Copy link
Contributor Author

kwannoel commented Jan 5, 2023

Hmm... Seems like the following case is still not fixed.

select CASE WHEN true THEN ROW(1::int, interval '1') ELSE ROW(1.1::decimal, interval '1') END;
ERROR:  QueryError: Bind error: types Struct(StructType { fields: [Int32, Interval], field_names: [] }) and Struct(StructType { fields: [Decimal, Interval], field_names: [] }) cannot be matched

pg

postgres=# select CASE WHEN true THEN ROW(1::int, interval '1') ELSE ROW(1.1::decimal, interval '1') END;
     row
--------------
 (1,00:00:01)
(1 row)

Reopen to track.

Ignore this, it works as expected I was testing on the old branch 😶‍🌫️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component/frontend Protocol, parsing, binder. found-by-sqlsmith type/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants