feat: select-query-parser-v2

[avallete]

What kind of change does this PR introduce?

• Introduces new, separate code for the parser and result types, each tested independently.
• Adds corresponding type tests for each runtime test case.
• Fixes several bugs related to type result inference.

What is the current behavior?

There are several result type mismatches referenced across multiple issues:

• Inconsistent Typing for Joined Field in Supabase #546
• [supabase-js] Parser error for using aggregate and spread #524
• Typing issue when querying children #509
• Types for column name disambiguation #450
• Database typings are not correct on table join for entity to one relationship #408

I have reproduced these issues in test cases with similar topologies to verify runtime behavior and ensure proper type inference.

What is the new behavior?

The rewrite should resolve all the related issues regarding select operations that I’ve encountered so far. However, I may have missed some cases, and there could be regressions in edge cases (despite passing all existing tests). Given the number of related issues, I’m unsure if the previous test coverage was sufficient. I recommend to deploy that out over an rc version first in coordination with supabase-js rc candidate so we can gather feedbacks about possibles regression before making the switch.

Additionally, we might need to evaluate how this performs on large databases with many tables and relations. The use of recursive types could potentially introduce performance concerns in such cases.

Additional context

I didn’t rewrite the original parser for fun. After spending a considerable amount of time trying to fix the existing one (I swear I tried see: #554 😓 ), it became clear that starting fresh with a cleaner, more testable, and debuggable version was necessary. I know it make for a big PR but I wasn't able to really reduce more than that the amount of changes, sorry 😅

[avallete]

Alright !

I've been able to use this new version within the larger codebase without lint errors in worker/api/monitoring so I'm now more confident that there should be little regression with this new parser version.

Only change I had to make on the supabase-js side of things were for rpc calls and porting the relationships types to be 1-1 with postgrest-js:

diff --git a/src/SupabaseClient.ts b/src/SupabaseClient.ts
index 9f0a32c..3ca6f8e 100644
--- a/src/SupabaseClient.ts
+++ b/src/SupabaseClient.ts
@@ -221,7 +221,9 @@ export default class SupabaseClient<
         ? Fn['Returns'][number]
         : never
       : never,
-    Fn['Returns']
+    Fn['Returns'],
+    null,
+    null
   > {
     return this.rest.rpc(fn, args, options)
   }
diff --git a/src/lib/types.ts b/src/lib/types.ts
index 85531a0..0b8d329 100644
--- a/src/lib/types.ts
+++ b/src/lib/types.ts
@@ -80,16 +80,26 @@ export type SupabaseClientOptions<SchemaName> = {
   accessToken?: () => Promise<string>
 }
 
+export type GenericRelationship = {
+  foreignKeyName: string
+  columns: string[]
+  isOneToOne?: boolean
+  referencedRelation: string
+  referencedColumns: string[]
+}
+
 export type GenericTable = {
   Row: Record<string, unknown>
   Insert: Record<string, unknown>
   Update: Record<string, unknown>
+  Relationships: GenericRelationship[]
 }
 
 export type GenericUpdatableView = GenericTable
 
 export type GenericNonUpdatableView = {
   Row: Record<string, unknown>
+  Relationships: GenericRelationship[]
 }
 
 export type GenericView = GenericUpdatableView | GenericNonUpdatableView

So this is now ready for review whenever you have time for it @soedirgo 👍

[whollacsek]

Would this PR fix type error when using virtual columns (ie. calling postgres function as column)?

[avallete]

Would this PR fix type error when using virtual columns (ie. calling postgres function as column)?

Hey, I'm not sure what issue you're referring too I don't think so, from my understanding the only way to call functions in postgrest is via RPC (https://docs.postgrest.org/en/v12/references/api/functions.html).

The only way I think you could achieve to use them as columns would be via some kind of views maybe ?

Also note that this PR does not impact any runtime behavior, only make the result types inference match it better.

[whollacsek]

What I was referring to does work (see: https://docs.postgrest.org/en/latest/references/api/computed_fields.html#computed-fields), my question is about fixing the generated type as the typescript error message is preventing builds of my next.js app to succeed. I've been forced to add ts-ignore everywhere since 2 years and I was thinking that this PR sounds like it would fix this issue

[avallete]

What I was referring to does work (see: https://docs.postgrest.org/en/latest/references/api/computed_fields.html#computed-fields), my question is about fixing the generated type as the typescript error message is preventing builds of my next.js app to succeed. I've been forced to add ts-ignore everywhere since 2 years and I was thinking that this PR sounds like it would fix this issue

Thank's for pointing out the case, I'll have a look, if it's correctly picked up as a column during introspection then it should work. Otherwise, if it's considered a function I don't think the current version will fix this issue. I'll make a test case to make sure of it tough.

[whollacsek]

I see in any case thank you for taking the time to improve this part of supabase SDK!

[avallete]

I'm locking the discussion to keep the focus on the PR review. Please open an issue or comment over a current one if you want to discuss details.