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

Support for NO_BACKSLASH_ESCAPES #701

Closed
lauxjpn opened this issue Sep 21, 2019 · 8 comments
Closed

Support for NO_BACKSLASH_ESCAPES #701

lauxjpn opened this issue Sep 21, 2019 · 8 comments

Comments

@lauxjpn
Copy link
Contributor

lauxjpn commented Sep 21, 2019

As previously discussed here:
PomeloFoundation/Pomelo.EntityFrameworkCore.MySql#827

A connection string setting as an opt-in feature activates a special mode in the state machine of SqlParser to handle the sql_mode NO_BACKSLASH_ESCAPES correctly for strings, when translating parameters into inline literals (e.g. when calling MySqlCommand.Execute) or splitting the statements (e.g. when calling Prepare).

@bgrainger
Copy link
Member

bgrainger commented Sep 21, 2019

Two possible ways of approaching this problem:

  • Add NoBackslashEscapes (default false); when true, performs string escaping and SQL parsing in a way compatible with the NO_BACKSLASH_ESCAPES SQL mode.
  • Add DetectSqlMode (default false); when true, executes SELECT @@sql_mode; when a connection is made, and automatically turns on backslash escaping if NO_BACKSLASH_ESCAPES is detected.

Advantages of the latter: user doesn't have to specify (or guess) the escaping mode; extensible in the future to other modes (ANSI_QUOTES? NO_ZERO_DATE (although already handled by AllowZeroDateTime)?); no functional harm from always setting it to true; not a breaking change to default it to true in the future.

Disadvantages of the latter: performance penalty for checking sql_mode at connect time; there's already precedence (AllowZeroDateTime) for a connection string setting controlling one aspect of SQL Mode functionality.

AFAIK there is no relevant Connector/NET option to be compatible with here. (I think it may always check @@sql_mode on each connection.)

@bgrainger
Copy link
Member

NoBackslashEscapes is the simplest thing that could possibly work, and is compatible with how Pomelo will use this library.

DetectSqlMode could be added in the future and override the default value of NoBackslashEscapes.

@lauxjpn
Copy link
Contributor Author

lauxjpn commented Sep 21, 2019

DetectSqlMode could be added in the future and override the default value of NoBackslashEscapes.

Absolutely. Also consider this approach:

To track the mode, the incomming queries would need to be parsed to some extend. Probably looking for sql_mode = and if so, querying the sql_mode and saving it after executing the original query.
So the cost would be an additional roundtrip when sql_mode appears in a query.

It would have the advantage of not triggering an automatic round trip when opening the connection, but only, when the mode is being changed and would detect changes in the mode during the lifetime of the connection.
On the other hand, knowing from the start, what sql modes are enabled (they could be specified in the my.ini/my.cnf as default options), is definitely advantages.

NoBackslashEscapes is the simplest thing that could possibly work, and is compatible with how Pomelo will use this library.

I think this is a good starting point.

lauxjpn added a commit to lauxjpn/MySqlConnector that referenced this issue Sep 21, 2019
…derived classes and MySqlParameter.

Single Quotes are now always escaped with two single quotes instead of a leading backspace.
This addresses everything except triggering/detecting the NO_BACKSLASH_ESCAPES from mysql-net#701 and PomeloFoundation/Pomelo.EntityFrameworkCore.MySql#827
lauxjpn added a commit to lauxjpn/MySqlConnector that referenced this issue Sep 21, 2019
…derived classes and MySqlParameter.

Single quotes are now always escaped with an additional single quote instead of a leading backspace.
This addresses everything except triggering/detecting the NO_BACKSLASH_ESCAPES from mysql-net#701 and PomeloFoundation/Pomelo.EntityFrameworkCore.MySql#827
lauxjpn added a commit to lauxjpn/MySqlConnector that referenced this issue Sep 21, 2019
…derived classes and MySqlParameter.

Single quotes are now always escaped with an additional single quote instead of a leading backspace.
This addresses everything except triggering/detecting the NO_BACKSLASH_ESCAPES from mysql-net#701 and PomeloFoundation/Pomelo.EntityFrameworkCore.MySql#827
lauxjpn added a commit to lauxjpn/MySqlConnector that referenced this issue Sep 21, 2019
…derived classes and MySqlParameter.

Single quotes are now always escaped with an additional single quote instead of a leading backspace.
This addresses everything except triggering/detecting the NO_BACKSLASH_ESCAPES from mysql-net#701 and PomeloFoundation/Pomelo.EntityFrameworkCore.MySql#827

Signed-off-by: Laurents Meyer <[email protected]>
bgrainger pushed a commit that referenced this issue Sep 23, 2019
…derived classes and MySqlParameter.

Single quotes are now always escaped with an additional single quote instead of a leading backspace.
This addresses everything except triggering/detecting the NO_BACKSLASH_ESCAPES from #701 and PomeloFoundation/Pomelo.EntityFrameworkCore.MySql#827

Signed-off-by: Laurents Meyer <[email protected]>
bgrainger pushed a commit that referenced this issue Sep 23, 2019
…derived classes and MySqlParameter.

Single quotes are now always escaped with an additional single quote instead of a leading backspace.
This addresses everything except triggering/detecting the NO_BACKSLASH_ESCAPES from #701 and PomeloFoundation/Pomelo.EntityFrameworkCore.MySql#827

Signed-off-by: Laurents Meyer <[email protected]>
bgrainger pushed a commit that referenced this issue Sep 23, 2019
…derived classes and MySqlParameter.

Single quotes are now always escaped with an additional single quote instead of a leading backspace.
This addresses everything except triggering/detecting the NO_BACKSLASH_ESCAPES from #701 and PomeloFoundation/Pomelo.EntityFrameworkCore.MySql#827

Signed-off-by: Laurents Meyer <[email protected]>
@lauxjpn
Copy link
Contributor Author

lauxjpn commented Sep 24, 2019

NoBackslashEscapes is the simplest thing that could possibly work, and is compatible with how Pomelo will use this library.

The connection string option is now all that is left to do. Do you want to implement it, or should I?

@bgrainger
Copy link
Member

I can do it.

@lauxjpn
Copy link
Contributor Author

lauxjpn commented Sep 24, 2019

Perfect! I close this one then.

@lauxjpn lauxjpn closed this as completed Sep 24, 2019
@bgrainger
Copy link
Member

I plan to ship an updated package later today.

@bgrainger
Copy link
Member

Added in 0.58.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants