Always quote schema name with backticks in MySQL queries.

fsprojects / SQLProvider

A general F# SQL database erasing type provider, supporting LINQ queries, schema exploration, individuals, CRUD operations and much more besides.

https://fsprojects.github.io/SQLProvider

Other

564 stars 144 forks source link

Always quote schema name with backticks in MySQL queries. #777

Closed Choc13 closed 1 year ago

Choc13 commented 1 year ago

Proposed Changes

Adds backtick quotes around all schema names in MySQL queries. Addresses #776.

Types of changes

[x] Bugfix (non-breaking change which fixes an issue)
[ ] New feature (non-breaking change which adds functionality)
[ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)

Checklist

[x] Build and tests pass locally (Build passes, but I wasn't able to get all the tests to run locally yet. Will keep trying.)
[ ] I have added tests that prove my fix is effective or that my feature works (if appropriate)
[ ] I have added necessary documentation (if appropriate)

Thorium commented 1 year ago

Thanks, this is definitely improvement.

Do you think the replace is too naive? (I know it was there already.) Like what if your table/column contains a weird name like `Person[backup]` ?

Choc13 commented 1 year ago

That is a good point. I wondered the same but didn't change it as I saw a comment about [] being used as an internal representation so figured the replacement was there in order to switch to the MySQL specific quotations. However I think you are right that if the table/schéma did also contain those chars then they would be incorrectly substituted.

It seems like a separate change set though to remodel the internal quoting behaviour to address this. I would have thought it should be sufficient to not use any quote symbol internally and just do vendor specific quoting in each provider, but there are likely reasons I'm not aware of given I've only spent an hour or so looking over this code.

Thorium commented 1 year ago

You are correct, thanks, will release this.