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 pointers for schema and Table struct fields #574

Merged
merged 6 commits into from
Jan 9, 2025

Conversation

andrew-farries
Copy link
Collaborator

Update the schema and Tables structs in the schema package to use pointer types in fields.

As part of #239, the in-memory schema representation will need to be updated more often and more accurately by individual operations in a migration. Using pointers in these schema package structs makes this possible.

Part of #239

@andrew-farries andrew-farries marked this pull request as ready for review January 8, 2025 09:53
@kvch
Copy link
Contributor

kvch commented Jan 8, 2025

Just to be clear, it means that we will not maintain a separate virtual schema for in progress operations? Instead everything will be stored in the same schema, not just the physical schema?

@andrew-farries
Copy link
Collaborator Author

Just to be clear, it means that we will not maintain a separate virtual schema for in progress operations? Instead everything will be stored in the same schema, not just the physical schema?

Yes, that's right.

The virtual schema is updated by each operation in a migration to reflect the changes made by that operation. I don't think we need a separate virtual schema for in-progress operations.

In practice, the only changes that need to be made to the virtual schema by each operation are those changes that involve creating indirections. For example, when an OpRenameTable renames a table from foo to bar the virtual schema is updated to look like:

{
  "tables": {
    "bar": {
      "name": "foo",
      ...
    },
    ...
  }
}

Likewise if a column is renamed by an OpAlterColumn from foo to bar the virtual schema is updated to look like:

{
  "tables": {
    "some_table": {
      "name": "some_table",
      "columns": {
        "bar": { "name": "foo",  ...},
      },
      ...
    },
    ...
  }
}

This is because ultimately, the only use for the virtual schema is to be able to create the views in the version schema at the end of the migration for which precise details about changes to column types and constraints etc is not needed.

@andrew-farries andrew-farries force-pushed the use-pointers-in-schema-struct branch from 96111a1 to 86ff3dd Compare January 8, 2025 15:51
Change the type of `schema.Tables` from `map[string]schema.Table` to
`map[string]*schema.Table`. This is so that methods that will need to
make changes to the virtual schema can make changes to tables in the
virtual schema reliably.
Change the type of `Table.Columns` in the schema package from
`map[string]Column` to `map[string]*Column`. This allows methods that
need to update the virtual schema to do so more easily, after looking up
a column by name.
Change the type of `Table.Indexes` in the schema package from
`map[string]Index` to `map[string]*Index`. This allows methods that need
to update the virtual schema to do so more easily, after looking up an
index by name.
Change the type of `Table.ForeignKeys` in the schema package from
`map[string]ForeignKey` to `map[string]*ForeignKey`. This allows methods
that need to update the virtual schema to do so more easily, after
looking up a foreign key by name.
Change the type of `Table.CheckConstraints` in the schema package from
`map[string]CheckConstraint` to `map[string]*CheckConstraint`. This allows methods
that need to update the virtual schema to do so more easily, after
looking up a check constraint by name.
Change the type of `Table.UniqueConstraints` in the schema package from
`map[string]UniqueConstraint` to `map[string]*UniqueConstraint`. This
allows methods that need to update the virtual schema to do so more
easily, after looking up a unique constraint by name.
@andrew-farries andrew-farries force-pushed the use-pointers-in-schema-struct branch from 86ff3dd to 31bf275 Compare January 8, 2025 16:09
@andrew-farries andrew-farries merged commit daba767 into main Jan 9, 2025
28 checks passed
@andrew-farries andrew-farries deleted the use-pointers-in-schema-struct branch January 9, 2025 06:40
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.

3 participants