Move overload disambiguation into functionResourceName inflector and simplify overload detection

Author: GeertJohan

graphile-build/graphile-build-pg/src/plugins/PgProceduresPlugin.ts

@@ -23,32 +23,11 @@ import {
   EXPORTABLE_OBJECT_CLONE,
   gatherConfig,
 } from "graphile-build";
-import type { PgProc, PgProcArgument, PgType } from "pg-introspection";
+import type { PgProc, PgProcArgument } from "pg-introspection";
 
 import { exportNameHint, forbidRequired } from "../utils.ts";
 import { version } from "../version.ts";
 
-/**
- * Given a type OID, walk through domain chains and return the type if
- * it resolves to a composite type (typtype 'c' with a typrelid), or
- * null otherwise.
- */
-function resolveToCompositeType(
-  typeOid: string,
-  types: ReadonlyArray<PgType>,
-): PgType | null {
-  let type = types.find((t) => t._id === typeOid);
-  // Follow domain chains (typtype 'd') to reach the underlying type
-  while (type && type.typtype === "d" && type.typbasetype) {
-    type = types.find((t) => t._id === type!.typbasetype!);
-  }
-  // Return only if it resolved to a composite (table) type
-  if (type && type.typtype === "c" && type.typrelid) {
-    return type;
-  }
-  return null;
-}
-
 declare global {
   namespace GraphileBuild {
     interface BehaviorStrings {
@@ -151,6 +130,20 @@ export const PgProceduresPlugin: GraphileConfig.Plugin = {
         }
         const pgNamespace = pgProc.getNamespace()!;
         const schemaPrefix = this._schemaPrefix({ serviceName, pgNamespace });
+        // For computed column functions whose name doesn't follow the
+        // tablename_funcname convention, prefix with the composite type
+        // name to ensure unique resource names. This allows overloaded
+        // functions like code(a.pets) and code(a.buildings) to produce
+        // distinct names (pets_code, buildings_code).
+        if (pgProc.provolatile !== "v") {
+          const firstArg = pgProc.getArguments().find((a) => a.isIn);
+          if (firstArg && firstArg.type.typtype === "c") {
+            const compositePrefix = firstArg.type.typname + "_";
+            if (!pgProc.proname.startsWith(compositePrefix)) {
+              return `${schemaPrefix}${firstArg.type.typname}_${pgProc.proname}`;
+            }
+          }
+        }
         return `${schemaPrefix}${pgProc.proname}`;
       },
       functionRecordReturnCodecName(options, details) {
@@ -235,68 +228,10 @@ export const PgProceduresPlugin: GraphileConfig.Plugin = {
           const executor =
             info.helpers.pgIntrospection.getExecutorForService(serviceName);
 
-          let name = info.inflection.functionResourceName({
+          const name = info.inflection.functionResourceName({
             serviceName,
             pgProc,
           });
-
-          // When overloaded functions target distinct composite types, each
-          // is allowed through the overload check in pgIntrospection_proc.
-          // However, the resource registry keys by name (see datasource.ts),
-          // so we must ensure each resource has a unique name. We suffix with
-          // the target table's schema and name, for example `code(a.pets)`
-          // becomes `code_a_pets`.
-          // GraphQL field names are unaffected since computedAttributeField
-          // inflection uses resource.extensions.pg.name (the raw SQL name).
-          {
-            const introspection = (
-              await info.helpers.pgIntrospection.getIntrospection()
-            ).find((n) => n.pgService.name === serviceName)!.introspection;
-            // Check if another proc in the same namespace shares this name
-            let hasOverload = false;
-            for (const p of introspection.procs) {
-              if (
-                p.pronamespace === pgProc.pronamespace &&
-                p.proname === pgProc.proname &&
-                p._id !== pgProc._id
-              ) {
-                hasOverload = true;
-                break;
-              }
-            }
-            if (hasOverload) {
-              // Resolve the first argument to its target table type
-              const firstArgType = pgProc.proargtypes?.[0];
-              if (firstArgType) {
-                const composite = resolveToCompositeType(
-                  firstArgType,
-                  introspection.types,
-                );
-                if (composite) {
-                  // Look up the pg_class and pg_namespace for the target
-                  // table so we can build the suffix
-                  let className: string | undefined;
-                  let schemaName: string | undefined;
-                  for (const c of introspection.classes) {
-                    if (c._id === composite.typrelid) {
-                      className = c.relname;
-                      for (const n of introspection.namespaces) {
-                        if (n._id === c.relnamespace) {
-                          schemaName = n.nspname;
-                          break;
-                        }
-                      }
-                      break;
-                    }
-                  }
-                  if (className && schemaName) {
-                    name = `${name}_${schemaName}_${className}`;
-                  }
-                }
-              }
-            }
-          }
-
           const identifier = `${serviceName}.${namespace.nspname}.${
             pgProc.proname
           }(${pgProc.getArguments().map(argTypeName).join(",")})`;
@@ -685,7 +620,10 @@ export const PgProceduresPlugin: GraphileConfig.Plugin = {
       resourceOptionsByPgProcByService: new Map(),
     }),
     hooks: {
-      async pgIntrospection_proc({ helpers, resolvedPreset }, event) {
+      async pgIntrospection_proc(
+        { helpers, resolvedPreset, inflection },
+        event,
+      ) {
         const { entity: pgProc, serviceName } = event;
 
         const pgService = resolvedPreset.pgServices?.find(
@@ -727,65 +665,26 @@ export const PgProceduresPlugin: GraphileConfig.Plugin = {
           return;
         }
 
-        // We don’t want procedures that have been defined in our namespace
-        // twice, as this leads to duplicate fields in the API. However, we
-        // allow overloads where each targets a distinct composite type as its
-        // first argument (i.e. computed columns on different tables), since
-        // each will be exposed on a different GraphQL type.
-        {
-          let hasOverload = false;
-          for (const p of introspection.procs) {
-            if (
-              p.pronamespace === pgProc.pronamespace &&
-              p.proname === pgProc.proname &&
-              p._id !== pgProc._id
-            ) {
-              hasOverload = true;
-              break;
-            }
-          }
-          if (hasOverload) {
-            // This proc has overloads — check if it qualifies as a computed
-            // column on a unique composite type.
-            const thisFirstArgType = pgProc.proargtypes?.[0];
-            if (!thisFirstArgType) {
-              // No first arg — not a computed column candidate
-              return;
-            }
-            const thisComposite = resolveToCompositeType(
-              thisFirstArgType,
-              introspection.types,
-            );
-            if (!thisComposite) {
-              // First arg doesn’t resolve to a composite type
-              return;
-            }
-            // Check that no other overload targets the same composite type;
-            // if two overloads both target, they'd produce duplicate fields on
-            // the same GraphQL type so we must skip.
-            for (const p of introspection.procs) {
-              if (
-                p.pronamespace === pgProc.pronamespace &&
-                p.proname === pgProc.proname &&
-                p._id !== pgProc._id
-              ) {
-                const otherFirstArgType = p.proargtypes?.[0];
-                if (!otherFirstArgType) continue;
-                const otherComposite = resolveToCompositeType(
-                  otherFirstArgType,
-                  introspection.types,
-                );
-                if (
-                  otherComposite &&
-                  otherComposite._id === thisComposite._id
-                ) {
-                  // Another overload targets the same composite type — skip
-                  return;
-                }
-              }
-            }
-            // This proc targets a unique composite type — allow it through
-          }
+        // We don’t want procedures whose inflected resource name clashes
+        // with another overload — this would produce duplicate fields.
+        // Overloads targeting distinct composite types get unique names
+        // from the inflector (e.g. pets_code vs buildings_code).
+        const overload = introspection.procs.find(
+          (p) =>
+            p.pronamespace === pgProc.pronamespace &&
+            p.proname === pgProc.proname &&
+            p._id !== pgProc._id &&
+            inflection.functionResourceName({
+              serviceName,
+              pgProc: p,
+            }) ===
+              inflection.functionResourceName({
+                serviceName,
+                pgProc,
+              }),
+        );
+        if (overload) {
+          return;
         }
 
         await helpers.pgProcedures.getResourceOptions(serviceName, pgProc);

postgraphile/postgraphile/__tests__/kitchen-sink-schema.sql

@@ -30,7 +30,9 @@ drop schema if exists
   issue_2334,
   relay,
   cjk,
-  scifi
+  scifi,
+  function_overloads,
+  function_overloads_other_schema
 cascade;
 drop extension if exists tablefunc;
 drop extension if exists intarray;
@@ -2574,20 +2576,38 @@ create table cjk."期间" (
 
 --------------------------------------------------------------------------------
 
--- Test overloaded computed column functions targeting different tables.
-create table a.pets (id serial primary key, name text);
-create table a.buildings (id serial primary key, address text);
-create function a.code(a.pets) returns text as $$ select 'P' || $1.id::text; $$ language sql stable;
-create function a.code(a.buildings) returns text as $$ select 'B' || $1.id::text; $$ language sql stable;
-comment on function a.code(a.pets) is E'@behavior +typeField';
-comment on function a.code(a.buildings) is E'@behavior +typeField';
-
---------------------------------------------------------------------------------
-
 -- SCIFI: Snake Case Is For Initiates.
 -- All the cool kids use UpperCamelCase
 create schema scifi;
 create table scifi."Accessory" (
   "Name" varchar(200) not null,
   "Id" integer not null primary key
 );
+
+--------------------------------------------------------------------------------
+
+-- Test overloaded computed column functions targeting different tables
+create schema function_overloads;
+create schema function_overloads_other_schema;
+create table function_overloads.pets (id serial primary key, name text);
+create table function_overloads.buildings (id serial primary key, address text);
+
+-- Two overloaded functions in the SAME schema as their target tables
+create function function_overloads.code(function_overloads.pets) returns text
+  as $$ select 'P' || $1.id::text; $$ language sql stable;
+create function function_overloads.code(function_overloads.buildings) returns text
+  as $$ select 'B' || $1.id::text; $$ language sql stable;
+comment on function function_overloads.code(function_overloads.pets)
+  is E'@behavior +typeField -queryField';
+comment on function function_overloads.code(function_overloads.buildings)
+  is E'@behavior +typeField -queryField';
+
+-- Cross-schema computed column functions (different schema from target tables)
+create function function_overloads_other_schema.age(function_overloads.pets) returns int
+  as $$ select 42; $$ language sql stable;
+create function function_overloads_other_schema.age(function_overloads.buildings) returns int
+  as $$ select 99; $$ language sql stable;
+comment on function function_overloads_other_schema.age(function_overloads.pets)
+  is E'@behavior +typeField -queryField';
+comment on function function_overloads_other_schema.age(function_overloads.buildings)
+  is E'@behavior +typeField -queryField';