Re: [pgadmin-support] Possible simple enhancement

On Wed, Sep 16, 2009 at 07:09, Guillaume Lelarge <guillaume(at)lelarge(dot)info> wrote:
> Le mardi 15 septembre 2009 à 18:47:24, Guillaume Lelarge a écrit :
>> Le mardi 15 septembre 2009 à 09:57:55, Magnus Hagander a écrit :
>> > On Tue, Sep 15, 2009 at 07:34, Guillaume Lelarge <guillaume(at)lelarge(dot)info>
>>
>> wrote:
>> > > Le mardi 15 septembre 2009 à 06:54:59, Magnus Hagander a écrit :
>> > >> On 15 sep 2009, at 00.48, Guillaume Lelarge <guillaume(at)lelarge(dot)info>
>> > >>
>> > >> wrote:
>> > >> > Le samedi 12 septembre 2009 à 14:49:04, Guillaume Lelarge a écrit :
>> > >> >> Le mercredi 9 septembre 2009 à 20:46:00, Roger Niederland a écrit :
>> > >> >>> First of all thanks for PGAdmin I've been using it for years now.
>> > >> >>>
>> > >> >> :)
>> > >> >>>
>> > >> >>> One small enhancement that would be very useful is:
>> > >> >>>
>> > >> >>> When viewing the Statistics tab for the server where the Current
>> > >> >>> Executing Queries is available (along with the current user,
>> > >> >>> database
>> > >> >>> etc..) It would be good to be able to copy the sql of a current
>> > >> >>> query.
>> > >> >>> I realize that it is impractical to show the full query sql (and
>> > >> >>> do not
>> > >> >>> want this).  But I would like to be able to copy the "full sql" of
>> > >> >>> the
>> > >> >>> current query.  I know that I can log long running queries, but
>> > >> >>> feel it
>> > >> >>> would be useful to copy the query from the statistics, so I can
>> > >> >>> paste it
>> > >> >>> into a query window for further analysis.
>> > >> >>
>> > >> >> It's already possible in 1.10, but it copies the whole line (not
>> > >> >> only the
>> > >> >> query). Anyways, that's one item on my todolist. We also want to be
>> > >> >> able to
>> > >> >> launch the query tool with this query (see
>> > >> >> http://code.pgadmin.org/trac/ticket/9). I hope all of these will be
>> > >> >> in
>> > >> >> 1.12.
>> > >> >
>> > >> > Here is a patch to lauch the query tool with the selected query. The
>> > >> > icon is
>> > >> > the copy one. Not great, but I don't really have design skills. I
>> > >> > wonder if we
>> > >> > can make the query tool icon smaller enough, so that it will fit in
>> > >> > the
>> > >> > toolbar. Anyways, with this patch, when you click on the toolbar
>> > >> > button, the
>> > >> > query tool opens a new connection on the selected database of the
>> > >> > same server,
>> > >> > displays the query and allows the user to modify/execute it.
>> > >> >
>> > >> > Comments?
>> > >>
>> > >> For long queries the whole string won't be available. Maybe we should
>> > >> try to detect that and issue an error?
>> > >
>> > > Yeah, sure. This new version checks if the query size is at max size
>> > > and displays a message if it is. The contents of the message probably
>> > > need some work.
>> >
>> > Should we bother copying at all if it's short?
>>
>> Don't understand this one ?!?!
>>
>> > If we keep that, how about:
>> > "The query is longer than the maximum length, and has been truncated. "
>>
>> Well, we don't really know if it has been truncated. All we know is that
>>  the query is at the maximum length.
>>
>
> New version of the patch:
>
>  * Previously, only the first 250 characters of the query were displayed.
>  * We won't launch the query tool if the selected process is in <IDLE> or
>   <IDLE in transaction> state.

Quick comment:
The logic around getting the max length is wrong. If the query to get
it fails, the value of maxlength will be unspecified (the else
statement only comes in effect if the version is <8.4). How about just
initializing it to 1024 at the start of the function? Also, the
"delete set" should be inside the check for NULL value - with a NULL
returned you'll attempt to delete NULL.

>> Should we bother copying at all if it's short?
>
> Don't understand this one ?!?!

I mean if it's cut off, should we actually start a query tool with it,
or should we just say "hey, this has been truncated" and *not* start
the query tool.

>> If we keep that, how about:
>> "The query is longer than the maximum length, and has been truncated. "
>
> Well, we don't really know if it has been truncated. All we know is that the
> query is at the maximum length.

We don't, but it's pretty likely. So change it to "it may have been
truncated"? :-)

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/