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

Fix parsing keys when loading database index #6500

Merged
merged 1 commit into from
Apr 30, 2016
Merged

Fix parsing keys when loading database index #6500

merged 1 commit into from
Apr 30, 2016

Conversation

jwilder
Copy link
Contributor

@jwilder jwilder commented Apr 28, 2016

Required for all non-trivial PRs
  • Rebased/mergable
  • Tests pass
  • CHANGELOG.md updated

The code for parsing a key our of the WAL or TSM files in the engine
was naive and didn't account for measurements with escape chars. This
uses the correct parsing code to parse and load them correctly.

Fixes #6496

@jwilder
Copy link
Contributor Author

jwilder commented Apr 28, 2016

@e-dard

@jwilder jwilder added this to the 0.13.0 milestone Apr 29, 2016
return key[:idx]
// Ignoring the error because the func returns "missing fields"
k, _, _ := models.ParseKey(key)
return escape.UnescapeString(k)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this a breaking change? Looks like the keys were not previously unescaped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it has always been broken, but never discovered. This is called to parse a key already on disk.

@e-dard
Copy link
Contributor

e-dard commented Apr 29, 2016

Aside from my question about storing the keys, LGTM 👍

The code for parsing a key our of the WAL or TSM files in the engine
was naive and didn't account for measurements with escape chars. This
uses the correct parsing code to parse and load them correctly.

Fixes #6496
@jwilder jwilder merged commit 8a070d3 into master Apr 30, 2016
@jwilder jwilder deleted the jw-6496 branch April 30, 2016 21:20
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.

2 participants