Обсуждение: SQLExecDirectW returns SQL_SUCCESS even if sql finishes with error

Hi,


I have a problem in psqlODBC.

SQLExecDirectW() returns SQL_SUCCESS even if sql finishes with error when client_min_messages = fatal.
It works correctly when client_min_messages = error.

I think the cause is following.


connection.c
CC_send_query_append()

...
    while (self->pqconn && (pgres = PQgetResult(self->pqconn)) != NULL) // (1)
    {
        int status = PQresultStatus(pgres); // (2)

        if (discardTheRest)
            continue;
        switch (status) 
        {
            ...
            case PGRES_BAD_RESPONSE:
            case PGRES_FATAL_ERROR:
                handle_pgres_error(self, pgres, "send_query", res, TRUE); // (3)
        ...



When client_min_messages = error and sql finishes with error,
PostgreSQL server sends 'E' message to the client.
(https://www.postgresql.org/docs/11/static/protocol-message-formats.html)

In this case, PQgetResult() returns non-NULL in (1) and PQresultStatus() returns PGRES_FATAL_ERROR in (2).
Then, handle_pgres_error() is called in (3) and finally SQLExecDirectW() returns SQL_ERROR.


However, when client_min_messages = fatal and sql finishes with error,
PostgreSQL server sends only 'Z' message to the client.

In this case, PQgetResult() returns NULL in (1) and handle_pgres_error() is never called in (3).
Then, SQLExecDirectW() returns SQL_SUCCESS.


Is this right?



Regards,
Ryohei Takahashi

Hi Takahashi-san,

Thanks for the report.

On 2018/10/31 16:53, Takahashi, Ryohei wrote:
> Hi,
>
>
> I have a problem in psqlODBC.
>
> SQLExecDirectW() returns SQL_SUCCESS even if sql finishes with error when client_min_messages = fatal.
> It works correctly when client_min_messages = error.

Why do you set client_min_messages to fatal in the first place?

regards,
Hiroshi Inoue

>
> I think the cause is following.
>
>
> connection.c
> CC_send_query_append()
>
> ...
>     while (self->pqconn && (pgres = PQgetResult(self->pqconn)) != NULL) // (1)
>     {
>         int status = PQresultStatus(pgres); // (2)
>
>         if (discardTheRest)
>             continue;
>         switch (status)
>         {
>             ...
>             case PGRES_BAD_RESPONSE:
>             case PGRES_FATAL_ERROR:
>                 handle_pgres_error(self, pgres, "send_query", res, TRUE); // (3)
>         ...
>
>
>
> When client_min_messages = error and sql finishes with error,
> PostgreSQL server sends 'E' message to the client.
> (https://www.postgresql.org/docs/11/static/protocol-message-formats.html)
>
> In this case, PQgetResult() returns non-NULL in (1) and PQresultStatus() returns PGRES_FATAL_ERROR in (2).
> Then, handle_pgres_error() is called in (3) and finally SQLExecDirectW() returns SQL_ERROR.
>
>
> However, when client_min_messages = fatal and sql finishes with error,
> PostgreSQL server sends only 'Z' message to the client.
>
> In this case, PQgetResult() returns NULL in (1) and handle_pgres_error() is never called in (3).
> Then, SQLExecDirectW() returns SQL_SUCCESS.
>
>
> Is this right?
>
>
>
> Regards,
> Ryohei Takahashi

Hi Inoue-san,


Thank you for replying.


> Why do you set client_min_messages to fatal in the first place?

Actually, my customer used PostgreSQL with default settings at first.
He wanted to reduce server logs later and set log_min_messages = fatal and client_min_messages = fatal.

I know client_min_messages does not reduce server logs and recommended him to change client_min_messages again.


Anyway, I think it is a problem that SQLExecDirectW() returns SQL_SUCCESS even if sql finishes with error.


Regards,
Ryohei Takahashi

Takahashi, Ryohei wrote:
> Anyway, I think it is a problem that SQLExecDirectW() returns SQL_SUCCESS even if sql finishes with error.

When client_min_messages set that high, the server never reports the
error.  So the ODBC driver cannot do much about it; at best, it could
report something like "I do not know the status", but that is not
actually possible in the ODBC API.

> He wanted to reduce server logs later and set ... client_min_messages = fatal.

Setting "client_min_messages = fatal" literally means "I do not want to
know about normal errors."

This is an error in the documentation.
I've submitted a bug report: <https://www.postgresql.org/message-id/flat/15479-ef0f4cc2fd995ca2%40postgresql.org>


Regards,
Clemens

Hi Takahashi-san,

Could you please tell the customer that it's meaningless to set  
client_min_messages to 'fatal' or 'panic'?

regards,
Hiroshi Inoue

On 2018/11/01 15:57, Takahashi, Ryohei wrote:
> Hi Inoue-san,
>
>
> Thank you for replying.
>
>
>> Why do you set client_min_messages to fatal in the first place?
> Actually, my customer used PostgreSQL with default settings at first.
> He wanted to reduce server logs later and set log_min_messages = fatal and client_min_messages = fatal.
>
> I know client_min_messages does not reduce server logs and recommended him to change client_min_messages again.
>
>
> Anyway, I think it is a problem that SQLExecDirectW() returns SQL_SUCCESS even if sql finishes with error.
>
>
> Regards,
> Ryohei Takahashi

"Inoue, Hiroshi" <h-inoue@dream.email.ne.jp> writes:
> Could you please tell the customer that it's meaningless to set  
> client_min_messages to 'fatal' or 'panic'?

Yeah, I wonder why we allow that at all.  It's basically breaking
the wire protocol ...

            regards, tom lane

Hi Clemens,

Thank you for replying.

> This is an error in the documentation.
> I've submitted a bug report:

I understand.
Thank you for submitting the bug report.


Regards,
Ryohei Takahashi

Hi Inoue-san, Tom and Jonah,

Thank you for replying.

I understand the problem is that PostgreSQL allows users to set client_min_messages = fatal or panic.
I told my customer this problem and that it's meaningless to set client_min_messages to 'fatal' to reduce server logs.


Regards,
Ryohei Takahashi

"Jonah H. Harris" <jonah.harris@gmail.com> writes:
> On Thu, Nov 1, 2018 at 10:50 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Yeah, I wonder why we allow that at all.  It's basically breaking
>> the wire protocol ...

> Agreed. I'll send in a patch if you want.

I started a thread about this on -hackers,

https://www.postgresql.org/message-id/7809.1541521180@sss.pgh.pa.us

If you have or want to start on a patch, please do so and follow up
there.

            regards, tom lane