Обсуждение: Re: [COMMITTERS] pgsql: Implement current_query(), that shows the currently executing

Bruce Momjian wrote:
> Log Message:
> -----------
> Implement current_query(), that shows the currently executing query.
> At the same time remove dblink/dblink_current_query() as it is no longer
> necessary
> *BACKWARD COMPATIBILITY ISSUE* for dblink

I think the agreement was that dblink_current_query was to be
implemented on top of this.  In fact I don't see any reason not to.

Also, wasn't the name supposed to be client_query?

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

Alvaro Herrera wrote:
> Bruce Momjian wrote:
> > Log Message:
> > -----------
> > Implement current_query(), that shows the currently executing query.
> > At the same time remove dblink/dblink_current_query() as it is no longer
> > necessary
> > *BACKWARD COMPATIBILITY ISSUE* for dblink
> 
> I think the agreement was that dblink_current_query was to be
> implemented on top of this.  In fact I don't see any reason not to.

Really?  It seemed like just duplicate functionality.

> Also, wasn't the name supposed to be client_query?

Because pg_stat_activity already has current_query (and no one has
complained about it) there was discussion to just make it current_query.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> Alvaro Herrera wrote:

> > I think the agreement was that dblink_current_query was to be
> > implemented on top of this.  In fact I don't see any reason not to.
> 
> Really?  It seemed like just duplicate functionality.

It's called "backwards compatibility".  The nice thing about it is that
it doesn't cost us any extra code.

> > Also, wasn't the name supposed to be client_query?
> 
> Because pg_stat_activity already has current_query (and no one has
> complained about it) there was discussion to just make it current_query.

I don't think you can call that an agreement.  It was just a suggestion
IIRC.

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

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Bruce Momjian wrote:
>> Alvaro Herrera wrote:
>>> I think the agreement was that dblink_current_query was to be
>>> implemented on top of this.  In fact I don't see any reason not to.
>> 
>> Really?  It seemed like just duplicate functionality.

> It's called "backwards compatibility".  The nice thing about it is that
> it doesn't cost us any extra code.

Indeed.  It's just silly to break dblink users when there's no need.

>>> Also, wasn't the name supposed to be client_query?
>> 
>> Because pg_stat_activity already has current_query (and no one has
>> complained about it) there was discussion to just make it current_query.

> I don't think you can call that an agreement.  It was just a suggestion
> IIRC.

Well, there wasn't any strong consensus for client_query either ...
        regards, tom lane

Tom Lane wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
> > Bruce Momjian wrote:
> >> Alvaro Herrera wrote:
> >>> I think the agreement was that dblink_current_query was to be
> >>> implemented on top of this.  In fact I don't see any reason not to.
> >> 
> >> Really?  It seemed like just duplicate functionality.
> 
> > It's called "backwards compatibility".  The nice thing about it is that
> > it doesn't cost us any extra code.
> 
> Indeed.  It's just silly to break dblink users when there's no need.

OK.  Did someone mention this before because I don't remember it and the
patch removed the dblink usage.  Do we continue to document the
function?

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:

> > >> Alvaro Herrera wrote:
> > >>> I think the agreement was that dblink_current_query was to be
> > >>> implemented on top of this.  In fact I don't see any reason not to.

> OK.  Did someone mention this before because I don't remember it and the
> patch removed the dblink usage.  Do we continue to document the
> function?

Yes, I did:

http://archives.postgresql.org/pgsql-patches/2007-05/msg00098.php

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Alvaro Herrera wrote:
> Bruce Momjian wrote:
> 
> > > >> Alvaro Herrera wrote:
> > > >>> I think the agreement was that dblink_current_query was to be
> > > >>> implemented on top of this.  In fact I don't see any reason not to.
> 
> > OK.  Did someone mention this before because I don't remember it and the
> > patch removed the dblink usage.  Do we continue to document the
> > function?
> 
> Yes, I did:
> 
> http://archives.postgresql.org/pgsql-patches/2007-05/msg00098.php

I see what happened.  The author said he had made the change, but the
patch didn't contain it:

http://archives.postgresql.org/pgsql-patches/2007-05/msg00132.php
> >     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?

So do we document it or just add a function entry point?

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> I see what happened.  The author said he had made the change, but the
> patch didn't contain it:
>
> http://archives.postgresql.org/pgsql-patches/2007-05/msg00132.php
> > >     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?

I have re-added it as an undocumented dblink SQL function.

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

  + If your life is a hard drive, Christ can be your backup. +
Index: contrib/dblink/dblink.sql.in
===================================================================
RCS file: /cvsroot/pgsql/contrib/dblink/dblink.sql.in,v
retrieving revision 1.15
diff -c -c -r1.15 dblink.sql.in
*** contrib/dblink/dblink.sql.in    4 Apr 2008 16:57:21 -0000    1.15
--- contrib/dblink/dblink.sql.in    5 Apr 2008 02:23:39 -0000
***************
*** 163,168 ****
--- 163,173 ----
  AS 'MODULE_PATHNAME','dblink_build_sql_update'
  LANGUAGE C STRICT;

+ CREATE OR REPLACE FUNCTION dblink_current_query ()
+ RETURNS text
+ AS 'SELECT current_query()'
+ LANGUAGE SQL;
+
  CREATE OR REPLACE FUNCTION dblink_send_query(text, text)
  RETURNS int4
  AS 'MODULE_PATHNAME', 'dblink_send_query'
Index: contrib/dblink/uninstall_dblink.sql
===================================================================
RCS file: /cvsroot/pgsql/contrib/dblink/uninstall_dblink.sql,v
retrieving revision 1.6
diff -c -c -r1.6 uninstall_dblink.sql
*** contrib/dblink/uninstall_dblink.sql    4 Apr 2008 16:57:21 -0000    1.6
--- contrib/dblink/uninstall_dblink.sql    5 Apr 2008 02:23:39 -0000
***************
*** 3,8 ****
--- 3,10 ----
  -- Adjust this setting to control where the objects get dropped.
  SET search_path = public;

+ DROP FUNCTION dblink_current_query ();
+
  DROP FUNCTION dblink_build_sql_update (text, int2vector, int4, _text, _text);

  DROP FUNCTION dblink_build_sql_delete (text, int2vector, int4, _text);

Bruce Momjian <bruce@momjian.us> writes:
> + CREATE OR REPLACE FUNCTION dblink_current_query ()
> + RETURNS text
> + AS 'SELECT current_query()'
> + LANGUAGE SQL;

Needs to be pg_catalog.current_query
        regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > + CREATE OR REPLACE FUNCTION dblink_current_query ()
> > + RETURNS text
> > + AS 'SELECT current_query()'
> > + LANGUAGE SQL;
> 
> Needs to be pg_catalog.current_query

Oh, good point.  Done.

--  Bruce Momjian  <bruce@momjian.us>        http://momjian.us EnterpriseDB
http://enterprisedb.com
 + If your life is a hard drive, Christ can be your backup. +