Failing to apply snapshot schema when altering field sort order

[smonn]

Checklist

• I'm using the latest version of Directus
• There's no other issue that already describes the problem.
• I've completed all Troubleshooting Steps.

Describe the Bug

Getting this exception (articles is a data model):

error: alter table "articles" alter column "id" drop not null - column "id" is in a primary key
    at Parser.parseErrorMessage (/.../node_modules/pg-protocol/dist/parser.js:287:98)
    at Parser.handlePacket (/.../node_modules/pg-protocol/dist/parser.js:126:29)
    at Parser.parse (/.../node_modules/pg-protocol/dist/parser.js:39:38)
    at Socket.<anonymous> (/.../node_modules/pg-protocol/dist/index.js:11:42)
    at Socket.emit (node:events:513:28)
    at addChunk (node:internal/streams/readable:324:12)
    at readableAddChunk (node:internal/streams/readable:297:9)
    at Readable.push (node:internal/streams/readable:234:10)
    at TCP.onStreamRead (node:internal/stream_base_commons:190:23)

It happens after only altering the visual order of a field, i.e. when using drag-and-drop and then updating the schema snapshot file to be shared with other developers. I'd expect that just altering sort would not cause a database schema change since it's just a value in the directus_fields table.

I'm not 100% certain no other issues also encounter this, but similar stack traces refer to other possible causes.

To Reproduce

I've setup a repository to reproduce the issue. You can find it here https://github.com/smonn/repro-directus-snapshot-issue

Hosting Strategy

Self-Hosted (Custom)

[LiamLombard]

Not a solution but a report of a similar error in hopes it helps narrow down debugging efforts:

I saw this error yesterday also, but under slightly different circumstances. I unfortunately haven't had time to work out the minimal reproduction steps.

My usecase for directus might be slightly abnormal, as I manage my postgres data schema with a separate db migration tool so I can keep my schema in code/version control. I then rely on directus introspection and some manual tinkering of field config to make the UI user friendly.

I was trying to replicate my prod instance locally, with steps:

1. bring up fresh docker postgres and directus
2. apply migrations to postgres to match prod db
3. apply snapshot schema to local directus
4. I see a similar error as above, where the snapshot mechanism appears to be trying to remove a "not null" clause from an primary key id column in one of my tables, despite checking in the schema I am trying to apply that the id field in directus is known to be a primary key (is_primary_key: true in the yaml), and also is known to not be nullable (is_nullable: false in the yaml)

Due to time constraints I had to revert back to 9.22.4, where things seem to work okay.

I tried dumping the snapshot schema from both directus 9.22.4 and 9.23.1, with neither being applied successfully by 9.23.1.

[paescuj]

/linear

[github-actions]

🤖 Linear issue created! Maintainers can access it here: ENG-827

[paescuj]

Related issue #17832

[claytongulick]

So, I've been digging into this a bit, it doesn't seem limited to just sort order. The issue looks like it comes up whenever a primary key has any sort of metadata change, for example - change from hidden to visible.

What's happening is that in fieldsService.updateField a schema comparison is being made here

This comparison always fails because the object returned from the schemaInspector and the hookAdjustedField.schema is different - not meaningfully, but there are extra fields that are set to null, so there's a false detection of schema change.

Next, because this fails, the knex alter() call is issued to attempt to modify the schema to match. Well, the alter() call definitely shouldn't be invoked on a metadata change, but what's more, alterNullable defaults to true. When the SQL gets generated this completely blows up on a primary key field, because you can't make that nullable, hence the "alter column "id" drop not null - column "id" is in a primary key" error you're seeing.

@rijkvanzanten hope this helps debug and resolve the issue.

[paescuj]

@claytongulick Thank you very much! That's certainly going to be helpful! We're about to investigate this one as well 👍

(May I kindly ask you not to randomly ping team members?)

[claytongulick]

(May I kindly ask you not to randomly ping team members?)

Sure thing, sorry - have had lots of discussions with Rijk and don't know how y'all get notified internally - this one was a couple weeks old, so wanted to mention that I'd dug into it a bit to hopefully save someone some time.

Won't happen again 😄

[paescuj]

(May I kindly ask you not to randomly ping team members?)

Sure thing, sorry - have had lots of discussions with Rijk and don't know how y'all get notified internally - this one was a couple weeks old, so wanted to mention that I'd dug into it a bit to hopefully save someone some time.

Won't happen again 😄

No problemo, just wanted to have it mentioned & thanks again ❤️ In general it should be enough to just post in the threads, in most cases the right person will be notified, otherwise we'll forward it internally 👍