Everything

php

pg_execute error

php.internals

Yasuo Ohgaki

20 years ago

Hi.

Current pgsql.c supports prepared statement, but pg_execute() will raise
E_WARNING if query plan is not prepared yet. This is annoying when
database connection is persistent under web environment.

I would like to do something like

if (!pg_execute($db, 'myquery', array()) {
pg_prepare($db, 'myquery', 'SQL');
pg_execute($db, 'myquery', array();
}

WITHOUT raising any error, since @ operator does not work well with
error handler. (i.e. need special handling for pg_execute)

There are 4 options:

1) just ignore errors on pg_execute(). errors can be identified by return status anyway.
2) add pg_is_prepared()
3) add bool parameter to pg_execute()
e.g. pg_execute(resource connection, string stmtname, array params, bool ignore_error)
4) ignore pg_execute() error if params is null array.

Any comments?

--
Yasuo Ohgaki

Lukas Smith

20 years ago

Yasuo Ohgaki wrote:

> 3) add bool parameter to pg_execute()
>    e.g. pg_execute(resource connection, string stmtname, array params, bool ignore_error)

how would you intent to implement this?
AFAIK there is currently no catalog to find out the prepared statements
in the current sessions (I hear 8.2 will change this). So the only way
to find out is to simply "try it" and cause an error in the session
which could trigger all sorts of error handlers on the database side ..
something I would not expect from calling a php function like this.

regards,
Lukas

Yasuo Ohgaki

20 years ago

Lukas Smith wrote:

> Yasuo Ohgaki wrote:
>
>> 3) add bool parameter to pg_execute()
>>    e.g. pg_execute(resource connection, string stmtname, array params, bool ignore_error)
>
> how would you intent to implement this?
> AFAIK there is currently no catalog to find out the prepared statements
> in the current sessions (I hear 8.2 will change this). So the only way
> to find out is to simply "try it" and cause an error in the session
> which could trigger all sorts of error handlers on the database side ..
> something I would not expect from calling a php function like this.

I hear about that.

Current pg_execute raise E_WARNING if plan is not prepared.
The error is annoying since programmers has to try and
see if it works.

Anyway, since there are no other comments, I'll get rid of
the error from pg_execute. Since this is the most efficient
way.

--
Yasuo Ohgaki

Lukas Smith

20 years ago

Yasuo Ohgaki wrote:

> Lukas Smith wrote:
>> Yasuo Ohgaki wrote:
>>
>>> 3) add bool parameter to pg_execute()
>>>    e.g. pg_execute(resource connection, string stmtname, array params, bool ignore_error)
>> how would you intent to implement this?
>> AFAIK there is currently no catalog to find out the prepared statements
>> in the current sessions (I hear 8.2 will change this). So the only way
>> to find out is to simply "try it" and cause an error in the session
>> which could trigger all sorts of error handlers on the database side ..
>> something I would not expect from calling a php function like this.
>
> I hear about that.
>
> Current pg_execute raise E_WARNING if plan is not prepared.
> The error is annoying since programmers has to try and
> see if it works.
>
> Anyway, since there are no other comments, I'll get rid of
> the error from pg_execute. Since this is the most efficient
> way.

Just to make it clear: calling pg_execute() on a not yet prepared
statement will cause your transaction to be rolled  back on the next
commit. Encouraging the use of pg_execute() to find out of the statement
has been prepared is therefore wrong.

Moreover by your logic we would need to remove E_WARNING's from php
entirely. Finally you can detect if a function call was called with
error suppression in your error handler and that is the appropriate
place to address your issue.

regards,
Lukas

Yasuo Ohgaki

20 years ago

Lukas Smith wrote:

> Just to make it clear: calling pg_execute() on a not yet prepared
> statement will cause your transaction to be rolled  back on the next
> commit. Encouraging the use of pg_execute() to find out of the statement
> has been prepared is therefore wrong.
>
> Moreover by your logic we would need to remove E_WARNING's from php
> entirely. Finally you can detect if a function call was called with
> error suppression in your error handler and that is the appropriate
> place to address your issue.

This issue is like file_exists() raises E_ERROR, since
PostgreSQL does not provide means to check if plan is
already defined. i.e. design error for the function.

Even if PostgreSQL provide view for already defined plans,
selecting the view to check requires. It requires overheads
which requires network traffic. Thus adding new feature to
check if plan is defined is not good idea.
i.e. DB is usually a bottle neck of web systems.

--
Yasuo Ohgaki

Lukas Smith

20 years ago

Yasuo Ohgaki wrote:

> Lukas Smith wrote:
>> Just to make it clear: calling pg_execute() on a not yet prepared
>> statement will cause your transaction to be rolled  back on the next
>> commit. Encouraging the use of pg_execute() to find out of the statement
>> has been prepared is therefore wrong.
>>
>> Moreover by your logic we would need to remove E_WARNING's from php
>> entirely. Finally you can detect if a function call was called with
>> error suppression in your error handler and that is the appropriate
>> place to address your issue.
>
> This issue is like file_exists() raises E_ERROR, since
> PostgreSQL does not provide means to check if plan is
> already defined. i.e. design error for the function.
>
> Even if PostgreSQL provide view for already defined plans,
> selecting the view to check requires. It requires overheads
> which requires network traffic. Thus adding new feature to
> check if plan is defined is not good idea.
> i.e. DB is usually a bottle neck of web systems.

So why dont you just do @pg_execute() and in your error handler you can do:

function ErrorHandler($errno, $errstr, $errfile, $errline)
{
// ignore silenced function calls
if (!error_reporting()) {
return;
}
..

If you want a "beautiful" solution you will have to manage your prepared
statements in some persistant layer.

regards,
Lukas

Yasuo Ohgaki

20 years ago

Lukas Smith wrote:

> So why dont you just do @pg_execute() and in your error handler you can do:
>
> function ErrorHandler($errno, $errstr, $errfile, $errline)
> {
>  // ignore silenced function calls
>  if (!error_reporting()) {
>     return;
>   }
>  ..
>
> If you want a "beautiful" solution you will have to manage your prepared
> statements in some persistant layer.

This is one option.

@ operator is ok, but usually @ operator is not recommenned.
Don't you think so? I try not to use @ as much as possible.

pg_execute() does not have to raise error just like file_exists().
It may be good idea to raise error when connection is bad, etc.

--
Yasuo Ohgaki

Lukas Smith

20 years ago

Yasuo Ohgaki wrote:

> @ operator is ok, but usually @ operator is not recommenned.
> Don't you think so? I try not to use @ as much as possible.
>
> pg_execute() does not have to raise error just like file_exists().
> It may be good idea to raise error when connection is bad, etc.

the problem is a limitation in pgsql and hiding this fact imho causes
more problems than it solves, especially since pgsql will eventually
solve this limitation (i hear 8.2 will solve this).

another solution that does of course add a fixed amount of overhead is
to simply prepare the statement again before you enter the loop. you
will then only have to supress a single call to pg_prepare().

regards,
Lukas

Yasuo Ohgaki

20 years ago

Lukas Smith wrote:

> Yasuo Ohgaki wrote:
>
>> @ operator is ok, but usually @ operator is not recommenned.
>> Don't you think so? I try not to use @ as much as possible.
>>
>> pg_execute() does not have to raise error just like file_exists().
>> It may be good idea to raise error when connection is bad, etc.
>
> the problem is a limitation in pgsql and hiding this fact imho causes
> more problems than it solves, especially since pgsql will eventually
> solve this limitation (i hear 8.2 will solve this).

I heard postgresql people intends

just execute PQexecute, if it fails, prepare

since this is most natural way to handle persistent database connections.
8.2 backends will have view for currently registered plans. Since backends
have query plans, clients must requests send over net work just to see if
plan is defined or not.

i.e.
if (!pg_execute('plan')) {
pg_prepare('plan', sql);
pg_execute('plan');
}

Looking up plan data in backends is just a waste of resources even when 8.2
is available.

>
> another solution that does of course add a fixed amount of overhead is
> to simply prepare the statement again before you enter the loop. you
> will then only have to supress a single call to pg_prepare().

pg_prepare() also raises E_WARNING when plan is already defined, thus
calling pg_prepare() is not an option. PQprepare is not light function also.
It has to initialize SQL parser etc, while PQexecute just looks up plan
names when it fails.

When app uses PHP's persistent connections (or pgpooled connections), programmers
can take advantages of prepared statements performance benefits.
i.e. PostgreSQL have to compile SQL only once for a web server thread /process.

I don't see any down sides removing pg_execute E_WARNING for duplicate plans.

I will restore pg_execute E_WARING except already defined plans.
Is this satisfactory?

--
Yasuo Ohgaki

Wez Furlong

20 years ago

Regardless of whether it's a good idea or not, you should not just go
ahead and commit such a big behaviour change to the stable branch
during the release process.

Please revert your commit.

--Wez.

On 3/27/06, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:

Yasuo Ohgaki

20 years ago

Wez Furlong wrote:

> Regardless of whether it's a good idea or not, you should not just go
> ahead and commit such a big behaviour change to the stable branch
> during the release process.
>
> Please revert your commit.

That's good point.
Any people should not depend on pg_execute's E_WARNING error
in transaction, but I'll revert patch against PHP_5_1.

--
Yasuo Ohgaki

Lukas Smith

20 years ago

Yasuo Ohgaki wrote:

> Wez Furlong wrote:
>> Regardless of whether it's a good idea or not, you should not just go
>> ahead and commit such a big behaviour change to the stable branch
>> during the release process.
>>
>> Please revert your commit.
>
> That's good point.
> Any people should not depend on pg_execute's E_WARNING error
> in transaction, but I'll revert patch against PHP_5_1.

Sorry but your comparison to file_exists() is wrong. pg_execute()
compares to something like include() if at all. Again I do want
something like pg_is_prepared() but I rather wait until pgsql 8.2 where
we get a proper native solution.

For your case you should simply do:

// this will trigger an error if the query plan is already prepared
// but we can ignore the error
@pg_prepare($dbconn, 'myplan', $sql);
for ($i = 0; $i < 10000; $i++) {
pg_execute($dbconn, 'myplan', $values);
}

along with the necessary changes to your error handler you are all set.
you only have a single error silenced call.

so this change should be reverted in all branches ..

regards,
Lukas

Yasuo Ohgaki

20 years ago

Yasuo Ohgaki wrote:

> Lukas Smith wrote:
>> Just to make it clear: calling pg_execute() on a not yet prepared
>> statement will cause your transaction to be rolled  back on the next
>> commit. Encouraging the use of pg_execute() to find out of the statement
>> has been prepared is therefore wrong.
>>
>> Moreover by your logic we would need to remove E_WARNING's from php
>> entirely. Finally you can detect if a function call was called with
>> error suppression in your error handler and that is the appropriate
>> place to address your issue.
>
> This issue is like file_exists() raises E_ERROR, since
> PostgreSQL does not provide means to check if plan is
> already defined. i.e. design error for the function.
>
> Even if PostgreSQL provide view for already defined plans,
> selecting the view to check requires. It requires overheads
> which requires network traffic. Thus adding new feature to
> check if plan is defined is not good idea.
> i.e. DB is usually a bottle neck of web systems.
>

To make this issue clear, consider following code

for ($i = 0; $i < 10000; $i++) {
if (!pg_is_prepared('myplan')) { // suppose this function is there
pg_prepare('myplan', 'sql');
}
pg_execute('myplan', $values);
}

w/o the patch, pg_is_prepared() has to be called 10k times which
probably requires additional network traffic.

Best practice tells users should prepare statement before get into
transaction block and check transaction is committed or aborted
anyway. There are other reasons that transaction may fail as you
probably knew.

I disabled all E_WARNING, since all users are encouraged to check
the return status of pg_execute(), but other error may be caught
by pg_execute(). Is this enough?

--
Yasuo Ohgaki : yohgaki@ohgaki.net : http://www.ohgaki.net/

Wez Furlong

20 years ago

This sounds like a really bad, backwards compatibility-breaking idea.

--Wez.

On 3/26/06, Yasuo Ohgaki <yohgaki@ohgaki.net> wrote:

Edin Kadribasic

20 years ago

Agred, the patch should be reverted.

Edin

Wez Furlong wrote: