fix(typeorm): correct quotes for column identifiers when driver is ‘mariadb‘

[constb]

#531 #545

TypeOrmCrudService.getFieldWithAlias escapes identifiers using backticks or double quotes.

it only uses backticks when this.dbName === 'mysql' but TypeORM also uses 'mariadb' to address a few subtle implementation differences.

this causes service to use double quotes in SQL and that fails with You have an error in your SQL syntax…

[jafio]

I tested the fix in a project that had the bug and it fixed it. I think it can be merged!

[adrianobnu]

I think this is better:

const i = ['mysql','mariadb'].includes(this.dbName) ? '`' : '"';

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/includes

[constb]

@adrianobnu personally I feel with only two comparisons the explicit code is better and more readable. but honestly whatever as long as a bug gets fixed, I'm fine with it… :)

[adrianobnu]

@adrianobnu personally I feel with only two comparisons the explicit code is better and more readable. but honestly whatever as long as a bug gets fixed, I'm fine with it… :)

Yes, it works anyway, but this alternative only makes 1 comparison (instead of 2 or more if you have other databases in the same situation), and I see no difficulties in those who see the code understand what it does.

[adrianobnu]

@zMotivat0r Can you do the review and approve the merge of this small fix?

[constb]

Yes, it works anyway, but this alternative only makes 1 comparison (instead of 2 or more if you have other databases in the same situation), and I see no difficulties in those who see the code understand what it does.

@adrianobnu actually, Array.prototype.includes() must make 2 comparisons to make sure the item isn't in array, and an additional function call. :)

if you're thinking about doing micro-optimisations, I think regexp would be a couple nanoseconds faster here…

[jeremyhalin]

Having same error.
This issue has been opened for 10 months now, it's a small fix, and it works fine for all MariaDB's users.
Can we merge it? How can we help?

[Nest] 11360   - 2021-04-09 23:09:08   [ExceptionsHandler] You have an error in your SQL syntax;
check the manual that corresponds to your MariaDB server version for the right syntax to use near '."id" = 1)' at line 1 +30543ms

[jspizziri]

Would love to see this merged.

[sgClaudia98]

any update on this??

[sgClaudia98]

This worked for me why is not merged?

[rewiko]

I am trying to help and have forked this repo (can't reach the owner of this repo and do not have the credential for the npm repo).

See #710 (comment)

We can hopefully merge it via rewiko#5

[rewiko]

Merged via rewiko#5

[sgClaudia98]

This change works!! Merge immediately