Re: [HACKERS] unaccent module - two params function should be immutable
On Fri, Nov 8, 2013 at 06:00:53PM -0500, Tom Lane wrote: > Bruce Momjian writes: > > [ mark unaccent functions immutable ] > > > Applied. > > This patch is flat out wrong and needs to be reverted. > > The functions were correctly marked (by you!) in commit > c0577c92a84cc477a88fe6868c16c4a7e3348b11 on the basis of the discussion of > bug #5781, > http://www.postgresql.org/message-id/201012021544.ob2fitn1041...@wwwmaster.postgresql.org > which was a request exactly like this one and was denied for good and > sufficient reasons. There was absolutely no reasoning given in this > thread that explained why we should ignore the previous objections. > > In particular, marking the single-argument version of unaccent() as > immutable is the height of folly because its behavior depends on the > setting of search_path. Changing the two-argument function is maybe > a bit more debatable, but that's not what you did. > > If we were going to change the behavior, this patch would still be wrong > because it fails to provide an upgrade path. The objections saying you > needed a 1.1 migration script were completely correct. Thanks, patch reverted. If people still want this, it needs to be resbumitted with this feedback in mind. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unaccent module - two params function should be immutable
Bruce Momjian writes: > [ mark unaccent functions immutable ] > Applied. This patch is flat out wrong and needs to be reverted. The functions were correctly marked (by you!) in commit c0577c92a84cc477a88fe6868c16c4a7e3348b11 on the basis of the discussion of bug #5781, http://www.postgresql.org/message-id/201012021544.ob2fitn1041...@wwwmaster.postgresql.org which was a request exactly like this one and was denied for good and sufficient reasons. There was absolutely no reasoning given in this thread that explained why we should ignore the previous objections. In particular, marking the single-argument version of unaccent() as immutable is the height of folly because its behavior depends on the setting of search_path. Changing the two-argument function is maybe a bit more debatable, but that's not what you did. If we were going to change the behavior, this patch would still be wrong because it fails to provide an upgrade path. The objections saying you needed a 1.1 migration script were completely correct. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unaccent module - two params function should be immutable
Bruce Momjian escribió: > On Tue, Oct 8, 2013 at 02:25:25PM -0300, Alvaro Herrera wrote: > > Bruce Momjian escribió: > > > > > Do we need to update any version or anything? I didn't think so. > > > > I think there should be an 1.1 version here. That way, if somebody is > > using the existing definition from the 1.0 module, they can get the new > > definition by doing an extension upgrade. > > Uh, how would they get this new version? By compiling 9.4 and > installing it in 9.3? Oh, is this only in 9.4? Then there's no point. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unaccent module - two params function should be immutable
On Tue, Oct 8, 2013 at 02:25:25PM -0300, Alvaro Herrera wrote: > Bruce Momjian escribió: > > > Do we need to update any version or anything? I didn't think so. > > I think there should be an 1.1 version here. That way, if somebody is > using the existing definition from the 1.0 module, they can get the new > definition by doing an extension upgrade. Uh, how would they get this new version? By compiling 9.4 and installing it in 9.3? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unaccent module - two params function should be immutable
Bruce Momjian escribió: > Do we need to update any version or anything? I didn't think so. I think there should be an 1.1 version here. That way, if somebody is using the existing definition from the 1.0 module, they can get the new definition by doing an extension upgrade. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unaccent module - two params function should be immutable
2013/10/8 Bruce Momjian > On Tue, Oct 8, 2013 at 06:38:30PM +0200, Pavel Stehule wrote: > > I am not sure - does pg_upgrade change of flag after upgrade without > increasing > > version number? > > What happens in pg_upgrade is that the CREATE EXTENSION command is > pg_dump'ed, and run by pg_uprade, and it then pulls from the SQL file to > create the new function signature. > ok, then it is ok > > -- > Bruce Momjian http://momjian.us > EnterpriseDB http://enterprisedb.com > > + Everyone has their own god. + >
Re: [HACKERS] unaccent module - two params function should be immutable
On Tue, Oct 8, 2013 at 06:38:30PM +0200, Pavel Stehule wrote: > I am not sure - does pg_upgrade change of flag after upgrade without > increasing > version number? What happens in pg_upgrade is that the CREATE EXTENSION command is pg_dump'ed, and run by pg_uprade, and it then pulls from the SQL file to create the new function signature. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unaccent module - two params function should be immutable
2013/10/8 Bruce Momjian
> On Tue, Oct 8, 2013 at 06:31:03PM +0200, Pavel Stehule wrote:
> >
> >
> >
> > 2013/10/8 Bruce Momjian
> >
> > On Tue, Sep 24, 2013 at 05:36:58PM -0400, Bruce Momjian wrote:
> > > On Tue, Sep 17, 2013 at 10:15:47AM -0400, Robert Haas wrote:
> > > > On Sat, Sep 14, 2013 at 9:42 AM, Pavel Stehule <
> pavel.steh...@gmail.com
> > > wrote:
> > > > >> I have developed the attached patch based on your suggestion.
> I did
> > not
> > > > >> see anything in the code that would make it STABLE, except a
> lookup
> > of a
> > > > >> dictionary library:
> > > > >>
> > > > >> dictOid = get_ts_dict_oid(stringToQualifiedNameList
> > ("unaccent"),
> > > > >> false);
> > > > >
> > > > > yes, it risk, but similar is with timezones, and other
> external data.
> > > >
> > > > That's a catalog lookup - not a reliance on external data.
> > >
> > > Sorry, I was wrong. Only unaccent_dict() calls get_ts_dict_oid(),
> and
> > > we aren't changing the signature of that function.
> >
> > Applied.
> >
> >
> > nice
> >
> > thank you
>
> Do we need to update any version or anything? I didn't think so.
>
I am not sure - does pg_upgrade change of flag after upgrade without
increasing version number?
Regards
Pavel
>
> --
> Bruce Momjian http://momjian.us
> EnterpriseDB http://enterprisedb.com
>
> + Everyone has their own god. +
>
Re: [HACKERS] unaccent module - two params function should be immutable
On Tue, Oct 8, 2013 at 06:31:03PM +0200, Pavel Stehule wrote:
>
>
>
> 2013/10/8 Bruce Momjian
>
> On Tue, Sep 24, 2013 at 05:36:58PM -0400, Bruce Momjian wrote:
> > On Tue, Sep 17, 2013 at 10:15:47AM -0400, Robert Haas wrote:
> > > On Sat, Sep 14, 2013 at 9:42 AM, Pavel Stehule
> > wrote:
> > > >> I have developed the attached patch based on your suggestion. I
> did
> not
> > > >> see anything in the code that would make it STABLE, except a lookup
> of a
> > > >> dictionary library:
> > > >>
> > > >> dictOid = get_ts_dict_oid(stringToQualifiedNameList
> ("unaccent"),
> > > >> false);
> > > >
> > > > yes, it risk, but similar is with timezones, and other external
> data.
> > >
> > > That's a catalog lookup - not a reliance on external data.
> >
> > Sorry, I was wrong. Only unaccent_dict() calls get_ts_dict_oid(), and
> > we aren't changing the signature of that function.
>
> Applied.
>
>
> nice
>
> thank you
Do we need to update any version or anything? I didn't think so.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unaccent module - two params function should be immutable
2013/10/8 Bruce Momjian
> On Tue, Sep 24, 2013 at 05:36:58PM -0400, Bruce Momjian wrote:
> > On Tue, Sep 17, 2013 at 10:15:47AM -0400, Robert Haas wrote:
> > > On Sat, Sep 14, 2013 at 9:42 AM, Pavel Stehule <
> pavel.steh...@gmail.com> wrote:
> > > >> I have developed the attached patch based on your suggestion. I
> did not
> > > >> see anything in the code that would make it STABLE, except a lookup
> of a
> > > >> dictionary library:
> > > >>
> > > >> dictOid =
> get_ts_dict_oid(stringToQualifiedNameList("unaccent"),
> > > >> false);
> > > >
> > > > yes, it risk, but similar is with timezones, and other external data.
> > >
> > > That's a catalog lookup - not a reliance on external data.
> >
> > Sorry, I was wrong. Only unaccent_dict() calls get_ts_dict_oid(), and
> > we aren't changing the signature of that function.
>
> Applied.
>
nice
thank you
Regards
Pavel Stehule
>
> --
> Bruce Momjian http://momjian.us
> EnterpriseDB http://enterprisedb.com
>
> + It's impossible for everything to be true. +
>
Re: [HACKERS] unaccent module - two params function should be immutable
On Tue, Sep 24, 2013 at 05:36:58PM -0400, Bruce Momjian wrote:
> On Tue, Sep 17, 2013 at 10:15:47AM -0400, Robert Haas wrote:
> > On Sat, Sep 14, 2013 at 9:42 AM, Pavel Stehule
> > wrote:
> > >> I have developed the attached patch based on your suggestion. I did not
> > >> see anything in the code that would make it STABLE, except a lookup of a
> > >> dictionary library:
> > >>
> > >> dictOid = get_ts_dict_oid(stringToQualifiedNameList("unaccent"),
> > >> false);
> > >
> > > yes, it risk, but similar is with timezones, and other external data.
> >
> > That's a catalog lookup - not a reliance on external data.
>
> Sorry, I was wrong. Only unaccent_dict() calls get_ts_dict_oid(), and
> we aren't changing the signature of that function.
Applied.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unaccent module - two params function should be immutable
On Tue, Sep 17, 2013 at 10:15:47AM -0400, Robert Haas wrote:
> On Sat, Sep 14, 2013 at 9:42 AM, Pavel Stehule
> wrote:
> >> I have developed the attached patch based on your suggestion. I did not
> >> see anything in the code that would make it STABLE, except a lookup of a
> >> dictionary library:
> >>
> >> dictOid = get_ts_dict_oid(stringToQualifiedNameList("unaccent"),
> >> false);
> >
> > yes, it risk, but similar is with timezones, and other external data.
>
> That's a catalog lookup - not a reliance on external data.
Sorry, I was wrong. Only unaccent_dict() calls get_ts_dict_oid(), and
we aren't changing the signature of that function.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unaccent module - two params function should be immutable
On Sat, Sep 14, 2013 at 9:42 AM, Pavel Stehule wrote:
>> I have developed the attached patch based on your suggestion. I did not
>> see anything in the code that would make it STABLE, except a lookup of a
>> dictionary library:
>>
>> dictOid = get_ts_dict_oid(stringToQualifiedNameList("unaccent"),
>> false);
>
> yes, it risk, but similar is with timezones, and other external data.
That's a catalog lookup - not a reliance on external data.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] unaccent module - two params function should be immutable
2013/9/11 Bruce Momjian
> On Tue, Feb 19, 2013 at 08:30:29AM +0100, Pavel Stehule wrote:
> > Hello
> >
> > There was a proposal to change flag of function to immutable - should
> > be used in indexes
> >
> > CREATE FUNCTION unaccent(regdictionary, text)
> > RETURNS text
> > AS 'MODULE_PATHNAME', 'unaccent_dict'
> > LANGUAGE C STABLE STRICT;
> >
> >
> > is there any progress?
>
> I have developed the attached patch based on your suggestion. I did not
> see anything in the code that would make it STABLE, except a lookup of a
> dictionary library:
>
> dictOid = get_ts_dict_oid(stringToQualifiedNameList("unaccent"),
> false);
>
yes, it risk, but similar is with timezones, and other external data.
Regards
Pavel
>
> --
> Bruce Momjian http://momjian.us
> EnterpriseDB http://enterprisedb.com
>
> + It's impossible for everything to be true. +
>
Re: [HACKERS] unaccent module - two params function should be immutable
On Tue, Feb 19, 2013 at 08:30:29AM +0100, Pavel Stehule wrote:
> Hello
>
> There was a proposal to change flag of function to immutable - should
> be used in indexes
>
> CREATE FUNCTION unaccent(regdictionary, text)
> RETURNS text
> AS 'MODULE_PATHNAME', 'unaccent_dict'
> LANGUAGE C STABLE STRICT;
>
>
> is there any progress?
I have developed the attached patch based on your suggestion. I did not
see anything in the code that would make it STABLE, except a lookup of a
dictionary library:
dictOid = get_ts_dict_oid(stringToQualifiedNameList("unaccent"), false);
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/contrib/unaccent/unaccent--1.0.sql b/contrib/unaccent/unaccent--1.0.sql
new file mode 100644
index 9085ca4..072b749
*** a/contrib/unaccent/unaccent--1.0.sql
--- b/contrib/unaccent/unaccent--1.0.sql
***
*** 6,17
CREATE FUNCTION unaccent(regdictionary, text)
RETURNS text
AS 'MODULE_PATHNAME', 'unaccent_dict'
! LANGUAGE C STABLE STRICT;
CREATE FUNCTION unaccent(text)
RETURNS text
AS 'MODULE_PATHNAME', 'unaccent_dict'
! LANGUAGE C STABLE STRICT;
CREATE FUNCTION unaccent_init(internal)
RETURNS internal
--- 6,17
CREATE FUNCTION unaccent(regdictionary, text)
RETURNS text
AS 'MODULE_PATHNAME', 'unaccent_dict'
! LANGUAGE C IMMUTABLE STRICT;
CREATE FUNCTION unaccent(text)
RETURNS text
AS 'MODULE_PATHNAME', 'unaccent_dict'
! LANGUAGE C IMMUTABLE STRICT;
CREATE FUNCTION unaccent_init(internal)
RETURNS internal
diff --git a/contrib/unaccent/unaccent--unpackaged--1.0.sql b/contrib/unaccent/unaccent--unpackaged--1.0.sql
new file mode 100644
index abd0698..5bff74e
*** a/contrib/unaccent/unaccent--unpackaged--1.0.sql
--- b/contrib/unaccent/unaccent--unpackaged--1.0.sql
*** ALTER EXTENSION unaccent ADD function un
*** 10,16
ALTER EXTENSION unaccent ADD text search template unaccent;
ALTER EXTENSION unaccent ADD text search dictionary unaccent;
! -- These functions are marked as stable in 9.1, were not before:
! ALTER FUNCTION unaccent(regdictionary, text) STABLE;
! ALTER FUNCTION unaccent(text) STABLE;
--- 10,16
ALTER EXTENSION unaccent ADD text search template unaccent;
ALTER EXTENSION unaccent ADD text search dictionary unaccent;
! -- These functions were marked as stable in 9.1; they were now marked as immutable
! ALTER FUNCTION unaccent(regdictionary, text) IMMUTABLE;
! ALTER FUNCTION unaccent(text) IMMUTABLE;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
