Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Schema apply with char 36 becomes varchar 36 #20428

Open
u12206050 opened this issue Nov 15, 2023 · 4 comments
Open

Schema apply with char 36 becomes varchar 36 #20428

u12206050 opened this issue Nov 15, 2023 · 4 comments
Labels
Bug Core Data Modeling

Comments

@u12206050
Copy link
Contributor

u12206050 commented Nov 15, 2023 •
edited
Loading

Describe the Bug

So we have noticed some inconsistencies between different environments when importing the schema, causing havoc when ever we try to sync the schema.

We start of with a schema containing a foreign key that has the following schema:

{
        "collection": "registrations",
        "field": "event",
        "type": "string",
        "meta": {
          "collection": "registrations",
          "conditions": null,
          "display": "related-values",
          "display_options": {
            "template": "{{event_translations}}"
          },
          "field": "event",
          "group": null,
          "hidden": false,
          "interface": "select-dropdown-m2o",
          "note": null,
          "options": {},
          "readonly": false,
          "required": true,
          "sort": 2,
          "special": ["m2o"],
          "translations": null,
          "validation": null,
          "validation_message": null,
          "width": "half"
        },
        "schema": {
          "name": "event",
          "table": "registrations",
          "data_type": "char",
          "default_value": null,
          "max_length": 36,
          "numeric_precision": null,
          "numeric_scale": null,
          "is_nullable": false,
          "is_unique": false,
          "is_primary_key": false,
          "is_generated": false,
          "generation_expression": null,
          "has_auto_increment": false,
          "foreign_key_table": null,
          "foreign_key_column": null
        }
      },

Specifically this part is important:

 "data_type": "char",
 "default_value": null,
 "max_length": 36,
…
 "is_nullable": false,

When we apply this schema to a fresh database right after bootstrapping Directus, the column ends up being varchar(36) leading to the next time we export the database that the schema gets updated to varchar(36) which on another environment seems to break.

During investigation I found that some foreign keys are added as char(36) and upon looking at what differs, the only property that was different was is_nullable: true. This might be the reason but further tests are needed.

After testing more and changing the is_nullable property, it did not seem to affect the final state. The same foreign keys would still map to either varchar(36) or char(36)

To Reproduce

Add/edit two field such that it matches the above type and max length, BUT set one of them to is_nullable: false and the other to true

Apply the schema on a fresh database after Directus bootstrap ensure the columns is created with the correct type char(36) and nullable setting.

Note the nullable issue itself is tracked here: #20418

Directus Version

10.6-10.7

Hosting Strategy

Self-Hosted (Docker Image)
@u12206050
Copy link
Contributor Author

This is still a huge issue, every new env is setting up the schema with varchar, even though the data_type of the field is set to char

@u12206050
Copy link
Contributor Author

Found the source of the issue in apply-diff.ts

.map((fieldDiff) => {
   // Casts field type to UUID when applying non-PostgreSQL schema onto PostgreSQL database.
   // This is needed because they snapshots UUID fields as char/varchar with length 36.
   if (
   	['char', 'varchar'].includes(String(fieldDiff.schema?.data_type).toLowerCase()) &&
   	fieldDiff.schema?.max_length === 36 &&
   	(fieldDiff.schema?.is_primary_key ||
   		(fieldDiff.schema?.foreign_key_table && fieldDiff.schema?.foreign_key_column))
   ) {
   	return merge(fieldDiff, { type: 'uuid', schema: { data_type: 'uuid', max_length: null } });
   } else {
   	return fieldDiff;
   }
});

Removing this fixes the issue. Leave our char and varchar as is :)

@u12206050 u12206050 mentioned this issue Jan 24, 2024
Closed
@u12206050
Copy link
Contributor Author

Turns out, removing the block only converted all the should be uuid fields to varchar making them also msmatch their char id fields. So to fix this I created a PR that results in a better matching schema file to what will be applied to the db.

@u12206050
Copy link
Contributor Author

Arg! What a mess, it works now on initial db schema create, but only if I remove the code block mentioned above, and event still when you take a snapshot of the newly created db, then the snapshot is again different than it was used to create it, which it should be. ZXC -> In should result in ZXC -> OUT. But seems like certain rules are either not being exported such as CONSTRAINTS ON UPDATE.

@hanneskuettner hanneskuettner self-assigned this Apr 23, 2024
@hanneskuettner hanneskuettner removed their assignment Aug 5, 2024
@rijkvanzanten rijkvanzanten added the Engine label Aug 28, 2024 — with Linear
@rijkvanzanten rijkvanzanten added the Bug label Sep 2, 2024 — with Linear
@rijkvanzanten rijkvanzanten assigned br41nslug and unassigned br41nslug Sep 4, 2024
@rijkvanzanten rijkvanzanten reopened this Sep 6, 2024
@rijkvanzanten rijkvanzanten added the Directus label Nov 25, 2024 — with Linear
@rijkvanzanten rijkvanzanten added the Core label Nov 25, 2024 — with Linear
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug Core Data Modeling
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Set fields as type UUID on export u12206050/directus
4 participants
@hanneskuettner @u12206050 @rijkvanzanten @br41nslug