sqlc-dev / sqlc

Generate type-safe code from SQL
https://sqlc.dev
MIT License
13.37k stars 803 forks source link

Updated to v1.19.0 and sqlc generate no longer runs #2416

Closed mwhitm closed 1 year ago

mwhitm commented 1 year ago

Version

Other

What happened?

updated from v1.17.2 and get the following log when i run sqlc generate. I use brew to install sqlc and it looks like old versions are not being tracked in brew so we cannot downgrade.

Relevant log output

sqlc generate
panic: interface conversion: ast.Node is *ast.SQLValueFunction, not *ast.FuncCall

goroutine 19 [running]:
github.com/kyleconroy/sqlc/internal/compiler.(*Compiler).sourceTables(0x104b0f2e0?, 0x14000a7b4a0, {0x104d52198?, 0x14000d90dc0?})
    github.com/kyleconroy/sqlc/internal/compiler/output_columns.go:510 +0xdb4
github.com/kyleconroy/sqlc/internal/compiler.(*Compiler).outputColumns(0x14000114000, 0x14000a7b4a0, {0x104d52198?, 0x14000d90dc0})
    github.com/kyleconroy/sqlc/internal/compiler/output_columns.go:55 +0x38
github.com/kyleconroy/sqlc/internal/compiler.(*Compiler).buildQueryCatalog(0x104a95200?, 0x14000116000, {0x104d52198?, 0x14000d90fd0?}, {0x0, 0x0, 0x0})
    github.com/kyleconroy/sqlc/internal/compiler/query_catalog.go:35 +0x1c4
github.com/kyleconroy/sqlc/internal/compiler.(*Compiler).parseQuery(0x14000114000, {0x104d51f38?, 0x14000e97620?}, {0x14000203200, 0x8ba}, {{0x0?, 0x0?, {0x0?, 0x0?}, 0x0?}})
    github.com/kyleconroy/sqlc/internal/compiler/parse.go:93 +0x65c
github.com/kyleconroy/sqlc/internal/compiler.(*Compiler).parseQueries(0x14000114000, {{0x0?, 0x0?, {0x0?, 0x1?}, 0x1?}})
    github.com/kyleconroy/sqlc/internal/compiler/compile.go:80 +0x3bc
github.com/kyleconroy/sqlc/internal/compiler.(*Compiler).ParseQueries(...)
    github.com/kyleconroy/sqlc/internal/compiler/engine.go:49
github.com/kyleconroy/sqlc/internal/cmd.parse({_, _}, {_, _}, {_, _}, {{0x140004ac030, 0xa}, {0x14000110000, 0x1, ...}, ...}, ...)
    github.com/kyleconroy/sqlc/internal/cmd/generate.go:350 +0x1e0
github.com/kyleconroy/sqlc/internal/cmd.Generate.func1()
    github.com/kyleconroy/sqlc/internal/cmd/generate.go:224 +0x654
golang.org/x/sync/errgroup.(*Group).Go.func1()
    golang.org/x/sync@v0.3.0/errgroup/errgroup.go:75 +0x5c
created by golang.org/x/sync/errgroup.(*Group).Go
    golang.org/x/sync@v0.3.0/errgroup/errgroup.go:72 +0xa0

Database schema

No response

SQL queries

No response

Configuration

No response

Playground URL

No response

What operating system are you using?

Linux, macOS

What database engines are you using?

No response

What type of code are you generating?

Go

andrewmbenton commented 1 year ago

Sorry about that. We'll need to take a deeper look, but in the meantime if you need an older version you can grab a pre-built binary from the v1.17.2 GitHub release page or from https://downloads.sqlc.dev/

andrewmbenton commented 1 year ago

The bug is an invalid type assumption in internal/compiler/output_columns#510.

I can fix this with a type assertion and an error bail-out when it fails, but I'm not sure that's the correct thing to do. The offending commit looks like it was part of this PR by @haines and @kyleconroy so hopefully one of them can advise.

kyleconroy commented 1 year ago

@mwhitm We're going to fix the panic, but v1.19.0 will still return an error. To fix the regression, we'll need a bit more information from you. Can you include the query (with database tables) that's causing the issue?

mwhitm commented 1 year ago

Hi Kyle, it happens when i run sqlc generate (needed to build our application). For completeness i also ran the other cli functions and get the same thing.

So it's not on a query per se but building generating the files. I am using postgres.

andrewmbenton commented 1 year ago

@mwhitm the bug occurs as part of a query parsing step, so it would be ideal if we had access to the offending query or queries. I can fix the panic, but I would also like to ensure that sqlc output is correct for your queries.

I'd be happy to take a look at your queries and see if I can figure it out, if you can attach your query.sql (or whatever files contain your queries) to an email to hello@sqlc.dev.

andrewmbenton commented 1 year ago

Digging through the pganalyze source a bit led me to try a few different test-cases, and I have one that triggers this panic (using postgres):

SELECT * FROM CURRENT_DATE;

I'll work on an endtoend test and a bug fix using this as a starting point.

j0holo commented 1 year ago

I have a similar issue with 1.19.0.

(select id
from chat
where id < ? or id = (select min(id) from chat)
order by id desc
limit 1)
union all
(select id
from chat
where id > ? or id = (select max(id) from chat)
order by id asc
limit 1);

Commenting this query out works like a charm, but generating with this query gives the following error:

panic: interface conversion: ast.Node is *ast.SetOprSelectList, not *ast.SelectStmt

goroutine 6 [running]:
github.com/kyleconroy/sqlc/internal/engine/dolphin.(*cc).convertSetOprSelectList(0xc000a7ebc0?, 0xc0001e4930)
        /go/pkg/mod/github.com/kyleconroy/sqlc@v1.19.0/internal/engine/dolphin/convert.go:1212 +0x505
github.com/kyleconroy/sqlc/internal/engine/dolphin.(*cc).convertSetOprStmt(0x20?, 0xc000100000?)
        /go/pkg/mod/github.com/kyleconroy/sqlc@v1.19.0/internal/engine/dolphin/convert.go:1255 +0x7b
github.com/kyleconroy/sqlc/internal/engine/dolphin.(*cc).convert(0xc000810000?, {0x2804eb0?, 0xc0001e48c0?})
        /go/pkg/mod/github.com/kyleconroy/sqlc@v1.19.0/internal/engine/dolphin/convert.go:1735 +0x20f6
github.com/kyleconroy/sqlc/internal/engine/dolphin.(*Parser).Parse(0xc000015130, {0x27f0380?, 0xc000a4e0a0?})
        /go/pkg/mod/github.com/kyleconroy/sqlc@v1.19.0/internal/engine/dolphin/parse.go:63 +0x205
github.com/kyleconroy/sqlc/internal/compiler.(*Compiler).parseQueries(0xc00021dc00, {{0x40?, 0xf1?, {0x0?, 0x1?}, 0x1?}})
        /go/pkg/mod/github.com/kyleconroy/sqlc@v1.19.0/internal/compiler/compile.go:74 +0x26d
github.com/kyleconroy/sqlc/internal/compiler.(*Compiler).ParseQueries(...)
        /go/pkg/mod/github.com/kyleconroy/sqlc@v1.19.0/internal/compiler/engine.go:49
github.com/kyleconroy/sqlc/internal/cmd.parse({_, _}, {_, _}, {_, _}, {{0xc0004520f6, 0x5}, {0xc00043f140, 0x1, ...}, ...}, ...)
        /go/pkg/mod/github.com/kyleconroy/sqlc@v1.19.0/internal/cmd/generate.go:350 +0x285
github.com/kyleconroy/sqlc/internal/cmd.Generate.func1()
        /go/pkg/mod/github.com/kyleconroy/sqlc@v1.19.0/internal/cmd/generate.go:224 +0x827
golang.org/x/sync/errgroup.(*Group).Go.func1()
        /go/pkg/mod/golang.org/x/sync@v0.3.0/errgroup/errgroup.go:75 +0x64
created by golang.org/x/sync/errgroup.(*Group).Go
        /go/pkg/mod/golang.org/x/sync@v0.3.0/errgroup/errgroup.go:72 +0xa5

When installing sqlc via go install I also get the following error:

# github.com/kyleconroy/sqlc/cmd/sqlc
/usr/bin/ld: warning: x86_64.o: missing .note.GNU-stack section implies executable stack
/usr/bin/ld: NOTE: This behaviour is deprecated and will be removed in a future version of the linker

But sqlc version and go generate work fine without the above query.

UPDATE: Using the precompiled binary also gives the same error.

sqlc playground does not give me any issues with this query: https://play.sqlc.dev/p/d2f08367622724c2eb6f9402ef6f5a70e035e5279774d3ad3f4accd75a59a7ee

j0holo commented 1 year ago

1.14.0 the previous version we used also returns the same error panic: interface conversion: ast.Node is *ast.SetOprSelectList, not *ast.SelectStmt with the UNION ALL query.

andrewmbenton commented 1 year ago

Regarding the initial *ast.SQLValueFunction interface conversion issue, I have a "fix" for that here: https://github.com/kyleconroy/sqlc/pull/2449.

@j0holo I think there are two unrelated issues you've discovered. Regarding the /usr/bin/ld: warning you're getting from the linker when running sqlc, is that preventing sqlc from running correctly on all of your queries that don't cause panics? Or does sqlc run correctly in those cases?

Regarding the panic from *ast.SetOprSelectList interface conversion, we'll need to create a separate issue for that. I'll try to create the issue this morning and comment here, but feel free to create the issue if you don't see a comment from me yet.

andrewmbenton commented 1 year ago

I've created an issue for the *ast.SetOprSelectList panic: https://github.com/kyleconroy/sqlc/issues/2453.

I'm closing this issue now, but if the linker warning is a blocking issue please open another GH issue to track it.

j0holo commented 1 year ago

I think there are two unrelated issues you've discovered. Regarding the /usr/bin/ld: warning you're getting from the linker when running sqlc, is that preventing sqlc from running correctly on all of your queries that don't cause panics? Or does sqlc run correctly in those cases?

@andrewmbenton sqlc does run fine while throwing this /usr/bin/ld warning. Possibly because it is deprecated but not yet removed from the linker.