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

Add constraints option to create_table and unique constraint support #585

Merged
merged 14 commits into from
Jan 13, 2025

Conversation

kvch
Copy link
Contributor

@kvch kvch commented Jan 10, 2025

This PR adds support for a new option of create_table operation named constraints.
It expects a list of constraints that is defined on the table when the table is created.

At the moment the only constraint we support is unique. But it includes support for all options
of unique constraints including NULLS NOT DISTINCT, index configuration settings, constraint deference, etc. Once I am done with these table constraints, I will open a follow-up PR to extend the constraint options for column constraints and create_constraint operation.

Example migration:

{
  "name": "50_create_table_with_table_constraint",
  "operations": [
    {
      "create_table": {
        "name": "phonebook",
        "columns": [
          {
            "name": "id",
            "type": "serial",
            "pk": true
          },
          {
            "name": "name",
            "type": "varchar(255)"
          },
          {
            "name": "city",
            "type": "varchar(255)"
          },
          {
            "name": "phone",
            "type": "varchar(255)"
          }
        ],
        "constraints": [
          {
            "name": "unique_numbers",
            "type": "unique",
            "columns": ["phone"],
            "index_parameters": {
              "include_columns": ["name"]
            }
          }
        ]
      }
    }
  ]
}

The table definition above turns into this table in PostgreSQL:

postgres=# \d phonebook
                                    Table "public.phonebook"
 Column |          Type          | Collation | Nullable |                Default
--------+------------------------+-----------+----------+---------------------------------------
 id     | integer                |           | not null | nextval('phonebook_id_seq'::regclass)
 name   | character varying(255) |           | not null |
 city   | character varying(255) |           | not null |
 phone  | character varying(255) |           | not null |
Indexes:
    "phonebook_pkey" PRIMARY KEY, btree (id)
    "unique_numbers" UNIQUE CONSTRAINT, btree (phone) INCLUDE (name)

Part of #580

@kvch kvch requested a review from andrew-farries January 10, 2025 15:03
Copy link
Collaborator

@andrew-farries andrew-farries left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good 👍 I left some comments.

I haven't had time to review the sql2pgroll part of the changes yet, will review that as soon as I can.

Once I am done with these table constraints, I will open a follow-up PR to extend the constraint options for column constraints and create_constraint operation.

Thanks, I was going to ask about potential duplication between the Constraint type added in this behaviour and the kinds of constraints that are possible via OpAlterColumn.

pkg/sql2pgroll/create_table_test.go Outdated Show resolved Hide resolved
schema.json Outdated Show resolved Hide resolved
schema.json Outdated Show resolved Hide resolved
Comment on lines +241 to +243
constraint += fmt.Sprintf("UNIQUE %s (%s)", nullsDistinct, strings.Join(quoteColumnNames(w.Columns), ", "))
constraint += w.addIndexParameters()
constraint += w.addDeferrable()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

optional: we could use strings.Builder and pass it to the addIndexParameters and addDeferrable methods to avoid all the string concatenation.

pkg/migrations/op_create_table.go Show resolved Hide resolved
pkg/sql2pgroll/convert.go Outdated Show resolved Hide resolved
pkg/sql2pgroll/create_table.go Outdated Show resolved Hide resolved
pkg/sql2pgroll/create_table_test.go Outdated Show resolved Hide resolved
pkg/sql2pgroll/create_table.go Outdated Show resolved Hide resolved
@@ -47,3 +47,4 @@
47_add_table_foreign_key_constraint.json
48_drop_tickets_check.json
49_unset_not_null_on_indexed_column.json
50_create_table_with_table_constraint.json
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be updated because I have another example in a PR....

@kvch kvch merged commit 43381e9 into xataio:main Jan 13, 2025
28 checks passed
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 this pull request may close these issues.

2 participants