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 br...@momjian.us 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 br...@momjian.ushttp://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 br...@momjian.us 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
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. -- Bruce Momjian br...@momjian.ushttp://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
2013/10/8 Bruce Momjian br...@momjian.us 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 br...@momjian.ushttp://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, Oct 8, 2013 at 06:31:03PM +0200, Pavel Stehule wrote: 2013/10/8 Bruce Momjian br...@momjian.us 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. -- Bruce Momjian br...@momjian.ushttp://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 br...@momjian.us On Tue, Oct 8, 2013 at 06:31:03PM +0200, Pavel Stehule wrote: 2013/10/8 Bruce Momjian br...@momjian.us 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 br...@momjian.ushttp://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 br...@momjian.ushttp://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 br...@momjian.us 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 br...@momjian.ushttp://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +
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
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 br...@momjian.ushttp://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ó: 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, 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. -- Bruce Momjian br...@momjian.ushttp://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 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. -- 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 br...@momjian.us 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 br...@momjian.ushttp://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 br...@momjian.ushttp://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
[HACKERS] unaccent module - two params function should be immutable
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? Regards Pavel Stehule -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
