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

asserts: implement Database.FindSequence #8907

Merged
merged 2 commits into from
Jul 7, 2020

Conversation

pedronis
Copy link
Collaborator

FindSequence finds a sequence-forming assertion based the given
headers. Provided headers must contain a prefix of the primary key
for the assertion type except for the sequence header.
Th assertion is the first in the sequence under the prefix with
sequential number > after.
If after is -1 it returns instead the assertion with the largest
sequential number.

@pedronis
Copy link
Collaborator Author

based on #8906

FindSequence finds a sequence-forming assertion based the given
headers. Provided headers must contain a prefix of the primary key
for the assertion type except for the sequence header.
Th assertion is the first in the sequence under the prefix with
sequential number > after.
If after is -1 it returns instead the assertion with the largest
sequential number.
return nil, err
}
if !assertType.SequenceForming() {
return nil, fmt.Errorf("cannot use FindSequence with not sequence-forming assertion type %q", assertType.Name)
Copy link
Contributor

Choose a reason for hiding this comment

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

s/not/non/ ? (not sure, cc @degville )

Copy link
Contributor

Choose a reason for hiding this comment

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

s/not/non/ ? (not sure, cc @degville )

you're right - it should be cannot use FindSequence with non sequence-forming assertion type

@bboozzoo bboozzoo self-requested a review July 7, 2020 06:40
Copy link
Contributor

@stolowski stolowski left a comment

Choose a reason for hiding this comment

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

LGTM, with one nitpick about error message

Copy link
Contributor

@bboozzoo bboozzoo left a comment

Choose a reason for hiding this comment

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

LGTM with a tiny suggestion about a comment

}
}

seqKey, err := keysFromHeaders(assertType.PrimaryKey[:len(assertType.PrimaryKey)-1], sequenceHeaders)
Copy link
Contributor

Choose a reason for hiding this comment

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

// all keys but the last one which is the sequence number

@pedronis
Copy link
Collaborator Author

pedronis commented Jul 7, 2020

@bboozzoo @stolowski I'll land this and address the two small points in a follow up

@pedronis pedronis merged commit 4844511 into canonical:master Jul 7, 2020
@pedronis pedronis deleted the asserts-find-sequence branch July 8, 2020 12:15
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.

4 participants