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

Bump the yacc pointer. #1804

Merged
merged 1 commit into from
Jul 24, 2015
Merged

Bump the yacc pointer. #1804

merged 1 commit into from
Jul 24, 2015

Conversation

petermattis
Copy link
Collaborator

Use the newer yacc to re-generate the sql grammar.

Fixes #1801.

@@ -129,7 +129,7 @@ check:
grep -vE '(declaration of err shadows|^vet: cannot process directory \.git)'
@echo "golint"
@! golint $(PKG) | \
grep -vE '(\.pb\.go|embedded\.go|_string\.go|LastInsertId|sql\.y)' \
grep -vE '(\.pb\.go|embedded\.go|_string\.go|LastInsertId|sql/parser/(yaccpar|sql\.y):)' \
Copy link
Contributor

Choose a reason for hiding this comment

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

what is yaccpar?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It is the "file" containing the generated parser code and is annotated in sql.go with:

//line yaccpar:1

@tamird
Copy link
Contributor

tamird commented Jul 24, 2015

LGTM

Use the newer yacc to re-generate the sql grammar.

Fixes #1801.
petermattis added a commit that referenced this pull request Jul 24, 2015
@petermattis petermattis merged commit eab419f into master Jul 24, 2015
@petermattis petermattis removed the PTAL label Jul 24, 2015
@petermattis petermattis deleted the pmattis/sql-yacc branch July 24, 2015 19:32
@tbg
Copy link
Member

tbg commented Jul 25, 2015

LGTM.
do we know that this reliably fixes this class of panic?

@petermattis
Copy link
Collaborator Author

@tschottdorf The code that caused the panic has seen changes in the updated yacc code. I didn't look at the logs, but I'd guess that the similar problem go-fuzz found in the vitess parser resulted in changes to yacc.

@tbg
Copy link
Member

tbg commented Jul 25, 2015

the posts in the sister thread in vitess sounded like "I dunno but it seems to work now". See vitessio/vitess#767.

@petermattis
Copy link
Collaborator Author

I was referring to the the git logs for yacc.go. Brief inspection right now reveals that golang/go@a034e47 is most likely the source of the fix as the line of code just after the 7 new ones in that commit is where the index out of bounds panic occurred.

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