As suggested in the TODO list (and as I need the functionality
myself), I have implemented the current_query interface to
debug_query_string.

I'm not sure the best place to put this, suggestions welcome..

Please review the patch attached.

Cheers
Tom



---------------------------(end of broadcast)---------------------------
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match

On Mon, 2007-07-05 at 19:48 +0100, Tomas Doran wrote:
> As suggested in the TODO list (and as I need the functionality
> myself), I have implemented the current_query interface to
> debug_query_string.

Comments:

* docs need a bit more detail (they should emphasize that it is the
current query string submitted by the client, as opposed to the
currently executing SPI command or the like). Also, the docs currently
claim current_query() returns "name".

* use textin() to convert C-style strings to text, rather than
constructing a text datum by hand

* perhaps we can get away with marking current_query() stable?

* AFAIK debug_query_string() still does the wrong thing when the user
submits multiple queries in a single protocol message (separated by
semi-colons). Not sure there's a way to fix that that is both easy and
efficient, though...

-Neil



---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

On 7 May 2007, at 23:25, Neil Conway wrote:

> On Mon, 2007-07-05 at 19:48 +0100, Tomas Doran wrote:
>> As suggested in the TODO list (and as I need the functionality
>> myself), I have implemented the current_query interface to
>> debug_query_string.
>
> * docs need a bit more detail (they should emphasize that it is the

Detail added. I'm none too happy with the phrasing, anyone suggest
better?

> * use textin() to convert C-style strings to text, rather than
> constructing a text datum by hand

Done.

> * perhaps we can get away with marking current_query() stable?

Also done, note OID has changed as I was having conflicts (template1
wouldn't build). Should I either pick something else unused which is
lower (is there anything?), move current_query to the end of the file
or just leave it be..

> * AFAIK debug_query_string() still does the wrong thing when the user
> submits multiple queries in a single protocol message (separated by
> semi-colons). Not sure there's a way to fix that that is both easy and
> efficient, though...

Should that be added to the TODO list?

Cheers
Tom



---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster

Tomas Doran wrote:
>
> On 7 May 2007, at 23:25, Neil Conway wrote:
>
> >On Mon, 2007-07-05 at 19:48 +0100, Tomas Doran wrote:
> >>As suggested in the TODO list (and as I need the functionality
> >>myself), I have implemented the current_query interface to
> >>debug_query_string.

FWIW I think you should still provide dblink_current_query, even if it's
only a wrapper over current_query(), for backwards compatibility.

Also, typically we don't remove items from the TODO list. We mark them
as "done" prepending them with a dash. Patch authors are not expected
to do it either (though I don't see it be a problem if they did).


> Also done, note OID has changed as I was having conflicts (template1
> wouldn't build). Should I either pick something else unused which is
> lower (is there anything?), move current_query to the end of the file
> or just leave it be..

Doesn't matter ... just make sure duplicate_oids doesn't report a
problem. unused_oids is useful to find, err, unused OIDs.

> >* AFAIK debug_query_string() still does the wrong thing when the user
> >submits multiple queries in a single protocol message (separated by
> >semi-colons). Not sure there's a way to fix that that is both easy and
> >efficient, though...
>
> Should that be added to the TODO list?

Probably ...

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

http://archives.postgresql.org

On 10 May 2007, at 03:09, Alvaro Herrera wrote:
> FWIW I think you should still provide dblink_current_query, even if
> it's
> only a wrapper over current_query(), for backwards compatibility.

Good point. Done as suggested (I think, or did you mean also the
change of instances to use current_query()?). Replaced
dblink_current_query with an SQL procedure wrapper, I assume that's
the most efficient way of doing it?

> Also, typically we don't remove items from the TODO list. We mark
> them
> as "done" prepending them with a dash. Patch authors are not expected
> to do it either (though I don't see it be a problem if they did).

Not quite sure what you're suggesting (which way round), so I just
didn't do it (as you said I'm not expected to).

> Doesn't matter ... just make sure duplicate_oids doesn't report a
> problem. unused_oids is useful to find, err, unused OIDs.

Ahh, hadn't found those, thanks. They're in the dev FAQ too, *blush*.
I need this for something I'm doing at $ork, and thought I'd
implement it in the backend, as well as a .so, it's been a learning
experience

>>> * AFAIK debug_query_string() still does the wrong thing when the
>>> user
>>
>> Should that be added to the TODO list?
>
> Probably ...

Done!

Cheers
Tom



---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to majordomo@postgresql.org so that your
message can get through to the mailing list cleanly

This has been saved for the 8.4 release:

http://momjian.postgresql.org/cgi-bin/pgpatches_hold

---------------------------------------------------------------------------

Tomas Doran wrote:
>
> On 10 May 2007, at 03:09, Alvaro Herrera wrote:
> > FWIW I think you should still provide dblink_current_query, even if
> > it's
> > only a wrapper over current_query(), for backwards compatibility.
>
> Good point. Done as suggested (I think, or did you mean also the
> change of instances to use current_query()?). Replaced
> dblink_current_query with an SQL procedure wrapper, I assume that's
> the most efficient way of doing it?
>
> > Also, typically we don't remove items from the TODO list. We mark
> > them
> > as "done" prepending them with a dash. Patch authors are not expected
> > to do it either (though I don't see it be a problem if they did).
>
> Not quite sure what you're suggesting (which way round), so I just
> didn't do it (as you said I'm not expected to).
>
> > Doesn't matter ... just make sure duplicate_oids doesn't report a
> > problem. unused_oids is useful to find, err, unused OIDs.
>
> Ahh, hadn't found those, thanks. They're in the dev FAQ too, *blush*.
> I need this for something I'm doing at $ork, and thought I'd
> implement it in the backend, as well as a .so, it's been a learning
> experience
>
> >>> * AFAIK debug_query_string() still does the wrong thing when the
> >>> user
> >>
> >> Should that be added to the TODO list?
> >
> > Probably ...
>
> Done!
>
> Cheers
> Tom
>

[ Attachment, skipping... ]

>
> ---------------------------(end of broadcast)---------------------------
> TIP 1: if posting/reading through Usenet, please send an appropriate
> subscribe-nomail command to majordomo@postgresql.org so that your
> message can get through to the mailing list cleanly

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +


---------------------------(end of broadcast)---------------------------
TIP 3: Have you checked our extensive FAQ?

http://www.postgresql.org/docs/faq

I have applied a modified version of this patch, attached. I made a few
changes:

o You had current_query() returning 'void' so it didn't work
o I removed the dblink regression tests for current_query() as
it is now a backend function
o Update documentation to mention the possibility of multiple
statements
o Used the new cstring_to_text() usage that Tom had updated in
CVS for this function
o The pg_proc.h oids and number of columns didn't match CVS

Thanks for the patch.

---------------------------------------------------------------------------

Tomas Doran wrote:
>
> On 10 May 2007, at 03:09, Alvaro Herrera wrote:
> > FWIW I think you should still provide dblink_current_query, even if
> > it's
> > only a wrapper over current_query(), for backwards compatibility.
>
> Good point. Done as suggested (I think, or did you mean also the
> change of instances to use current_query()?). Replaced
> dblink_current_query with an SQL procedure wrapper, I assume that's
> the most efficient way of doing it?
>
> > Also, typically we don't remove items from the TODO list. We mark
> > them
> > as "done" prepending them with a dash. Patch authors are not expected
> > to do it either (though I don't see it be a problem if they did).
>
> Not quite sure what you're suggesting (which way round), so I just
> didn't do it (as you said I'm not expected to).
>
> > Doesn't matter ... just make sure duplicate_oids doesn't report a
> > problem. unused_oids is useful to find, err, unused OIDs.
>
> Ahh, hadn't found those, thanks. They're in the dev FAQ too, *blush*.
> I need this for something I'm doing at $ork, and thought I'd
> implement it in the backend, as well as a .so, it's been a learning
> experience

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +


--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches