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

Support for drop default on a column #450

Closed
divyenduz opened this issue Nov 6, 2024 · 6 comments · Fixed by #478
Closed

Support for drop default on a column #450

divyenduz opened this issue Nov 6, 2024 · 6 comments · Fixed by #478
Assignees

Comments

@divyenduz
Copy link

divyenduz commented Nov 6, 2024

The following operation is not supported natively by pgroll.

ALTER TABLE public."Pet" ALTER COLUMN id DROP DEFAULT;

Current workaround is to use SQL the escape hatch.

@ryanslade ryanslade self-assigned this Nov 20, 2024
@ryanslade
Copy link
Contributor

This feels like something we want to add to OpAlterColumn right?

We already allow setting the value of Default and so I'm wondering whether just handling the case where this is set to null is enough since this is what is implicitly happening when you drop the default value anyway:

This is effectively the same as setting the default to null. As a consequence, it is not an error to drop a default where one hadn't been defined, because the default is implicitly the null value.

From https://www.postgresql.org/docs/current/ddl-alter.html#DDL-ALTER-COLUMN-DEFAULT

@andrew-farries
Copy link
Collaborator

Yes, this belongs on OpAlterColumn.

I think we could handle this by distinguishing the case where default is explicitly set to null vs omitted from the operation in the same way as we did in #345 for removing comments from tables and columns.

@ryanslade
Copy link
Contributor

So I realised while implementing this that we already support it. If we set the default value of a column to the string NULL it will drop the default. We already have a test that shows this:

name: "set column default: remove the default",

Is this enough? Maybe we should just update the docs and add an example?

@andrew-farries
Copy link
Collaborator

The following statements look to have slightly different effects:

alter table products alter column name set default NULL

results in a column that looks like this:

 name   | character varying(255) |           |          | NULL::character varying                          | extended |             |              |

whereas:

alter table products alter column name drop default

produces a column like this:

 name   | character varying(255) |           |          |                                                  | extended |             |              |

i.e Postgres catalogs record a default of NULL vs no default slightly differently. If we wanted to implement this we'd need to distinguish:

  "operations": [
    {
      "alter_column": {
        "table": "products",
        "column": "name",
        "default": "NULL"
      }
    }
  ]

from:

  "operations": [
    {
      "alter_column": {
        "table": "products",
        "column": "name",
        "default": null
      }
    }
  ]

WDYT?

@ryanslade
Copy link
Contributor

Ok, so we would implement the second case where the value is null by dropping the default. I'm still not totally clear on the semantic difference between no default and a default of NULL.

@andrew-farries
Copy link
Collaborator

andrew-farries commented Nov 21, 2024

I don't think there is a difference in practical terms - looks as though Postgres just records each case differently in the system catalogs:

Using this query to retrieve information about column defaults:

SELECT
     a.attname AS column_name,
     pg_get_expr(ad.adbin, ad.adrelid) AS default_value
 FROM
     pg_catalog.pg_attrdef ad
     JOIN pg_catalog.pg_attribute a ON ad.adnum = a.attnum
 WHERE
     a.attrelid = 'products'::regclass;

a DEFAULT set to NULL still shows up in the result set:

+-------------+--------------------------------------------------+
| column_name | default_value                                    |
|-------------+--------------------------------------------------|
| name        | NULL::character varying                          |
+-------------+--------------------------------------------------+

whereas a dropped DEFAULT is no longer present in the catalogs as having a default (no entry for the column in pg_attrdef and pg_attribute.atthasdef is false for that column).

This is on Postgres 17; not sure if the behaviour is consistent between versions.

It looks as though dropping a DEFAULT is 'cleaner' than setting it to NULL if nothing else.

ryanslade added a commit that referenced this issue Nov 21, 2024
We no allow setting the value of `default` to `null` when altering a
column. In practice this will cause us to drop the default. It is also
possible to set `default` the *value* `NULL` which has a similar effect
but is recorded slightly differently in the PG catalogue. (Details
[here](#450 (comment)))

Closes #450
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants