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.

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

Use actual enum in Postgres Plateform instead of varchar with checks #886

Open
1 task done
Kyrela opened this issue Jun 20, 2024 · 0 comments
Open
1 task done

Use actual enum in Postgres Plateform instead of varchar with checks #886

Kyrela opened this issue Jun 20, 2024 · 0 comments
Labels
enhancement A feature that exists, works as intended but needs to be improved feature request A feature that does not yet exist but will be a good addition to the library

Comments

@Kyrela
Copy link
Contributor

Kyrela commented Jun 20, 2024

Describe the feature as you'd like to see it

Actually, when creating an enum column in a table using a postres connector, a varchar with checks is created. For example:

with self.schema.create("users") as blueprint:
    blueprint.enum("status", ["active", "inactive"]).default("active")

returns

CREATE TABLE "users" ("status" VARCHAR(255) CHECK(status IN ('active', 'inactive')) NOT NULL DEFAULT 'active')

Postgres doesn't have an ENUM type, but there is way to create ENUM types as can been seen here

CREATE TYPE mood AS ENUM ('sad', 'ok', 'happy');

Manipulating these enum require their own separate ddl query, and, as the PostgresPlatform is only responsible for sql translation and doesn't have any way to look over the actual table, it requires (without significal changes):

  • A "normized" name, such as {table}_{column}_enum that allows the enums to be retrived without being kept in memory
  • Tracking of these enums in order to, for example, delete the enum when the associated colmun or table is dropped. This require things like renaming the enum if the column is renamed.

Keep in mind that this solution is a proposal, and can of couse be implemented in other ways, but in any case, it's a complex addition that requires a good understanding of both masonite-orm and PostgreSQL.

What do we currently have to do now?

Varchar with checks.

Additional context

Here's a quick idea of what the solution I proposed could look like. I don't have neither the time or the experience to work on a clean PR, so I'm leaving this here:

Index: src/masoniteorm/schema/platforms/PostgresPlatform.py
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/masoniteorm/schema/platforms/PostgresPlatform.py b/src/masoniteorm/schema/platforms/PostgresPlatform.py
--- a/src/masoniteorm/schema/platforms/PostgresPlatform.py	(revision ec31a17aea4ac08cc1cb7a23f05aaab24e35b27b)
+++ b/src/masoniteorm/schema/platforms/PostgresPlatform.py	(date 1718889033107)
@@ -143,20 +143,20 @@
                 default = ""
 
             constraint = ""
-            column_constraint = ""
             if column.primary:
                 constraint = "PRIMARY KEY"
 
             if column.column_type == "enum":
                 values = ", ".join(f"'{x}'" for x in column.values)
-                column_constraint = f" CHECK({column.name} IN ({values}))"
+                sql.append(
+                    f"CREATE TYPE {self.wrap_column(name + '_enum')} AS ENUM ({values})"
+                )
 
             sql.append(
                 self.columnize_string()
                 .format(
                     name=self.wrap_column(column.name),
-                    data_type=self.type_map.get(column.column_type, ""),
-                    column_constraint=column_constraint,
+                    data_type=self.type_map.get(column.column_type, "") if column.column_type != "enum" else f"{name}_enum",
                     length=length,
                     constraint=constraint,
                     nullable=self.premapped_nulls.get(column.is_null) or "",
@@ -194,11 +194,17 @@
                 else:
                     default = ""
 
+                if column.column_type == "enum":
+                    values = ", ".join(f"'{x}'" for x in column.values)
+                    sql.append(
+                        f"CREATE TYPE {self.wrap_column(name + '_enum')} AS ENUM ({values})"
+                    )
+
                 add_columns.append(
                     self.add_column_string()
                     .format(
                         name=self.wrap_column(column.name),
-                        data_type=self.type_map.get(column.column_type, ""),
+                        data_type=self.type_map.get(column.column_type, "") if column.column_type != "enum" else f"{name}_enum",
                         length=length,
                         constraint="PRIMARY KEY" if column.primary else "",
                         nullable="NULL" if column.is_null else "NOT NULL",
@@ -221,6 +227,9 @@
             renamed_sql = []
 
             for name, column in table.get_renamed_columns().items():
+                # TODO: rename the enum type if exists to ensure correct tracking of the enum in the future
+                #  see https://stackoverflow.com/questions/7624919/check-if-a-user-defined-type-already-exists-in-postgresql
+
                 if column.length:
                     length = self.create_column_length(column.column_type).format(
                         length=column.length
@@ -247,6 +256,9 @@
             dropped_sql = []
 
             for name in table.get_dropped_columns():
+                sql.append(
+                    f"DROP TYPE IF EXISTS {self.wrap_column(name + '_enum')}")
+
                 dropped_sql.append(
                     self.drop_column_string()
                     .format(name=self.wrap_column(name))
@@ -263,11 +275,20 @@
             changed_sql = []
 
             for name, column in table.changed_columns.items():
+                sql.append(
+                    f"DROP TYPE IF EXISTS {self.wrap_column(name + '_enum')}")
+
+                if column.column_type == "enum":
+                    values = ", ".join(f"'{x}'" for x in column.values)
+                    sql.append(
+                        f"CREATE TYPE {self.wrap_column(name + '_enum')} AS ENUM ({values})"
+                    )
+
                 changed_sql.append(
                     self.modify_column_string()
                     .format(
                         name=self.wrap_column(name),
-                        data_type=self.type_map.get(column.column_type),
+                        data_type=self.type_map.get(column.column_type) if column.column_type != "enum" else f"{name}_enum",
                         nullable="NULL" if column.is_null else "NOT NULL",
                         length="(" + str(column.length) + ")"
                         if column.column_type not in self.types_without_lengths
@@ -392,7 +413,7 @@
         return "RENAME COLUMN {old} TO {to}"
 
     def columnize_string(self):
-        return "{name} {data_type}{length}{column_constraint} {nullable}{default} {constraint}"
+        return "{name} {data_type}{length} {nullable}{default} {constraint}"
 
     def constraintize(self, constraints, table):
         sql = []
@@ -452,9 +473,11 @@
         return f"ALTER TABLE {self.wrap_table(current_name)} RENAME TO {self.wrap_table(new_name)}"
 
     def compile_drop_table_if_exists(self, table):
+        # TODO: for each column (or by wildcard), drop the enum type if exists
         return f"DROP TABLE IF EXISTS {self.wrap_table(table)}"
 
     def compile_drop_table(self, table):
+        # TODO: for each column (or by wildcard), drop the enum type if exists
         return f"DROP TABLE {self.wrap_table(table)}"
 
     def compile_column_exists(self, table, column):

Please note that i'm only using the colmun name to generate the enum names in this example, because the columnize method doesn't have a reference to the table (so it'll probably need to be changed too as it could lead to duplicate names).

Also, here's a link about a discussion on this issue that occured in the Discord Server.

  • Is this a breaking change? (some people may relay on the actual behavior)
@Kyrela Kyrela added enhancement A feature that exists, works as intended but needs to be improved feature request A feature that does not yet exist but will be a good addition to the library labels Jun 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement A feature that exists, works as intended but needs to be improved feature request A feature that does not yet exist but will be a good addition to the library
Projects
None yet
Development

No branches or pull requests

1 participant