Обсуждение: Proposed patch for showing originating query on error

This just adds an exception to the existing backend exception showing
the originating query.

Given that the backend doesn't show us this information, a few people
have asked for it.

Dave

*** 426,437 ****
--- 432,451 ----

           StatementResultHandler handler = new StatementResultHandler
();
           result = null;
+         try
+         {
               connection.getQueryExecutor().execute(queryToExecute,
                                                 queryParameters,
                                                 handler,
                                                 maxrows,
                                                 fetchSize,
                                                 flags);
+         }
+         catch( SQLException ex )
+         {
+             handler.handleError(new PSQLException(GT.tr("Original
Query -->") + queryToExecute.toString(queryParameters),
PSQLState.UNKNOWN_ST
ATE));
+             throw ex;
+         }
           result = firstUnclosedResult = handler.getResults();
       }

Dave Cramer wrote:

> *** 426,437 ****
> --- 432,451 ----

Uh, what file is this a patch to?

-O

AbstractJdbc2Statement.java

On 28-Jul-05, at 10:22 AM, Oliver Jowett wrote:

> Dave Cramer wrote:
>
>
>> *** 426,437 ****
>> --- 432,451 ----
>>
>
> Uh, what file is this a patch to?
>
> -O
>
>
>

Dave Cramer wrote:
> This just adds an exception to the existing backend exception showing
> the originating query.

I think the basic idea is ok, but..

- this should be on the original exception, not on a chained synthetic
exception
- doing it only in execute() seems incomplete, there are other query
execution paths, perhaps this logic belongs somewhere in the query executor?
- ideally it should be configurable (hook it into whatever came out of
Kris' suggestions about including more info in server-generated
exception, perhaps?)

-O

On 28-Jul-05, at 10:46 AM, Oliver Jowett wrote:

> Dave Cramer wrote:
>
>> This just adds an exception to the existing backend exception showing
>> the originating query.
>>
>
> I think the basic idea is ok, but..
>
> - this should be on the original exception, not on a chained synthetic
> exception
Yeah, that would be cleaner.
> - doing it only in execute() seems incomplete, there are other query
> execution paths, perhaps this logic belongs somewhere in the query
> executor?

There are even more paths there to hook into. Putting it in
AbstractJdbc2Statement minimizes
the number of places it would need to be added to. QueryExecutor has
a few Implementations ...

> - ideally it should be configurable (hook it into whatever came out of
> Kris' suggestions about including more info in server-generated
> exception, perhaps?)
Well, lets get it working first before we make it configurable.
>
> -O
>
>

Dave Cramer wrote:

> There are even more paths there to hook into. Putting it in
> AbstractJdbc2Statement minimizes
> the number of places it would need to be added to. QueryExecutor has  a
> few Implementations ...

Well, do you have a better suggestion that catches all paths?

>> - ideally it should be configurable (hook it into whatever came out of
>> Kris' suggestions about including more info in server-generated
>> exception, perhaps?)
>
> Well, lets get it working first before we make it configurable.

sigh :( let's get it right before we commit it at all.. in practise it
seems "we'll fix this later" means "we won't fix it"

-O

On 28-Jul-05, at 11:06 AM, Oliver Jowett wrote:

> Dave Cramer wrote:
>
>
>> There are even more paths there to hook into. Putting it in
>> AbstractJdbc2Statement minimizes
>> the number of places it would need to be added to. QueryExecutor
>> has  a
>> few Implementations ...
>>
>
> Well, do you have a better suggestion that catches all paths?
I'll give it some serious attention later, I hacked it in to your
code for someone, just wanted to get it into the discussion.
>
>
>>> - ideally it should be configurable (hook it into whatever came
>>> out of
>>> Kris' suggestions about including more info in server-generated
>>> exception, perhaps?)
>>>
>>
>> Well, lets get it working first before we make it configurable.
>>
>
> sigh :( let's get it right before we commit it at all.. in practise it
> seems "we'll fix this later" means "we won't fix it"
Agreed.
>
> -O
>
>