Add pg_tables and pg_views to catalog

[skabbes]

Fixes: https://github.com/kyleconroy/sqlc/issues/1747

Problem - I was unable to write a query against pg_timezone_names.

Actual failing query

select
  name,
  abbrev,
  utc_offset,
  to_char((current_timestamp at time zone name), 'HH:MI am') as current_time
from pg_catalog.pg_timezone_names
order by utc_offset;

This because sqlc did not know about these built-in tables / views.

Warning I tried to make sqlc-pg-gen and make regen, but I was getting slightly different output, so in this commit I only committed the changes that affected the tables, not the existing functions (which I do not know why they were changed, perhaps a different version of postgres).

[skabbes]

@kyleconroy I might need your help regenerating the files. I am having a heck-uva time.

➜  sqlc_skabbes git:(catalog_tables_views) ✗ make regen
go build -o ~/bin/sqlc-dev ./cmd/sqlc/
go run ./scripts/regenerate/
2022/07/16 11:35:01 internal/endtoend/testdata/wasm_plugin_sqlc_gen_greeter: sqlc-dev generate failed
# package greeter
error generating code: call: Exited with i32 exit status 1
wasm backtrace:
    0: 0x1cf49 - <unknown>!__wasi_proc_exit
note: using the `WASMTIME_BACKTRACE_DETAILS=1` environment variable to may show more debugging information

exit status 1
make: *** [regen] Error 1

[kyleconroy]

Very strange error message. What os / arch are you on?

[skabbes]

I'm on an M1 Mac, arm64 I suppose.

[skabbes]

I added the env var, and get this next error message.

➜  wasm_plugin_sqlc_gen_greeter git:(catalog_tables_views) ✗ WASMTIME_BACKTRACE_DETAILS=1 sqlc-dev generate
# package greeter
error generating code: loadModule: Module was compiled without WebAssembly backtrace support but it is enabled for the host

[skabbes]

@kyleconroy given this commit fixed the build, I think it has something to do with the pure size of the data or number of tables in pg_catalog.

I believe since all of a sudden there are tables in the pg_catalog schema, it is exporting them - but probably that's not what you want / wanted with your plugin design.

[kyleconroy]

I'm not sure what limit we're running into, but for the meantime let's drop the pg_catalog schema before sending it to the WASM plugin.

diff --git a/internal/ext/wasm/wasm.go b/internal/ext/wasm/wasm.go
index a55d8551..937ece21 100644
--- a/internal/ext/wasm/wasm.go
+++ b/internal/ext/wasm/wasm.go
@@ -175,7 +175,23 @@ func (r *Runner) loadWASM(ctx context.Context, cache string, expected string) ([
        return wmod, nil
 }
 
+// Due to some memory or file size limit, we must drop the pg_catalog
+// schema
+func removePGCatalog(req *plugin.CodeGenRequest) {
+       if req.Catalog != nil && req.Catalog.Schemas != nil {
+               schemas := req.Catalog.Schemas
+               for i, _ := range schemas {
+                       if schemas[i].Name == "pg_catalog" {
+                               req.Catalog.Schemas = append(schemas[:i], schemas[i+1:]...)
+                               return
+                       }
+               }
+       }
+}
+
 func (r *Runner) Generate(ctx context.Context, req *plugin.CodeGenRequest) (*plugin.CodeGenResponse, error) {
+       removePGCatalog(req)

[kyleconroy]

@skabbes Something is wrong with the updated pg-gen. We should be generating two functions for COUNT. Here's the current function signatures on main.

		{
			Name: "count",
			Args: []*catalog.Argument{
				{
					Type: &ast.TypeName{Name: "any"},
				},
			},
			ReturnType: &ast.TypeName{Name: "bigint"},
		},
		{
			Name:       "count",
			Args:       []*catalog.Argument{},
			ReturnType: &ast.TypeName{Name: "bigint"},
		},

And here's the new definition:

		{
			Name:       "count",
			Args:       []*catalog.Argument{},
			ReturnType: &ast.TypeName{Name: "bigint"},
		},
		{
			Name:       "count",
			Args:       []*catalog.Argument{},
			ReturnType: &ast.TypeName{Name: "bigint"},
		},

We're missing the any parameter. Maybe it's something to do with the cross join?

[skabbes]

No, sorry I didn't explain well. routines in Postgres can be regular functions, procedures, aggregates, and window functions. https://www.postgresql.org/docs/current/catalog-pg-proc.html

If it is an aggregate, there are 2 kinds of arguments, the "direct" arguments, and the "aggregate" arguments. Previously, sqlc did not differentiate between them, and classified all of them as "direct" arguments.

Most aggregate functions that are used don't have any "direct" arguments at all, so it all kinda worked out just fine. However, percentile_cont and friends have both direct and aggregate arguments that sqlc was not equipped to handle.

You can see that you manually overrode the pg_catalog just to make that work in this commit.

My commit did the work to differentiate between these direct and aggregate arguments to routines.

So when I said:

I think there needs to be more work in the compiler to treat / lookup these functions differently based on the context in the query. So not sure where to go from here.

What I meant was we need to make "looking up a function" (actually, a routine) smarter in the sqlc internals, before this can work. Either that, or re-apply the manual workaround you did from September 2021.

The reality is that the function percentile_disc actually takes 2 parameters, the 0.5 (direct arguments) and the within group (order by authors.name) (the aggregate argument).

select percentile_disc(0.5) within group (order by authors.name)

@kyleconroy I think looking forward for sqlc, it needs to differentiate between them. I'd be happy to put some work into making that a reality, but just wanted to check-in and make sure you agree that's the right path.

[kyleconroy]

You can see that you manually overrode the pg_catalog just to make that work in this commit.

I honestly forgot about the manual changes I made to pg_catalog.go. While it was the right thing to do to fix the bug, it should have been fixed in sqlc-pg-gen. I'll take the // Code generated by sqlc-pg-gen. DO NOT EDIT. seriously in the future.

I'd be happy to put some work into making that a reality, but just wanted to check-in and make sure you agree that's the right path.

Yep, I think we're on the right path. That said, we're now fixing something unrelated to the original intent of this PR (aggregate functions vs PostgreSQL internal tables / views). I'd be happy to split this up if you'd like to get the tables and views in first, but it's up to you.

[skabbes]

I think this: https://github.com/kyleconroy/sqlc/pull/1809

Is the first step in actually making progress here.

[skabbes]

I islolated the pg_* fixes in this PR: https://github.com/kyleconroy/sqlc/pull/1811

I think the "aggreagated function args vs direct args" can / should be addressed independently.

[kyleconroy]

Agreed. Going to close this PR.