[wp-trac] [WordPress Trac] #54042: Extending wpdb::prepare() to support table/field names, and IN() operator
WordPress Trac
noreply at wordpress.org
Sat May 7 04:21:17 UTC 2022
#54042: Extending wpdb::prepare() to support table/field names, and IN() operator
-------------------------------------------------+-------------------------
Reporter: craigfrancis | Owner: (none)
Type: enhancement | Status: new
Priority: normal | Milestone: Future
| Release
Component: Database | Version:
Severity: normal | Resolution:
Keywords: has-patch dev-feedback needs- | Focuses:
testing early has-unit-tests |
-------------------------------------------------+-------------------------
Comment (by apokalyptik):
@craigfrancis
I've given the code a little review and a little thought. tested a few
things, and it looks pretty good! A lot of streamlining was possible when
moving it out of a proof like I had it and into the prepare function
itself.
I'm not sure I, personally, like the variadic-alike syntax here. I feel
something more terse with a single not-currently-used character is more
appropriate with its peers.
I think if you use a single array for the args instead of arg_variadics
and arg_identifiers and whatnot and make the value richer and use $arg_id
as the key you can take out all of the multiple array searching you're
doing at the end of prepare, making the whole thing lighter.
I would also encourage expanding your logic up top with the passed as
array checks. There's no need for it to be so terse, and using multiple
lines and statements and checks will make it more understandable at a
glance.
I especially like your use of PREG_SPLIT_DELIM_CAPTURE, and how you used
it to make parsing the specifiers simpler. That was very nice. that substr
-1 for the specifier was so good.
Overall very good quality code here, though. Nice work.
--
Ticket URL: <https://core.trac.wordpress.org/ticket/54042#comment:30>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list