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

SqlBuilder regex expression removes chars outside /** **/ clauses #436

Closed
feugen24 opened this issue Jan 15, 2016 · 4 comments
Closed

SqlBuilder regex expression removes chars outside /** **/ clauses #436

feugen24 opened this issue Jan 15, 2016 · 4 comments

Comments

@feugen24
Copy link

The issue is on SqlBuilder line 67:

private static readonly Regex _regex = new Regex(@"\/\*\*.+\*\*\/", RegexOptions.Compiled | RegexOptions.Multiline);

Given the following sql input:

 select table.* FROM SomeTable as table /**innerjoin**/  /**fulljoin**/  /**leftjoin**/  /**rightjoin**/ INNER          JOIN some join data /**where**/  /**orderby**/ OFFSET @skip ROWS FETCH NEXT @take ROWS    ONLY

it deletes "INNER JOIN some join data" if where/order by clause are not present.
It can be tested online on:
http://regexstorm.net/tester
with above sqlinput and \/\*\*.+\*\*\/ expression.

If I change the expression to /\*\*(.+?)\*\*/ seems to work fine.

@madagaga
Copy link

Same as #280 ?

@feugen24
Copy link
Author

feugen24 commented Feb 1, 2016

It's the same problem.

I tested input from here and #280 with both proposed regular expressions

\/\*\*[^\*]*\*\*\/ 
/\*\*(.+?)\*\*/

on http://regexstorm.net/tester tester side by side and they give similar results except for the split-list tab(regex split function?) where the expression in #280 omits some results.

I'm not sure what expression is better.

@RCGodward
Copy link

We started encountering this issue today. Guess we've been lucky before. Adding the question mark fixed it with no ill effects in our codebase.

\/\*\*.+?\*\*\/

@jods4
Copy link
Contributor

jods4 commented Oct 5, 2016

This can be closed as a dup of #280.

jods4 added a commit to jods4/dapper-dot-net that referenced this issue Oct 5, 2016
jods4 added a commit to jods4/dapper-dot-net that referenced this issue Oct 5, 2016
Fixes DapperLib#436, DapperLib#280
My editor has also normalized a few line endings to LF like 99% of the
file was.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants