Обсуждение: pg_replication_origin_xact_reset() and its argument variables
Hi,
The document explains that pg_replication_origin_xact_reset() doesn't have
any argument variables. But it's been actually defined so as to have two
argument variables with pg_lsn and timestamptz data types, as follows.
=# \df pg_replication_origin_xact_reset List of functions Schema |
Name | Result data type | Argument data types | Type
------------+----------------------------------+------------------+----------------------------------+--------pg_catalog
|pg_replication_origin_xact_reset | void |
pg_lsn, timestamp with time zone | normal
As far as I read the code of the function, those arguments don't seem to
be necessary. So I'm afraid that the pg_proc entry for the function might
be incorrect and those two arguments should be removed from the definition.
Is this analysis right?
Regards,
--
Fujii Masao
Fujii Masao <masao.fujii@gmail.com> writes:
> The document explains that pg_replication_origin_xact_reset() doesn't have
> any argument variables. But it's been actually defined so as to have two
> argument variables with pg_lsn and timestamptz data types, as follows.
> =# \df pg_replication_origin_xact_reset
> List of functions
> Schema | Name | Result data type |
> Argument data types | Type
> ------------+----------------------------------+------------------+----------------------------------+--------
> pg_catalog | pg_replication_origin_xact_reset | void |
> pg_lsn, timestamp with time zone | normal
> As far as I read the code of the function, those arguments don't seem to
> be necessary. So I'm afraid that the pg_proc entry for the function might
> be incorrect and those two arguments should be removed from the definition.
> Is this analysis right?
Sure looks that way from here. Copy-and-paste from the previous
line in pg_proc.h, perhaps?
regards, tom lane
On 2016-06-30 10:14:04 -0400, Tom Lane wrote: > Fujii Masao <masao.fujii@gmail.com> writes: > > The document explains that pg_replication_origin_xact_reset() doesn't have > > any argument variables. But it's been actually defined so as to have two > > argument variables with pg_lsn and timestamptz data types, as follows. > > > =# \df pg_replication_origin_xact_reset > > List of functions > > Schema | Name | Result data type | > > Argument data types | Type > > ------------+----------------------------------+------------------+----------------------------------+-------- > > pg_catalog | pg_replication_origin_xact_reset | void | > > pg_lsn, timestamp with time zone | normal > > > As far as I read the code of the function, those arguments don't seem to > > be necessary. So I'm afraid that the pg_proc entry for the function might > > be incorrect and those two arguments should be removed from the definition. > > Is this analysis right? > > Sure looks that way from here. Copy-and-paste from the previous > line in pg_proc.h, perhaps? Yes, that's clearly wrong. Damn. Can't fix that for 9.5 anymore. The function isn't all that important (especially not from SQL), but still, that's annoying. I'm inclined to just remove the args in 9.6. We could also add a note to the 9.5 docs, adding that the arguments are there by error? Andres
Andres Freund <andres@anarazel.de> writes:
> On 2016-06-30 10:14:04 -0400, Tom Lane wrote:
>> Fujii Masao <masao.fujii@gmail.com> writes:
>>> As far as I read the code of the function, those arguments don't seem to
>>> be necessary. So I'm afraid that the pg_proc entry for the function might
>>> be incorrect and those two arguments should be removed from the definition.
>> Sure looks that way from here. Copy-and-paste from the previous
>> line in pg_proc.h, perhaps?
> Yes, that's clearly wrong. Damn. Can't fix that for 9.5 anymore. The
> function isn't all that important (especially not from SQL), but still,
> that's annoying. I'm inclined to just remove the args in 9.6. We could
> also add a note to the 9.5 docs, adding that the arguments are there by
> error?
Yeah, seems like the best thing to do.
regards, tom lane
On Sat, Jul 2, 2016 at 7:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Andres Freund <andres@anarazel.de> writes: >> On 2016-06-30 10:14:04 -0400, Tom Lane wrote: >>> Fujii Masao <masao.fujii@gmail.com> writes: >>>> As far as I read the code of the function, those arguments don't seem to >>>> be necessary. So I'm afraid that the pg_proc entry for the function might >>>> be incorrect and those two arguments should be removed from the definition. > >>> Sure looks that way from here. Copy-and-paste from the previous >>> line in pg_proc.h, perhaps? > >> Yes, that's clearly wrong. Attached patch (pg_replication_origin_xact_reset_9.6.patch) fixes this. We need to apply this at least before RC1 of PostgreSQL9.6 will be released because the patch needs the change of catalog version. >> Damn. Can't fix that for 9.5 anymore. The >> function isn't all that important (especially not from SQL), but still, >> that's annoying. I'm inclined to just remove the args in 9.6. We could >> also add a note to the 9.5 docs, adding that the arguments are there by >> error? What about the attched patch (pg_replication_origin_xact_reset_9.5.patch)? Regards, -- Fujii Masao
Вложения
On Thu, Jul 28, 2016 at 3:44 AM, Fujii Masao <masao.fujii@gmail.com> wrote: >>>> Sure looks that way from here. Copy-and-paste from the previous >>>> line in pg_proc.h, perhaps? >> >>> Yes, that's clearly wrong. > > Attached patch (pg_replication_origin_xact_reset_9.6.patch) fixes this. > We need to apply this at least before RC1 of PostgreSQL9.6 will be released > because the patch needs the change of catalog version. > >>> Damn. Can't fix that for 9.5 anymore. The >>> function isn't all that important (especially not from SQL), but still, >>> that's annoying. I'm inclined to just remove the args in 9.6. We could >>> also add a note to the 9.5 docs, adding that the arguments are there by >>> error? > > What about the attched patch (pg_replication_origin_xact_reset_9.5.patch)? I think you should apply these ASAP. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On 2016-07-28 16:44:37 +0900, Fujii Masao wrote: > diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml > index d6ed0ce..0a3d3de 100644 > --- a/doc/src/sgml/func.sgml > +++ b/doc/src/sgml/func.sgml > @@ -17519,7 +17519,7 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup()); > <indexterm> > <primary>pg_replication_origin_xact_reset</primary> > </indexterm> > - <literal><function>pg_replication_origin_xact_reset()</function></literal> > + <literal><function>pg_replication_origin_xact_reset(<parameter>origin_lsn</parameter> <type>pg_lsn</type>, <parameter>origin_timestamp</parameter><type>timestamptz</type>)</function></literal> > </entry> > <entry> > void > @@ -17527,6 +17527,12 @@ postgres=# SELECT * FROM pg_xlogfile_name_offset(pg_stop_backup()); > <entry> > Cancel the effects of > <function>pg_replication_origin_xact_setup()</function>. > + Note that two arguments were introduced <emphasis>by mistake</> > + during the <productname>PostgreSQL</> 9.5 development cycle while > + <function>pg_replication_origin_xact_reset()</function> actually > + doesn't use them at all. Therefore, any dummy values like > + <literal>NULL</> can be safely specified as the arguments. > + This mistake will be fixed in a future release. > </entry> > </row> I don't think NULL works, the function is marked as strict. Otherwise that looks right. Thanks, Andres
Hi Fujii, On 2016-07-28 16:44:37 +0900, Fujii Masao wrote: > On Sat, Jul 2, 2016 at 7:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > > Andres Freund <andres@anarazel.de> writes: > >> On 2016-06-30 10:14:04 -0400, Tom Lane wrote: > >>> Fujii Masao <masao.fujii@gmail.com> writes: > >>>> As far as I read the code of the function, those arguments don't seem to > >>>> be necessary. So I'm afraid that the pg_proc entry for the function might > >>>> be incorrect and those two arguments should be removed from the definition. > > > >>> Sure looks that way from here. Copy-and-paste from the previous > >>> line in pg_proc.h, perhaps? > > > >> Yes, that's clearly wrong. > > Attached patch (pg_replication_origin_xact_reset_9.6.patch) fixes this. > We need to apply this at least before RC1 of PostgreSQL9.6 will be released > because the patch needs the change of catalog version. > > >> Damn. Can't fix that for 9.5 anymore. The > >> function isn't all that important (especially not from SQL), but still, > >> that's annoying. I'm inclined to just remove the args in 9.6. We could > >> also add a note to the 9.5 docs, adding that the arguments are there by > >> error? > > What about the attched patch (pg_replication_origin_xact_reset_9.5.patch)? except for the strictness remark in the other email, these look sane to me. Do you want to push them? I'll do so by Wednesday otherwise, to leave some room before the next RC. Greetings, Andres Freund
On Tue, Aug 2, 2016 at 2:48 AM, Andres Freund <andres@anarazel.de> wrote: > Hi Fujii, > > On 2016-07-28 16:44:37 +0900, Fujii Masao wrote: >> On Sat, Jul 2, 2016 at 7:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> > Andres Freund <andres@anarazel.de> writes: >> >> On 2016-06-30 10:14:04 -0400, Tom Lane wrote: >> >>> Fujii Masao <masao.fujii@gmail.com> writes: >> >>>> As far as I read the code of the function, those arguments don't seem to >> >>>> be necessary. So I'm afraid that the pg_proc entry for the function might >> >>>> be incorrect and those two arguments should be removed from the definition. >> > >> >>> Sure looks that way from here. Copy-and-paste from the previous >> >>> line in pg_proc.h, perhaps? >> > >> >> Yes, that's clearly wrong. >> >> Attached patch (pg_replication_origin_xact_reset_9.6.patch) fixes this. >> We need to apply this at least before RC1 of PostgreSQL9.6 will be released >> because the patch needs the change of catalog version. >> >> >> Damn. Can't fix that for 9.5 anymore. The >> >> function isn't all that important (especially not from SQL), but still, >> >> that's annoying. I'm inclined to just remove the args in 9.6. We could >> >> also add a note to the 9.5 docs, adding that the arguments are there by >> >> error? >> >> What about the attched patch (pg_replication_origin_xact_reset_9.5.patch)? > > except for the strictness remark in the other email, Yes, you're right. My careless mistake... :( > these look sane to > me. Do you want to push them? I'll do so by Wednesday otherwise, to > leave some room before the next RC. Could you do that if possible? Regards, -- Fujii Masao
On Tue, Aug 2, 2016 at 2:59 AM, Fujii Masao <masao.fujii@gmail.com> wrote: > On Tue, Aug 2, 2016 at 2:48 AM, Andres Freund <andres@anarazel.de> wrote: >> Hi Fujii, >> >> On 2016-07-28 16:44:37 +0900, Fujii Masao wrote: >>> On Sat, Jul 2, 2016 at 7:01 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> > Andres Freund <andres@anarazel.de> writes: >>> >> On 2016-06-30 10:14:04 -0400, Tom Lane wrote: >>> >>> Fujii Masao <masao.fujii@gmail.com> writes: >>> >>>> As far as I read the code of the function, those arguments don't seem to >>> >>>> be necessary. So I'm afraid that the pg_proc entry for the function might >>> >>>> be incorrect and those two arguments should be removed from the definition. >>> > >>> >>> Sure looks that way from here. Copy-and-paste from the previous >>> >>> line in pg_proc.h, perhaps? >>> > >>> >> Yes, that's clearly wrong. >>> >>> Attached patch (pg_replication_origin_xact_reset_9.6.patch) fixes this. >>> We need to apply this at least before RC1 of PostgreSQL9.6 will be released >>> because the patch needs the change of catalog version. >>> >>> >> Damn. Can't fix that for 9.5 anymore. The >>> >> function isn't all that important (especially not from SQL), but still, >>> >> that's annoying. I'm inclined to just remove the args in 9.6. We could >>> >> also add a note to the 9.5 docs, adding that the arguments are there by >>> >> error? >>> >>> What about the attched patch (pg_replication_origin_xact_reset_9.5.patch)? >> >> except for the strictness remark in the other email, > > Yes, you're right. My careless mistake... :( > >> these look sane to >> me. Do you want to push them? I'll do so by Wednesday otherwise, to >> leave some room before the next RC. > > Could you do that if possible? Pushed since right now I have time to do that. Anyway, thanks! Regards, -- Fujii Masao