A table name glued to a multi-line column list no longer crashes the Postgres reader - #15
Merged
Merged
Conversation
…Postgres reader CREATE TABLE IF NOT EXISTS reactions( with the column list starting on the next line made cdclint panic (slice bounds out of range [-1:]). The reader found the column list by searching the statement for the name's word, but the word splitter folds whitespace inside parentheses, so a name glued to a paren that opens a multi-line list is not in the text verbatim and the index came back -1. The column list is now taken from the first parenthesis after TABLE, which the name cannot contain. Found by reading Mattermost's migrations (v10.11.0), where three files have this shape (000016_create_reactions, 000043_thread_memberships, 000098_create_post_acknowledgements). The same-line form, reports(id ...), already worked and still does.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What it changes
CREATE TABLE IF NOT EXISTS reactions(with the column list starting on the next line made cdclint panic:The reader located the column list by searching the statement for the name's word, but the word splitter folds whitespace inside parentheses, so a name glued to a paren that opens a multi-line list is not in the text verbatim and the index was -1. The column list is now taken from the first parenthesis after
TABLE, which the table name cannot contain. The same-line form (reports(id ...)) already worked and still does.How it was found
Reading Mattermost's migrations (v10.11.0) while proving the MySQL reader against a real repository. Three of its Postgres files have this shape:
000016_create_reactions,000043_thread_memberships,000098_create_post_acknowledgements. Any repository with one of these would crash cdclint rather than lint it.Verified
TestANameGluedToAMultiLineColumnListDoesNotPanicuses 000016's statement as written. It panics onmain(checked by running it against the old file) and passes with the fix, including the second column's line number.gofmt -l .clean,go vet,go test ./...pass; every corpus entry unchanged.0 error(s), 0 warning(s), 159 info, unchanged.Why a separate PR
It is a crash in the shipped Postgres reader, found while working on the MySQL reader (which has its own copy of the fix). One concern per PR.