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

Bugfix for #3429 String representations of RegexLiterals generated in… #3484

Merged
merged 2 commits into from
Oct 8, 2015

Conversation

dawbs
Copy link
Contributor

@dawbs dawbs commented Jul 28, 2015

Fix for bug #3429
corrected issue where string representations of RegexLiterals generated in influxql/ast.go added the / char as a start/end delimiter, but did not escape any / characters that may exist with the regex.

  • CHANGELOG.md updated
  • Rebased/mergable
  • Tests pass
  • Sign CLA (if not already signed)

},
&Query{
name: "single field (regex tag match with escaping)",
command: `SELECT value FROM db0.rp0.status_code WHERE url !~ /http\:\/\/www\.akamai\.com/`,
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't the URL be influxdb.com in this test to match with line 887?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There is an entry for both URLs with different values and then a test for match/no match. Might be a bit redundant.
Probably worth making the test URL influxdb.com regardless

@beckettsean
Copy link
Contributor

@pauldix @dgnorton regex PR

@beckettsean
Copy link
Contributor

@dawbs can you rebase to latest?

@dawbs dawbs force-pushed the dawbs-fix-3429 branch 2 times, most recently from 1f3c657 to 67760c7 Compare August 7, 2015 08:30
@dawbs
Copy link
Contributor Author

dawbs commented Aug 7, 2015

@beckettsean rebased and URLs updated as discussed as well

@dgnorton
Copy link
Contributor

dgnorton commented Aug 7, 2015

+1

dawbs added 2 commits October 8, 2015 19:41
…nerated in influxql/ast.go add the / char as a start and end delimiter, but does not escape any / characters that may exist with the regex
@dawbs
Copy link
Contributor Author

dawbs commented Oct 8, 2015

@beckettsean Have rebased this and fixed up the formatting changes to make circleci happy again - would be good to merge this in so I can push us back onto an upstream release :)

@dgnorton
Copy link
Contributor

dgnorton commented Oct 8, 2015

@otoolep @corylanou @DanielMorsing can one of you guys review?

@DanielMorsing
Copy link
Contributor

This looks alright to me, but I'm wondering if there are other special characters that need to be escaped.

@dgnorton
Copy link
Contributor

dgnorton commented Oct 8, 2015

@DanielMorsing not that I know of. We can fix those in a later PR if we run across them though so I'm going to go ahead and merge this.

dgnorton added a commit that referenced this pull request Oct 8, 2015
Bugfix for #3429 String representations of RegexLiterals generated in…
@dgnorton dgnorton merged commit a9bf213 into influxdata:master Oct 8, 2015
@dgnorton
Copy link
Contributor

dgnorton commented Oct 8, 2015

@dawbs thanks for following up and seeing it through!

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