Everything

php

[RFC] Debugging PDO Prepared Statement Emulation v2

php.internals

Adam Baratz

9 years ago

Hi,

Based on my second thoughts on my initial approach, I created an RFC to
supplant it:
https://wiki.php.net/rfc/debugging_pdo_prepared_statement_emulation_v2

I believe this better addresses the negative feedback from the original
RFC[1] and is a better solution to the problem.

Thanks,
Adam

---
[1] https://wiki.php.net/rfc/debugging_pdo_prepared_statement_emulation

Adam Baratz

9 years ago

Hi,

> Based on my second thoughts on my initial approach, I created an RFC to
> supplant it:
> https://wiki.php.net/rfc/debugging_pdo_prepared_statement_emulation_v2
>

I'd appreciate any feedback on this RFC, especially from any of the people
who voted against the original[1].

Thanks,
Adam

---
[1] https://wiki.php.net/rfc/debugging_pdo_prepared_statement_emulation

Matteo Beccati

9 years ago

Hi Adam,

On 28/11/2016 16:29, Adam Baratz wrote:

>> Based on my second thoughts on my initial approach, I created an RFC to
>> supplant it:
>> https://wiki.php.net/rfc/debugging_pdo_prepared_statement_emulation_v2
>>
>
> I'd appreciate any feedback on this RFC, especially from any of the people
> who voted against the original[1].

v2 seems more reasonable to me. I would not use "Parsed SQL" though, as
it's not just the result of parsing. In fact it could effectively be
what has been sent to the backend, regardless of statement emulation...
however, nothing better than "Sent SQL" comes to mind atm.

I.e. in case of pdo_pgsql:

SQL: "SELECT * FROM tbl WHERE x = ?"
Sent SQL: "SELECT * FROM tbl WHERE x = $1"

(could be available before execute)

or w/ emulated prepares:

Sent SQL: "SELECT * FROM tbl WHERE x = 'foo'"

(only available after execute)

For consistency, I would prefer to only add the information after
execute() has been called.


Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Adam Baratz

9 years ago

Hi,

I would not use "Parsed SQL" though, as it's not just the result of

> parsing. In fact it could effectively be what has been sent to the backend,
> regardless of statement emulation...
>

I'd be happy to make the feature more specific. The "Parsed" line would
only show with emulated prepares enabled, after execute() has been called.
I'd prefer using "Parsed" because the language would be more specific. It
would also correspond to pdo_parse_params(), the function which does the
emulation.

Basically, you'd never see this kind of example:

SQL: "SELECT * FROM tbl WHERE x = ?"

> Sent SQL: "SELECT * FROM tbl WHERE x = $1"
>

I haven't tested, but I'm pretty sure the feature could be implemented by
adding this block below the first php_stream_printf() call in
debugDumpParams():

/* show parsed SQL if emulated prepares enabled */

> /* pointers will be equal if PDO::query() was invoked */
> if (stmt->active_query_string != NULL && stmt->active_query_string !=
> stmt->query_string) {
>   php_stream_printf(out, "Parsed SQL: [%zd] %.*s\n",
> stmt->active_query_stringlen, (int) stmt->active_query_stringlen,
> stmt->active_query_string);
> }

Let me know what you think of this approach. If it sounds fine, I'll update
the RFC.

Thanks,
Adam

Matteo Beccati

9 years ago

Hi Adam,

On 29/11/2016 00:16, Adam Baratz wrote:

> I'd be happy to make the feature more specific. The "Parsed" line would
> only show with emulated prepares enabled, after execute() has been called.
> I'd prefer using "Parsed" because the language would be more specific. It
> would also correspond to pdo_parse_params(), the function which does the
> emulation.

Sure, it might be the name of the function, but technically it's parsing
+ interpolation.

> Basically, you'd never see this kind of example:
>
> SQL: "SELECT * FROM tbl WHERE x = ?"
>> Sent SQL: "SELECT * FROM tbl WHERE x = $1"
>>

why not? That's what active_query_string contains e.g. in pdo_pgsql w/o
emulate prepares. Which is more or less what has been sent to the
server, unless cursors are used. I don't see why it is deemed to be
not-useful or less useful than emulated prepares.

In fact it would come in very handy for extended tests of a bugfix I
have in the pipeline.

> I haven't tested, but I'm pretty sure the feature could be implemented by
> adding this block below the first php_stream_printf() call in
> debugDumpParams():
>
> /* show parsed SQL if emulated prepares enabled */
>> /* pointers will be equal if PDO::query() was invoked */
>> if (stmt->active_query_string != NULL && stmt->active_query_string !=
>> stmt->query_string) {
>>   php_stream_printf(out, "Parsed SQL: [%zd] %.*s\n",
>> stmt->active_query_stringlen, (int) stmt->active_query_stringlen,
>> stmt->active_query_string);
>> }
>
>
> Let me know what you think of this approach. If it sounds fine, I'll update
> the RFC.

Possibly, but tbh I haven't had time to check.

Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Adam Baratz

9 years ago

Hi,

> Basically, you'd never see this kind of example:
> >
> > SQL: "SELECT * FROM tbl WHERE x = ?"
> >> Sent SQL: "SELECT * FROM tbl WHERE x = $1"
> >>
>
> why not? That's what active_query_string contains e.g. in pdo_pgsql w/o
> emulate prepares. Which is more or less what has been sent to the
> server, unless cursors are used. I don't see why it is deemed to be
> not-useful or less useful than emulated prepares.
>

active_query_string is only set with emulated prepares. I tried my patch
with pdo_pgsql. "Sent SQL" doesn't appear with real prepares. I put it up
here, with tests, if it'll help try it out:
https://github.com/adambaratz/php-src/commit/a030d2f4e6a13f3e6c10484fb5dbc4e8be6576c6

I'll incorporate it, and the content of this subthread, in the RFC when I
get my next free five minutes. :)

In fact it would come in very handy for extended tests of a bugfix I

> have in the pipeline.
>

I'm not sure how to get the query with the $1. My understanding of
pdo_pgsql/pgsql_stmt_execute() is that it creates some PG-specific data
structures for prepared statements. Since they're attached to
a pdo_pgsql_stmt value, you'd have to write some driver-specific code to
get them out.

Maybe there's a simple way to do what you're describing, but my instinct is
that it would be better to keep it out-of-scope for this RFC.

Thanks,
Adam

Adam Baratz

9 years ago

>
> I'll incorporate it, and the content of this subthread, in the RFC when I
> get my next free five minutes. :)
>

Done. And because of the scope of the v2 implementation, I changed the
target release to next 7.1.x.

Thanks,
Adam

Matteo Beccati

9 years ago

Hi,

On 01/12/2016 18:38, Adam Baratz wrote:

>>
>> I'll incorporate it, and the content of this subthread, in the RFC when I
>> get my next free five minutes. :)
>>
>
> Done. And because of the scope of the v2 implementation, I changed the
> target release to next 7.1.x.

Could you please revert? Albeit minor, it's a new feature and therefore
it belongs to 7.2.

We shouldn't try to arbitrarily squeeze things into stable releases.

Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Adam Baratz

9 years ago

>
> Could you please revert? Albeit minor, it's a new feature and therefore
> it belongs to 7.2.

No problem. Reverted.

Benoit SCHILDKNECHT

9 years ago

Le Thu, 17 Nov 2016 23:19:38 +0100, Adam Baratz <adambaratz@php.net> a
écrit:

> Hi,
>
> Based on my second thoughts on my initial approach, I created an RFC to
> supplant it:
> https://wiki.php.net/rfc/debugging_pdo_prepared_statement_emulation_v2
>
> I believe this better addresses the negative feedback from the original
> RFC[1] and is a better solution to the problem.
>
> Thanks,
> Adam
>
> ---
> [1] https://wiki.php.net/rfc/debugging_pdo_prepared_statement_emulation

So, if I only want to get the emulated prepared statement, I have to do
ob_start()/ob_get_clean(), then use a regexp to fetch it ? I want v1 back
T_T

Thomas Nunninger

9 years ago

Hi,

> So, if I only want to get the emulated prepared statement, I have to do
> ob_start()/ob_get_clean(), then use a regexp to fetch it ? I want v1
> back T_T

I second that. Why do you print it directly? Wouldn't it be better to
return a structured array with the information needed? If needed, you
can var_dump() the array yourself.

Regards
Thomas

Matteo Beccati

9 years ago

On 02/12/2016 08:36, Thomas Nunninger wrote:

> Hi,
>
>> So, if I only want to get the emulated prepared statement, I have to do
>> ob_start()/ob_get_clean(), then use a regexp to fetch it ? I want v1
>> back T_T
>
> I second that. Why do you print it directly? Wouldn't it be better to
> return a structured array with the information needed? If needed, you
> can var_dump() the array yourself.

Could you please explain what's the use case? As far as I'm concerned
such information would only be useful during PDO driver development,
phpt files and bug reporting / fixing.


Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Thomas Nunninger

9 years ago

Hi,

On 12/02/2016 02:54 PM, Matteo Beccati wrote:

> On 02/12/2016 08:36, Thomas Nunninger wrote:
>> Hi,
>>
>>> So, if I only want to get the emulated prepared statement, I have to do
>>> ob_start()/ob_get_clean(), then use a regexp to fetch it ? I want v1
>>> back T_T
>>
>> I second that. Why do you print it directly? Wouldn't it be better to
>> return a structured array with the information needed? If needed, you
>> can var_dump() the array yourself.
>
> Could you please explain what's the use case? As far as I'm concerned
> such information would only be useful during PDO driver development,
> phpt files and bug reporting / fixing.

Sometimes I'd like to log what SQL is really used that I can
analyze/modify a query without the need to replace all bound parameters
manually before applying it to the command line of my database.

Additionally, it's a matter of good style in my opinion. E.g. if you
return an array you can extend the structure later. If some test checks
a string output you have to change this. If it checks the values of some
array keys, the test is stable as long as you just add new keys.

Regards
Thomas

Matteo Beccati

9 years ago

Hi,

On 02/12/2016 15:20, Thomas Nunninger wrote:

>> Could you please explain what's the use case? As far as I'm concerned
>> such information would only be useful during PDO driver development,
>> phpt files and bug reporting / fixing.
>
> Sometimes I'd like to log what SQL is really used that I can
> analyze/modify a query without the need to replace all bound parameters
> manually before applying it to the command line of my database.
>
> Additionally, it's a matter of good style in my opinion. E.g. if you
> return an array you can extend the structure later. If some test checks
> a string output you have to change this. If it checks the values of some
> array keys, the test is stable as long as you just add new keys.

Your planned usage only applies if you use emulated prepared statements,
so it's not really something one should depend on.

That said, alas the only debug facility in PDO prints directly to
output. Version 1 of this RFC added something completely different and
v2 fixes it re-using what PDO already has.

If you ask me, what PDO offers sucks, but that's what we've got and
improving it is out of scope for this RFC. Anyone could create a new RFC
to add array output to debugDumpParams, that would be most welcome.


Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Benoit SCHILDKNECHT

9 years ago

Le Fri, 02 Dec 2016 14:54:04 +0100, Matteo Beccati <php@beccati.com> a
écrit:

> On 02/12/2016 08:36, Thomas Nunninger wrote:
>> Hi,
>>
>>> So, if I only want to get the emulated prepared statement, I have to do
>>> ob_start()/ob_get_clean(), then use a regexp to fetch it ? I want v1
>>> back T_T
>>
>> I second that. Why do you print it directly? Wouldn't it be better to
>> return a structured array with the information needed? If needed, you
>> can var_dump() the array yourself.
>
> Could you please explain what's the use case? As far as I'm concerned
> such information would only be useful during PDO driver development,
> phpt files and bug reporting / fixing.
>
>
> Cheers

It's pretty simple : debugging and optimizing scripts and statements. I've
lost hours, manually emulating tons of statements, or trying to guess
what's happening when I have something like 1000+ prepared values (mass
insert). And since I have to do it by hand, mistakes happen, so I lose
even more time debugging my emulations. Having a direct access to the
emulated statements, without having to dirtily parse a dump, will be a
huge plus for me, my team and the performances of my debug component.

Adam Baratz

9 years ago

>
> Could you please explain what's the use case? As far as I'm concerned
>> such information would only be useful during PDO driver development,
>> phpt files and bug reporting / fixing.
>>
>
> It's pretty simple : debugging and optimizing scripts and statements. I've
> lost hours, manually emulating tons of statements, or trying to guess
> what's happening when I have something like 1000+ prepared values (mass
> insert). And since I have to do it by hand, mistakes happen, so I lose even
> more time debugging my emulations. Having a direct access to the emulated
> statements, without having to dirtily parse a dump, will be a huge plus for
> me, my team and the performances of my debug component.

I understand the use case -- I submitted the "v1" RFC, after all -- but the
discussion for the last RFC seemed to stall at an "agree to disagree"
stage. My hope with "v2" is that it'll be favored more than the first one,
which basically squeezed past the required margin for acceptance.

That said, do you have thoughts on a third approach? It might be that we
should do what Matteo suggested earlier in this thread: make something more
robust for getting structured data out of statements. To be honest, the v2
RFC does exactly what I need. It'll let me write .phpt tests and do one-off
debugging.

But if your take is that simply that you prefer v1 over v2, the regular
vote should cover that.

Thanks,
Adam

Davey

9 years ago

This issue is not difficult to solve in userland, at least well enough for
debugging in my experience. Here's an older example I wrote up on my blog
that worked (but I'm told is broken right now):
https://daveyshafik.com/archives/605-debugging-pdo-prepared-statements.html

Thanks,

- Davey

On Mon, Dec 5, 2016 at 10:30 AM, Adam Baratz <adambaratz@php.net> wrote:

Adam Baratz

9 years ago

>
> This issue is not difficult to solve in userland, at least well enough for
> debugging in my experience. Here's an older example I wrote up on my blog
> that worked (but I'm told is broken right now): https://daveyshafik.com/
> archives/605-debugging-pdo-prepared-statements.html
>

I want to be able to write .phpt tests that validate how values are getting
interpolated into emulated prepares. Since this creates a separate code
path, it wouldn't let me validate what gets sent to the server.
PDO::quote() doesn't necessarily produce the same output as
pdo_parse_params().

Lester Caine

9 years ago

On 06/12/16 16:30, Adam Baratz wrote:

>> This issue is not difficult to solve in userland, at least well enough for
>> debugging in my experience. Here's an older example I wrote up on my blog
>> that worked (but I'm told is broken right now): https://daveyshafik.com/
>> archives/605-debugging-pdo-prepared-statements.html
>>
> I want to be able to write .phpt tests that validate how values are getting
> interpolated into emulated prepares. Since this creates a separate code
> path, it wouldn't let me validate what gets sent to the server.
> PDO::quote() doesn't necessarily produce the same output as
> pdo_parse_params().

The whole point of testing is that it tests real results not testing
'test code' that tries to emulate the real world. The whole point of
'emulated prepare statements' is that it creates a SQL query that the
target database engine can actually work with rather than one that a
more capable database engine will process. Testing the innards of some
driver is simply wrong, one tests the whole process is working! And
IDEALLY the test suit for PDO should work which ever target is selected,
but that is not lightly to happen any time soon.

--
Lester Caine - G8HFL
-----------------------------
Contact - http://lsces.co.uk/wiki/?page=contact
L.S.Caine Electronic Services - http://lsces.co.uk
EnquirySolve - http://enquirysolve.com/
Model Engineers Digital Workshop - http://medw.co.uk
Rainbow Digital Media - http://rainbowdigitalmedia.co.uk

Guilherme Blanco

9 years ago

Hi Adam,

Providing my input on why I voted "NO".
Your proposed new method is not pure. This means it does have distinct
responses depending on where it's located in the code.
That's a big no-no for me.

Cheers,

On Tue, Dec 6, 2016 at 12:15 PM, Lester Caine <lester@lsces.co.uk> wrote:

> On 06/12/16 16:30, Adam Baratz wrote:
> >> This issue is not difficult to solve in userland, at least well enough
> for
> >> debugging in my experience. Here's an older example I wrote up on my
> blog
> >> that worked (but I'm told is broken right now):
> https://daveyshafik.com/
> >> archives/605-debugging-pdo-prepared-statements.html
> >>
> > I want to be able to write .phpt tests that validate how values are
> getting
> > interpolated into emulated prepares. Since this creates a separate code
> > path, it wouldn't let me validate what gets sent to the server.
> > PDO::quote() doesn't necessarily produce the same output as
> > pdo_parse_params().
>
> The whole point of testing is that it tests real results not testing
> 'test code' that tries to emulate the real world. The whole point of
> 'emulated prepare statements' is that it creates a SQL query that the
> target database engine can actually work with rather than one that a
> more capable database engine will process. Testing the innards of some
> driver is simply wrong, one tests the whole process is working! And
> IDEALLY the test suit for PDO should work which ever target is selected,
> but that is not lightly to happen any time soon.
>
> --
> Lester Caine - G8HFL
> -----------------------------
> Contact - http://lsces.co.uk/wiki/?page=contact
> L.S.Caine Electronic Services - http://lsces.co.uk
> EnquirySolve - http://enquirysolve.com/
> Model Engineers Digital Workshop - http://medw.co.uk
> Rainbow Digital Media - http://rainbowdigitalmedia.co.uk
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

--
Guilherme Blanco
Senior Technical Architect at Huge Inc.