Fix column renames for indexes #147
Conversation
| @@ -1718,6 +1718,7 @@ func (isg *indexSQLVertexGenerator) addDepsOnTableAddAlterIfNecessary(index sche | |||
| } | |||
| } | |||
|
|
|||
| // Otherwise, we can drop the index whenever we want. | |||
There was a problem hiding this comment.
Initially was debugging this problem and thought adding a comment helped document it a little better
| Indexes: []Index{ | ||
| { | ||
| OwningTable: SchemaQualifiedName{SchemaName: "public", EscapedName: "\"foo\""}, | ||
| Name: "foo_value_idx", Columns: []string{"renamed_value"}, |
There was a problem hiding this comment.
Previously, it would have fetched the column as "value" instead of "renamed_value"
bplunkett-stripe
force-pushed
the
bplunkett/fix-column-renames-for-indexes
branch
from
fdaed98 to
8320e14
Compare
| SELECT ARRAY_AGG(att.attname ORDER BY indkey_ord.ord) | ||
| FROM UNNEST(i.indkey) WITH ORDINALITY AS indkey_ord (attnum, ord) |
There was a problem hiding this comment.
The ordinality here is just to make it explicit that the unnested array should be aggregated back in the same order after converting to column names right? And this isn't technically necessary since that should be the order anyways?
There was a problem hiding this comment.
Yep exactly. It's to enforce the order. I don't think it's technically necessary, but I definitely feel more comfortable with explicit ORDER BY
| @@ -95,6 +95,13 @@ SELECT | |||
| i.indisunique AS index_is_unique, | |||
| COALESCE(parent_c.relname, '')::TEXT AS parent_index_name, | |||
| COALESCE(parent_namespace.nspname, '')::TEXT AS parent_index_schema_name, | |||
| ( | |||
| SELECT ARRAY_AGG(att.attname ORDER BY indkey_ord.ord) | |||
There was a problem hiding this comment.
I almost wonder if it's worth a comment on this block here that this is just i.indkey.map(attnum -> column_name(attnum)) but in sql.
There was a problem hiding this comment.
Might leave out the comment, since it's probably best to read from the docs on it. Pretty nifty SQL function!
Description
Index columns effectively have two occurrences in pg_attribute, one is a "virtual column" created for the index and the other is the actual referenced column. For our purposes, we care about the latter, i.e., ensuring an index is deleted before its owning column is dropped, otherwise the index will be implicitly deleted (and not concurrently).
When a column is renamed, the virtual column name stays the same, but the underlying column name obviously changes. pg-schema-diff contained a bug where it referenced the virtual column name. Effectively, a column would be renamed but the column names in the fetched index would not change, leading to incorrectly generated dependencies.
I found this thread in the Postgres maling list to be very useful.
What I did:
Motivation
Bug fix
Testing
See added test case