Address single table polymorphism inconsistencies (#285)

.changeset/nervous-seas-vanish.md

@@ -0,0 +1,9 @@
+---
+"graphile-build-pg": patch
+"postgraphile": patch
+"@dataplan/pg": patch
+---
+
+Single table inheritance no longer exposes non-shared columns via
+condition/order, and also only exposes the relationships on the types where they
+are appropriate.

grafast/dataplan-pg/src/interfaces.ts

@@ -79,17 +79,22 @@ export interface PgCodecPolymorphismSingleTypeAttributeSpec<
 export interface PgCodecPolymorphismSingleTypeSpec<
   TAttributeName extends string = string,
 > {
+  /** The name of the polymorphic subentry of the parent single table polymorphic codec */
   name: string;
   // TODO: make this optional?
+  /** The attributes that are specific to this concrete type (including any modifiers) */
   attributes: Array<PgCodecPolymorphismSingleTypeAttributeSpec<TAttributeName>>;
 }
 export interface PgCodecPolymorphismSingle<
   TAttributeName extends string = string,
 > {
   mode: "single";
+  /** The list of attributes that is used to determine which polymorphic type the record is. Currently this should always have length 1. */
   typeAttributes: readonly TAttributeName[];
   // TODO: make this optional?
+  /** These attributes are shared by every concrete type of this codec */
   commonAttributes: readonly TAttributeName[];
+  /** Details the concrete types from this polymorphic single table, including what to call it, and what columns it has. */
   types: {
     [typeKey: string]: PgCodecPolymorphismSingleTypeSpec<TAttributeName>;
   };
@@ -480,8 +485,10 @@ export interface PgCodecRelationBase<
   TLocalCodec extends PgCodec = PgCodec,
   TRemoteAttributes extends string = string,
 > {
-  /* Where the relationship starts */
+  /** Where the relationship starts */
   localCodec: TLocalCodec;
+  /** If localCodec is polymorphic, which of the concrete subtypes should this relationship apply to? */
+  localCodecPolymorphicTypes?: string[];
 
   /**
    * The attributes locally used in this relationship.

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

@@ -77,7 +77,33 @@ export const PgConditionArgumentPlugin: GraphileConfig.Plugin = {
                 ),
                 fields: (context) => {
                   const { fieldWithHooks } = context;
-                  const attributes = codec.attributes;
+                  const allAttributes = codec.attributes;
+                  const allowedAttributes =
+                    codec.polymorphism?.mode === "single"
+                      ? [
+                          ...codec.polymorphism.commonAttributes,
+                          // TODO: add condition input type for the underlying concrete types, which should also include something like:
+                          /*
+                          ...(pgPolymorphicSingleTableType
+                            ? codec.polymorphism.types[
+                                pgPolymorphicSingleTableType.typeIdentifier
+                              ].attributes.map(
+                                (attr) =>
+                                  // FIXME: we should be factoring in the attr.rename
+                                  attr.attribute,
+                              )
+                            : []),
+                          */
+                        ]
+                      : null;
+                  const attributes = allowedAttributes
+                    ? Object.fromEntries(
+                        Object.entries(allAttributes).filter(
+                          ([attrName, _attr]) =>
+                            allowedAttributes.includes(attrName),
+                        ),
+                      )
+                    : allAttributes;
                   // TODO: move this to a separate plugin
                   return Object.entries(attributes).reduce(
                     (memo, [attributeName, attribute]) => {

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

@@ -92,6 +92,9 @@ export const PgConnectionArgOrderByPlugin: GraphileConfig.Plugin = {
               },
             )}`,*/
           );
+          if (codec.polymorphism?.mode === "single") {
+            // TODO: register OrderBy for each concrete type
+          }
         });
         return _;
       },

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

@@ -49,7 +49,11 @@ export const PgOrderAllAttributesPlugin: GraphileConfig.Plugin = {
       GraphQLEnumType_values(values, build, context) {
         const { extend, inflection, options } = build;
         const {
-          scope: { isPgRowSortEnum, pgCodec: rawPgCodec },
+          scope: {
+            isPgRowSortEnum,
+            pgCodec: rawPgCodec,
+            pgPolymorphicSingleTableType,
+          },
         } = context;
         const { pgOrderByNullsLast } = options;
         if (
@@ -61,7 +65,29 @@ export const PgOrderAllAttributesPlugin: GraphileConfig.Plugin = {
           return values;
         }
         const pgCodec = rawPgCodec as PgCodecWithAttributes;
-        const attributes = pgCodec.attributes;
+        const allAttributes = pgCodec.attributes;
+        const allowedAttributes =
+          pgCodec.polymorphism?.mode === "single"
+            ? [
+                ...pgCodec.polymorphism.commonAttributes,
+                ...(pgPolymorphicSingleTableType
+                  ? pgCodec.polymorphism.types[
+                      pgPolymorphicSingleTableType.typeIdentifier
+                    ].attributes.map(
+                      (attr) =>
+                        // FIXME: we should be factoring in the attr.rename
+                        attr.attribute,
+                    )
+                  : []),
+              ]
+            : null;
+        const attributes = allowedAttributes
+          ? Object.fromEntries(
+              Object.entries(allAttributes).filter(([attrName, _attr]) =>
+                allowedAttributes.includes(attrName),
+              ),
+            )
+          : allAttributes;
         const sources = Object.values(
           build.input.pgRegistry.pgResources,
         ).filter((s) => s.codec === pgCodec && !s.parameters);

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

@@ -63,6 +63,13 @@ declare global {
         name: string;
       };
     }
+    interface ScopeEnum {
+      pgPolymorphicSingleTableType?: {
+        typeIdentifier: string;
+        name: string;
+        attributes: ReadonlyArray<PgCodecPolymorphismSingleTypeAttributeSpec>;
+      };
+    }
   }
 }
 

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

@@ -343,10 +343,49 @@ export const PgRelationsPlugin: GraphileConfig.Plugin = {
         });
         const registryBuilder =
           await info.helpers.pgRegistry.getRegistryBuilder();
+        const { codec } = event.resourceOptions;
+        let localCodecPolymorphicTypes: string[] | undefined = undefined;
+        if (codec.polymorphism?.mode === "single") {
+          const poly = codec.polymorphism;
+          if (
+            localAttributes.every((attr) =>
+              poly.commonAttributes.includes(attr!.attname),
+            )
+          ) {
+            // Common to all types
+          } else {
+            if (isReferencee) {
+              // TODO: consider supporting backward relationships for single
+              // table polymorphic types. It's not immediately clear what the
+              // user would want in these cases: is it separate fields for each
+              // type (that would inflate the schema), or is it a relation to
+              // the underlying polymorphic type even though we know certain
+              // concrete types from it will never appear? For now we're
+              // skipping it entirely because then we can add whatever makes
+              // sense later.
+              return;
+            }
+            localCodecPolymorphicTypes = [];
+            for (const [typeKey, typeDetails] of Object.entries(poly.types)) {
+              if (
+                localAttributes.every(
+                  (attr) =>
+                    poly.commonAttributes.includes(attr!.attname) ||
+                    typeDetails.attributes.some(
+                      (a) => a.attribute === attr!.attname,
+                    ),
+                )
+              ) {
+                // MATCH!
+                localCodecPolymorphicTypes.push(typeKey);
+              }
+            }
+          }
+        }
         const existingRelation =
-          registryBuilder.getRegistryConfig().pgRelations[
-            event.resourceOptions.codec.name
-          ]?.[relationName];
+          registryBuilder.getRegistryConfig().pgRelations[codec.name]?.[
+            relationName
+          ];
         const { tags: rawTags, description: constraintDescription } =
           pgConstraint.getTagsAndDescription();
         // Clone the tags because we use the same tags on both relations
@@ -358,6 +397,7 @@ export const PgRelationsPlugin: GraphileConfig.Plugin = {
           : tags.forwardDescription ?? constraintDescription;
         const newRelation: PgCodecRelationConfig = {
           localCodec: localCodec as PgCodecWithAttributes,
+          localCodecPolymorphicTypes,
           localAttributes: localAttributes.map((c) => c!.attname),
           remoteAttributes: foreignAttributes.map((c) => c!.attname),
           remoteResourceOptions: foreignResourceOptions,
@@ -406,7 +446,7 @@ export const PgRelationsPlugin: GraphileConfig.Plugin = {
           }
         }
         registryBuilder.addRelation(
-          event.resourceOptions.codec as PgCodecWithAttributes,
+          codec as PgCodecWithAttributes,
           relationName,
           newRelation.remoteResourceOptions,
           newRelation,
@@ -622,6 +662,10 @@ function addRelations(
     "isMutationPayload" in scope ? scope.isMutationPayload : undefined;
   const pgPolymorphism =
     "pgPolymorphism" in scope ? scope.pgPolymorphism : undefined;
+  const pgPolymorphicSingleTableType =
+    "pgPolymorphicSingleTableType" in scope
+      ? scope.pgPolymorphicSingleTableType
+      : undefined;
 
   const codec = (pgTypeResource?.codec ?? pgCodec) as PgCodec;
   // TODO: make it so isMutationPayload doesn't trigger this by default (only in V4 compatibility mode)
@@ -751,6 +795,7 @@ function addRelations(
     // Digest relations
     for (const [relationName, relation] of Object.entries(relations)) {
       const {
+        localCodecPolymorphicTypes,
         localAttributes,
         remoteAttributes,
         remoteResource,
@@ -761,6 +806,28 @@ function addRelations(
         // Don't add backwards relations to mutation payloads
         continue;
       }
+
+      if (localCodecPolymorphicTypes) {
+        if (!pgPolymorphicSingleTableType) {
+          if (context.type === "GraphQLInterfaceType") {
+            // Ignore on interface
+            continue;
+          } else {
+            throw new Error(
+              `Relationship indicates it only relates to certain polymorphic subtypes, but this type doesn't seem to be polymorphic?`,
+            );
+          }
+        }
+        if (
+          !localCodecPolymorphicTypes.some(
+            (t) => t === pgPolymorphicSingleTableType.typeIdentifier,
+          )
+        ) {
+          // Does not apply to this polymorphic subtype; skip.
+          continue;
+        }
+      }
+
       // The behavior is the relation behavior PLUS the remote table
       // behavior. But the relation settings win.
       const behavior =