feat(typegen): add functions setof type introspection #971
Conversation
- Introspect the setof function fields for functions - Restore functions as unions of args + returns
avallete
force-pushed
the
feat/add-functions-setof-type-introspection-v2
branch
from
317219e to
64a1afc
Compare
Pull Request Test Coverage Report for Build 17789949268Details
💛 - Coveralls |
src/lib/PostgresMetaTypes.ts
Outdated
| or ( | ||
| select | ||
| c.relkind ${includeTableTypes ? `in ('c', 'r')` : `= 'c'`} | ||
| c.relkind ${includeTableTypes ? `in ('c', 'r', 'v')` : `= 'c'`} |
There was a problem hiding this comment.
comment
Include functions that loops to a view for embedded functions.
| select 1 from pg_class c | ||
| where c.oid = rt.typrelid | ||
| -- exclude custom types relation from what is considered a set of table | ||
| and c.relkind in ('r', 'p', 'v', 'm', 'f') |
There was a problem hiding this comment.
comment
All possible function embeding relations:
r | ordinary table
p | partitioned table
v | view
m | materialized view
f | foreign table
| for (const column of columns) { | ||
| if (column.table_id in columnsByTableId) { | ||
| columnsByTableId[column.table_id].push(column) | ||
| } | ||
| } |
There was a problem hiding this comment.
comment
Use traditional for loop to avoid the need to loop twice for filter then foreach/push.
| for (const schema in introspectionBySchema) { | ||
| introspectionBySchema[schema].tables.sort((a, b) => a.name.localeCompare(b.name)) | ||
| introspectionBySchema[schema].views.sort((a, b) => a.name.localeCompare(b.name)) | ||
| introspectionBySchema[schema].functions.sort((a, b) => a.fn.name.localeCompare(b.fn.name)) | ||
| introspectionBySchema[schema].enums.sort((a, b) => a.name.localeCompare(b.name)) | ||
| introspectionBySchema[schema].compositeTypes.sort((a, b) => a.name.localeCompare(b.name)) | ||
| } |
There was a problem hiding this comment.
comment
Group the place where we sort things for consistency to dedup the places where sorts are needed.
| // OR if the function takes a table row but doesn't qualify as embedded (for error reporting) | ||
| (inArgs[0].table_name && !func.return_table_name))) | ||
| ) { | ||
| introspectionBySchema[func.schema].functions.push({ fn: func, inArgs }) |
There was a problem hiding this comment.
comment
We save the inArgs along with the function definition as it'll be needed in many places when generating the function return types. Since it's already available keep it along for re-use.
| if (conflictingFns.length > 0) { | ||
| const conflictingFn = conflictingFns[0] | ||
| const returnTypeName = typesById[conflictingFn.fn.return_type_id]?.name || 'unknown' | ||
| return `Could not choose the best candidate function between: ${schema.name}.${fn.name}(), ${schema.name}.${fn.name}( => ${returnTypeName}). Try renaming the parameters or the function itself in the database so function overloading can be resolved` |
There was a problem hiding this comment.
comment
This'll be used to warn the user at compile time that his function won't be available in postgrest due to conflicts.
| } | ||
| ${ | ||
| 'is_updatable' in view && view.is_updatable | ||
| view.is_updatable |
There was a problem hiding this comment.
comment
Now we fill up the is_updatable field with false if necessary: https://github.com/supabase/postgres-meta/pull/971/files#diff-c903ec71df16a8ae5f665c19c21b273f74c2f8eb02f5e8a3552ebe69d18be18fR76-R79
| for (const fnName in schemaFunctionsGroupedByName) { | ||
| schemaFunctionsGroupedByName[fnName].sort((a, b) => | ||
| b.fn.definition.localeCompare(a.fn.definition) | ||
| ) | ||
| } |
There was a problem hiding this comment.
comment
With functions having multiple definition overrides (same name) we sort them by definition text value. We might want to do another approach to have more stability 🤔
🚀 Canary Deployment Status✅ Canary image deployed successfully! 🐳 Docker Image: You can test this canary deployment by pulling the image: docker pull supabase/postgres-meta:canary-pr-971-1e4daa0e55e4672c04d026423c1a54329ff13376You can also set the version in a supabase local project by running: echo "supabase/postgres-meta:canary-pr-971-1e4daa0e55e4672c04d026423c1a54329ff13376" > supabase/.temp/pgmeta-versionOr use it in your docker-compose.yml: services:
postgres-meta:
image: supabase/postgres-meta:canary-pr-971-1e4daa0e55e4672c04d026423c1a54329ff13376
# ... other configurationThe canary image is available on: false Last updated: 2025-09-17T07:35:04Z |
| case | ||
| when f.proretset and rt.typrelid != 0 and exists ( | ||
| select 1 from pg_class c | ||
| where c.oid = rt.typrelid | ||
| -- exclude custom types relation from what is considered a set of table | ||
| and c.relkind in ('r', 'p', 'v', 'm', 'f') | ||
| ) then true | ||
| else false | ||
| end as returns_set_of_table, | ||
| case | ||
| when rt.typrelid != 0 then | ||
| (select relname from pg_class where oid = rt.typrelid) | ||
| else null | ||
| end as return_table_name, | ||
| case | ||
| when f.proretset then | ||
| coalesce(f.prorows, 0) > 1 | ||
| else false | ||
| end as returns_multiple_rows, |
There was a problem hiding this comment.
Let's not do select from pg_class within result column expressions - this might cause performance issues.
returns_set_of_table should be redundant with is_set_returning_function and return_type_relation_id (is_set_returning_function && return_type_relation_id !== null)
And return_table_name should be redundant with return_type_relation_id since we can get the table name from the /tables output using the relation id
| when f.proretset then | ||
| coalesce(f.prorows, 0) > 1 | ||
| else false | ||
| end as returns_multiple_rows, |
There was a problem hiding this comment.
I'm thinking instead of returns_multiple_rows we can just return prorows as is (setting it as null if 0)
It's easy to confuse it with is_set_returning_function since they mean the same thing; rather what we're doing is adhering to PostgREST's idiosyncrasies
|
Can we split out the refactor into another PR? It's difficult to see which change is related to computed relationships. |
What kind of change does this PR introduce?
Add all the necessary informations for embeded functions type inferences to work in postgrest-js (see: feat(types): add embeded functions type inference postgrest-js#632 )
Canary tested that it's mostly retro-compatible by generating the types on the infrastructure codebase. The new type definition didn't raised much type errors but we don't use a lot of functions/
rpccalls in it. So there is still a chance for this to break some codebases. Best approach might be to release this one as an-rcrelease so it can be opt-in via local cliecho "vXX-rc" > supabase/.temp/postgres-meta-version && supabase gen typesThat way with a canary release of the
postgrest-jsand thesupabase-jswe might release gradually with a way for people to opt-out if it causes issue.I'm open to suggestion about how to release this !
Closes: PGMETA-61