NPE when accessing `this.index.table`

[stephenh]

Hi! I'm the maintainer of Joist which internally uses pg-structure to read metadata from postres, and love pg-structure--thank you!

We recently had a user report this error:

TypeError: Cannot read properties of undefined (reading 'table')
    at get referencedTable [as referencedTable] (/Users/hari/joist-sample/node_modules/pg-structure/dist/pg-structure/constraint/foreign-key.js:29:27)
    at /Users/hari/joist-sample/node_modules/pg-structure/dist/main.js:287:28
    at Array.forEach (<anonymous>)
    at addConstraints (/Users/hari/joist-sample/node_modules/pg-structure/dist/main.js:261:10)
    at addObjects (/Users/hari/joist-sample/node_modules/pg-structure/dist/main.js:337:5)
    at pgStructure (/Users/hari/joist-sample/node_modules/pg-structure/dist/main.js:378:5)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)
    at async loadSchemaMetadata (/Users/hari/joist-sample/node_modules/joist-codegen/build/index.js:83:16)
    at async main (/Users/hari/joist-sample/node_modules/joist-codegen/build/index.js:29:24)

That seems to come from this line in foreign-key.ts:

    get referencedTable() {
        // it's not possible to have an foreign key on a materialized view, so
        // `index.table` will always be defined
        return this.index.table;
    }

Which I assume is happening before of this line:

https://github.com/ozum/pg-structure/blob/master/src/main.ts#L296

Finding a row from the constraints.sql query, and using the row.indexOid to look up the index, and not finding one, but the as Index cast hides the missing-ness.

As a disclaimer, I've not seen our user's db schema to know exactly what is causing this--can you have a foreign key, with a constraint, but no index?

It seems like pg-structure is assuming no, but maybe the answer is sometimes yes...

[ozum]

Hi,

I added tests for a foreign key without an index, as shown below, and it passed without an error.

CREATE TABLE "game"
(
  "id" Serial NOT NULL,
  "publisher_id" Integer
);

ALTER TABLE "game" ADD CONSTRAINT "Key_game" PRIMARY KEY ("id");

CREATE TABLE "publisher"
(
  "id" Serial NOT NULL
);

ALTER TABLE "publisher" ADD CONSTRAINT "Key_publisher" PRIMARY KEY ("id");

ALTER TABLE "game"
  ADD CONSTRAINT "gamepublisher"
    FOREIGN KEY ("publisher_id")
    REFERENCES "publisher" ("id")
      ON DELETE RESTRICT
      ON UPDATE RESTRICT
;

[stephenh]

Hi @ozum ! Yeah, shoot :-/ it kinda sounds like this is another "pulling in some schemas but not others" issue, although:

a) I thought I'd fixed that by having Joist call pg-structure in "read all schemas" mode, and then only filter for what we want, after that. And

b) seems odd to have a foreign key in one schema, but it's index is in a different schema; like not sure if that's even possible (I'm also really guessing on the root cause here).

I copy/pasted together the pg-structure index query + constraint query with a "where index oid is null" and asked our user to run it, so we'll see if that brings back anything interesting:

https://github.com/joist-orm/joist-sample/issues/2

Thakns!

[ozum]

I'm blindly guessing, but one possible reason could be having a schema referencing an object in another non-included schema.

An example for FK:

• SchemaA is included in pg-structure
• SchemaA.TableA has an FK referencing SchemaB.TableB.
• SchemaB is not included in pg-strucutre

The example is for a FK, but it could be any object in another schema, but not found.

[stephenh]

Hi @ozum -- yeah, agreed that is probably what's happening.

What's weird is that, because we've ran into this before, with public.authors tables having triggers that live in the cyanaudit schema, we've been having pg-structure read in all the schemas, i.e. not passing a schema filter at the pg-structure level.

It turns out this is happening in a vanilla/out-of-the-box supabase account; i.e. I just made my very first trial account, setup a test db, and ran:

WITH indexes AS (
SELECT
  indexrelid AS "oid",
  ix.indrelid AS "tableOid",
  index_class.relname AS "name",
  ix.indisunique AS "isUnique",
  ix.indisprimary AS "isPrimaryKey",
  ix.indisexclusion AS "isExclusion",
  indkey::int[] AS "columnPositions",
  COALESCE(regexp_split_to_array(pg_get_expr(indexprs, ix.indrelid), ', '), '{}'::text[]) AS "indexExpressions",
  pg_get_expr(indpred, ix.indrelid) AS "partialIndexExpression",
  pg_catalog.obj_description(indexrelid, 'pg_index') AS "comment"
FROM
  pg_index ix
  INNER JOIN pg_class index_class ON index_class.oid = ix.indexrelid --AND index_class.relkind = 'i') -- Indexes only from pg_class, i: indexes
ORDER BY
  ix.indrelid,
  LOWER(index_class.relname)
)
SELECT
  c.conrelid AS "tableOid",
  c.conindid AS "indexOid",
  c.contypid AS "typeOid",
  c.contype AS "kind", -- c: check, f: foreign key, p: primary key, u: unique, t: constraint trigger, x: exclusion constraint
  c.conname AS "name",
  c.conkey::int[] AS "constrainedColumnPositions",
  c.confkey::int[] AS "referencedColumnPositions",
  c.confupdtype AS "onUpdate",
  c.confdeltype AS "onDelete",
  c.confmatchtype AS "matchType",
  c.condeferrable AS "isDeferrable",
  c.condeferred AS "isDeferred",
  pg_get_constraintdef(c.oid) AS "checkConstraintExpression",
  pg_catalog.obj_description(c.oid, 'pg_constraint') AS "comment"
FROM
  pg_constraint c
  LEFT JOIN indexes i ON c.conindid = i.oid
  WHERE i.oid IS NULL
ORDER BY
  c.connamespace,
  LOWER(c.conname)

By pasting together the two index + constraint queries, and got back these rows (just as our Joist user did):

| tableOid | indexOid | typeOid | kind | name                                    | constrainedColumnPositions | referencedColumnPositions | onUpdate | onDelete | matchType | isDeferrable | isDeferred | checkConstraintExpression                                                                                                                                                                                              | comment |
| -------- | -------- | ------- | ---- | --------------------------------------- | -------------------------- | ------------------------- | -------- | -------- | --------- | ------------ | ---------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------- |
| 0        | 0        | 13169   | c    | cardinal_number_domain_check            |                            |                           |          |          |           | false        | false      | CHECK ((VALUE >= 0))                                                                                                                                                                                                   |         |
| 0        | 0        | 13182   | c    | yes_or_no_check                         |                            |                           |          |          |           | false        | false      | CHECK (((VALUE)::text = ANY ((ARRAY['YES'::character varying, 'NO'::character varying])::text[])))                                                                                                                     |         |
| 28729    | 0        | 0       | c    | domain not empty                        | 3                          |                           |          |          |           | false        | false      | CHECK ((char_length(domain) > 0))                                                                                                                                                                                      |         |
| 28744    | 0        | 0       | c    | entity_id not empty                     | 3                          |                           |          |          |           | false        | false      | CHECK ((char_length(entity_id) > 0))                                                                                                                                                                                   |         |
| 28744    | 0        | 0       | c    | metadata_url not empty                  | 5                          |                           |          |          |           | false        | false      | CHECK (((metadata_url = NULL::text) OR (char_length(metadata_url) > 0)))                                                                                                                                               |         |
| 28744    | 0        | 0       | c    | metadata_xml not empty                  | 4                          |                           |          |          |           | false        | false      | CHECK ((char_length(metadata_xml) > 0))                                                                                                                                                                                |         |
| 28865    | 0        | 0       | c    | one_time_tokens_token_hash_check        | 4                          |                           |          |          |           | false        | false      | CHECK ((char_length(token_hash) > 0))                                                                                                                                                                                  |         |
| 28762    | 0        | 0       | c    | request_id not empty                    | 3                          |                           |          |          |           | false        | false      | CHECK ((char_length(request_id) > 0))                                                                                                                                                                                  |         |
| 28720    | 0        | 0       | c    | resource_id not empty                   | 2                          |                           |          |          |           | false        | false      | CHECK (((resource_id = NULL::text) OR (char_length(resource_id) > 0)))                                                                                                                                                 |         |
| 16489    | 0        | 0       | c    | users_email_change_confirm_status_check | 29                         |                           |          |          |           | false        | false      | CHECK (((email_change_confirm_status >= 0) AND (email_change_confirm_status <= 2)))                                                                                                                                    |         |
| 16790    | 0        | 0       | c    | key_key_context_check                   | 7                          |                           |          |          |           | false        | false      | CHECK ((length(key_context) = 8))                                                                                                                                                                                      |         |
| 16790    | 0        | 0       | c    | pgsodium_raw                            | 10,6,7,12                  |                           |          |          |           | false        | false      | CHECK (
CASE
    WHEN (raw_key IS NOT NULL) THEN ((key_id IS NULL) AND (key_context IS NULL) AND (parent_key IS NOT NULL))
    ELSE ((key_id IS NOT NULL) AND (key_context IS NOT NULL) AND (parent_key IS NULL))
END) |         |

These rows seem to be FK constraints within supabase's "out-of-the-box features", i.e. even in a vanilla project, they have an auth / storage / vault / etc type schemas that they've added to enable their various infra.

So, pg-structure is pulling those in (which is fine, we've told it too), but somehow those constraints still can't find their index.

I don't want to ask you to debug supabase's bespoke/out-of-the-box schemas--maybe it has to do with permissions, like maybe they've made the FKs visible (to user metadata queries), but not the underlying indexes?

Would you be open to a PR that skips FKs that can't find their index?

Or should we adjust our setup to "ignore supabase schemas" (b/c they're weird somehow) but "still pull in other non-public schemas" so we don't get tripped up by the cyanaudit triggers?

Thanks!

[ozum]

Skipping FKs would cause many unforeseen problems and unpredictable results because the whole pg-structure system depends on the existence of other objects. It would behave like a broken database.

Did you try using a superuser like postgres, so you can eliminate at least the privilege possibility?

[stephenh]

Ah man; I found it...the issue was:

• There was a vault.secrets table had a FOREIGN KEY (key_id) REFERENCES pgsodium.key(id)
• But none of the pgsodium schema's indexes were being returned
• Turns out the getSchemas query was:

SELECT oid, nspname AS name, obj_description(oid, 'pg_namespace') AS comment
FROM pg_namespace WHERE NOT pg_is_other_temp_schema(oid) AND nspname <> 'pg_toast'
AND nspname NOT LIKE 'information_schema'
AND nspname NOT LIKE 'pg_%' ORDER BY nspname`;

Note that NOT LIKE pg_%, that was actually excluding pgsodium as well.

If I change it to pg\_% then pgsodium is returned, and the query completes successfully.

Honestly I had no idea that _ was a single-character wildcard.

[ozum]

Oh, thank you, it was a really hard-to-find bug.

Honestly I had no idea that _ was a single-character wildcard.

I haven't used _ as a wildcard for years, and I, too, forgot it was a wildcard character and put it there as a regular character.

I'm going to merge the PR and publish it after checking the tests.

[stephenh]

Np!

The most "annoying" part was just setting up a supabase account, to debug what was actually happening, and even that was actually pretty easy.

Who knows, maybe I'll end up using supabase for something now. :-)

Thanks for taking a look at the PR!

[github-actions[bot]]

:tada: This issue has been resolved in version 7.15.3 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[ozum]

Who knows, maybe I'll end up using supabase for something now. :-)

Supabase is one of the most amazing tools I've seen. Fully developer-oriented, not bloated, and does not reinvent unnecessary wheels :). Also, they are making a good part of contribution to the open source community.

I didn't complete a production project with it but keep my eyes on it.