Thread: Proposed patch for showing originating query on error

Proposed patch for showing originating query on error

From
Dave Cramer
Date:
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();
       }



Re: Proposed patch for showing originating query on error

From
Oliver Jowett
Date:
Dave Cramer wrote:

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

Uh, what file is this a patch to?

-O


Re: Proposed patch for showing originating query on error

From
Dave Cramer
Date:
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
>
>
>


Re: Proposed patch for showing originating query on error

From
Oliver Jowett
Date:
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

Re: Proposed patch for showing originating query on error

From
Dave Cramer
Date:
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
>
>


Re: Proposed patch for showing originating query on error

From
Oliver Jowett
Date:
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

Re: Proposed patch for showing originating query on error

From
Dave Cramer
Date:
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
>
>