berlindb / core

All of the required core code
MIT License
255 stars 29 forks source link

WHERE clause with integers only #9

Closed szepeviktor closed 2 years ago

szepeviktor commented 5 years ago

Is it possible to add a WHERE <field> IN ( <int>, <int> ... ) clause? So quotes will not be inserted while escaping, unlike this: WHERE post_id IN ("1", "2", "3")

alexstandiford commented 4 years ago

Take a look at this PR: https://github.com/berlindb/core/pull/20

I am working on a method, where_clause, that may cover what you're trying to-do. If it doesn't, it's possible that it can be changed to-do so.

szepeviktor commented 4 years ago

Thank you.

JJJ commented 3 years ago

Did some research into this.

These values are always escaped into strings as a general precaution against SQL injection.

Quotes are added when $this->get_db()->_escape() is called in the Query class.

For a column that is always only %d format, this could theoretically be integers. Does sending integers through the pipe instead of strings provide any real benefit?

szepeviktor commented 3 years ago

Strictly speaking, the quotes are unnecessary, and force MySQL to do a typecasting/conversion, so it wastes a bit of CPU time. In practice, unless you're running a Google-sized operation, such conversion overhead is going to be microscopically small.

https://stackoverflow.com/a/6782019

I run a similar sized business as Google.

szepeviktor commented 3 years ago

If the ANSI_QUOTES SQL mode is enabled, it is also permissible to quote identifiers within double quotation marks

https://dev.mysql.com/doc/refman/8.0/en/identifiers.html

JJJ commented 3 years ago

ANSI_QUOTES is not listed as an incompatible mode of WordPress's WPDB class, so that double quotes remain an option, but I would think that has the same problem, no?

I've started working on a PR for this. My early opinion is this is nearly imperceptible performance wise, and a little weird on the code maintenance side, but I have more work to do to make it cleaner.

We can use the Column::pattern property to generate different SQL statements, but then it's not just here that's an issue; it's pretty much everywhere.

@szepeviktor would you be able to test a PR when one is available?

szepeviktor commented 3 years ago

would you be able to test a PR when one is available?

@JJJ Yes. Please give me a one-sentence instruction.

JJJ commented 3 years ago

I have not forgotten about this, though I have not been able to re-prioritize it.

Just letting you know.

JJJ commented 2 years ago

Completed via #140 for 2.1.0.