Add sequence support #35
Conversation
| } | ||
|
|
||
| func (o oldAndNew[S]) GetOld() S { | ||
| return o.new | ||
| return o.old |
There was a problem hiding this comment.
Crazy bug. I'm shocked this was working, but I'm guessing it's because we only use this function in a few places
bplunkett-stripe
force-pushed
the
bplunkett/serial-suppport
branch
from
977d5cd to
ecdb0c3
Compare
bplunkett-stripe
force-pushed
the
bplunkett/serial-suppport
branch
from
ecdb0c3 to
8538e24
Compare
| SequenceOwner struct { | ||
| TableName SchemaQualifiedName | ||
| // TableUnescapedName is a hackaround until we switch over Tables ot use fully-qualified, escaped names | ||
| TableUnescapedName string |
There was a problem hiding this comment.
thought: Add a deprecated docstring to reduce further usages? Not sure if this would actually work how we want it https://stackoverflow.com/questions/7849663/how-do-you-mark-code-as-deprecated-in-go
There was a problem hiding this comment.
The issue stems from using the unescaped name in the Table schema object. I don't think marking the field as deprecated indicates where the problem actually arises from (the Table schema object's logic for generating name). I do have a ticket to fix this though!
pkg/diff/sql_generator.go
Outdated
| if !cmp.Equal(diff.old, diff.new) { | ||
| // Technically, we could support altering expression, but I don't see the use case for it. it would require more test | ||
| // cases than forceReadding it, and I'm not convinced it unlocks any functionality | ||
| return nil, fmt.Errorf("altering check constraint to resolve the following diff %s: %w", cmp.Diff(diff.old, diff.new), ErrNotImplemented) |
There was a problem hiding this comment.
This error appears to refer to check constraints and should refer to sequence alters.
There was a problem hiding this comment.
Good spot!
| return []Statement{{ | ||
| DDL: fmt.Sprintf("DROP SEQUENCE %s", seq.GetFQEscapedName()), | ||
| Timeout: statementTimeoutDefault, | ||
| Hazards: hazards, |
There was a problem hiding this comment.
Dropping a sequence will destroy state (the internal value), so there should be a hazard for that right? This should trigger on recreates in the absence of alters.
bplunkett-stripe
force-pushed
the
bplunkett/serial-suppport
branch
from
d0863c7 to
2334bb7
Compare
pkg/diff/sql_generator.go
Outdated
| var hazards []MigrationHazard | ||
| hazards := []MigrationHazard{{ | ||
| Type: MigrationHazardTypeDeletesData, | ||
| Message: "By deleting a sequence, its state will be permanently lost", |
There was a problem hiding this comment.
nit: Maybe replace "its state" with "its current value" to be specific?
Description
Add sequence (and serial support). A little gross because we all columns ops are grouped in the same add/alter block.
Motivation
#34
Testing
Tested via acceptance tests