Re: [HACKERS] bug in citext's upgrade script for parallel aggregates
On Tue, Jul 26, 2016 at 3:48 AM, Noah Misch wrote: > On Thu, Jul 14, 2016 at 02:00:59AM +0200, Andreas Karlsson wrote: >> On 07/09/2016 05:42 AM, David Rowley wrote: >> >On 30 June 2016 at 03:49, Robert Haas wrote: >> >>On Sat, Jun 25, 2016 at 3:44 AM, Andreas Karlsson >> >>wrote: >> >>>On 06/24/2016 01:31 PM, David Rowley wrote: >> Seems there's a small error in the upgrade script for citext for 1.1 >> to 1.2 which will cause min(citext) not to be parallel enabled. >> >> max(citext)'s combinefunc is first set incorrectly, but then updated >> to the correct value. I assume it was meant to set the combine >> function for min(citext) instead. >> >> Fix attached. I've assumed that because we're still in beta that we >> can get away with this fix rather than making a 1.3 version to fix the >> issue. >> >>> >> >>>Yes, this is indeed a bug. >> >> >> >>Since we've already released beta2, I think we need to do a whole new >> >>extension version. We treated beta1 as a sufficiently-significant >> >>event to mandate a version bump, so we should do the same here. >> > >> >Ok, good point. Patch attached. >> >> Thanks! >> >> I tested the patch and it looks good. > > [Action required within 72 hours. This is a generic notification.] > > The above-described topic is currently a PostgreSQL 9.6 open item. Robert, > since you committed the patch believed to have created it, you own this open > item. If some other commit is more relevant or if this does not belong as a > 9.6 open item, please let us know. Otherwise, please observe the policy on > open item ownership[1] and send a status update within 72 hours of this > message. Include a date for your subsequent status update. Testers may > discover new open items at any time, and I want to plan to get them all fixed > well in advance of shipping 9.6rc1. Consequently, I will appreciate your > efforts toward speedy resolution. Thanks. Committed the patch. -- 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] bug in citext's upgrade script for parallel aggregates
On Thu, Jul 14, 2016 at 02:00:59AM +0200, Andreas Karlsson wrote: > On 07/09/2016 05:42 AM, David Rowley wrote: > >On 30 June 2016 at 03:49, Robert Haas wrote: > >>On Sat, Jun 25, 2016 at 3:44 AM, Andreas Karlsson wrote: > >>>On 06/24/2016 01:31 PM, David Rowley wrote: > Seems there's a small error in the upgrade script for citext for 1.1 > to 1.2 which will cause min(citext) not to be parallel enabled. > > max(citext)'s combinefunc is first set incorrectly, but then updated > to the correct value. I assume it was meant to set the combine > function for min(citext) instead. > > Fix attached. I've assumed that because we're still in beta that we > can get away with this fix rather than making a 1.3 version to fix the > issue. > >>> > >>>Yes, this is indeed a bug. > >> > >>Since we've already released beta2, I think we need to do a whole new > >>extension version. We treated beta1 as a sufficiently-significant > >>event to mandate a version bump, so we should do the same here. > > > >Ok, good point. Patch attached. > > Thanks! > > I tested the patch and it looks good. [Action required within 72 hours. This is a generic notification.] The above-described topic is currently a PostgreSQL 9.6 open item. Robert, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a 9.6 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within 72 hours of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping 9.6rc1. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] http://www.postgresql.org/message-id/20160527025039.ga447...@tornado.leadboat.com -- 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] bug in citext's upgrade script for parallel aggregates
On 14 July 2016 at 02:00, Andreas Karlsson wrote: > On 07/09/2016 05:42 AM, David Rowley wrote: >> On 30 June 2016 at 03:49, Robert Haas wrote: >>> Since we've already released beta2, I think we need to do a whole new >>> extension version. We treated beta1 as a sufficiently-significant >>> event to mandate a version bump, so we should do the same here. >> >> >> Ok, good point. Patch attached. > > > Thanks! > > I tested the patch and it looks good. Thanks for looking. I've added this to the open items list so that it does not get forgotten about. -- David Rowley http://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] bug in citext's upgrade script for parallel aggregates
On 07/09/2016 05:42 AM, David Rowley wrote: On 30 June 2016 at 03:49, Robert Haas wrote: On Sat, Jun 25, 2016 at 3:44 AM, Andreas Karlsson wrote: On 06/24/2016 01:31 PM, David Rowley wrote: Seems there's a small error in the upgrade script for citext for 1.1 to 1.2 which will cause min(citext) not to be parallel enabled. max(citext)'s combinefunc is first set incorrectly, but then updated to the correct value. I assume it was meant to set the combine function for min(citext) instead. Fix attached. I've assumed that because we're still in beta that we can get away with this fix rather than making a 1.3 version to fix the issue. Yes, this is indeed a bug. Since we've already released beta2, I think we need to do a whole new extension version. We treated beta1 as a sufficiently-significant event to mandate a version bump, so we should do the same here. Ok, good point. Patch attached. Thanks! I tested the patch and it looks good. Andreas -- 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] bug in citext's upgrade script for parallel aggregates
On 30 June 2016 at 03:49, Robert Haas wrote: > On Sat, Jun 25, 2016 at 3:44 AM, Andreas Karlsson wrote: >> On 06/24/2016 01:31 PM, David Rowley wrote: >>> Seems there's a small error in the upgrade script for citext for 1.1 >>> to 1.2 which will cause min(citext) not to be parallel enabled. >>> >>> max(citext)'s combinefunc is first set incorrectly, but then updated >>> to the correct value. I assume it was meant to set the combine >>> function for min(citext) instead. >>> >>> Fix attached. I've assumed that because we're still in beta that we >>> can get away with this fix rather than making a 1.3 version to fix the >>> issue. >> >> Yes, this is indeed a bug. > > Since we've already released beta2, I think we need to do a whole new > extension version. We treated beta1 as a sufficiently-significant > event to mandate a version bump, so we should do the same here. Ok, good point. Patch attached. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services From bb9e38894f928b47260f5fd9ba34535b47be3d88 Mon Sep 17 00:00:00 2001 From: David Rowley Date: Sat, 9 Jul 2016 15:37:33 +1200 Subject: [PATCH] Add missing combine function for min(citext) This would have been set correctly for new installs of 1.2, but the upgrade step from 1.1 to 1.2 was incorrect and instead of setting the combine function for min(citext) max(citext) was set twice. --- contrib/citext/Makefile | 4 +- contrib/citext/citext--1.2--1.3.sql | 7 + contrib/citext/citext--1.2.sql | 493 contrib/citext/citext--1.3.sql | 493 contrib/citext/citext.control | 2 +- 5 files changed, 503 insertions(+), 496 deletions(-) create mode 100644 contrib/citext/citext--1.2--1.3.sql delete mode 100644 contrib/citext/citext--1.2.sql create mode 100644 contrib/citext/citext--1.3.sql diff --git a/contrib/citext/Makefile b/contrib/citext/Makefile index 363a11b..e39d3ee 100644 --- a/contrib/citext/Makefile +++ b/contrib/citext/Makefile @@ -3,8 +3,8 @@ MODULES = citext EXTENSION = citext -DATA = citext--1.2.sql citext--1.1--1.2.sql citext--1.0--1.1.sql \ - citext--unpackaged--1.0.sql +DATA = citext--1.3.sql citext--1.2--1.3.sql citext--1.1--1.2.sql \ + citext--1.0--1.1.sql citext--unpackaged--1.0.sql PGFILEDESC = "citext - case-insensitive character string data type" REGRESS = citext diff --git a/contrib/citext/citext--1.2--1.3.sql b/contrib/citext/citext--1.2--1.3.sql new file mode 100644 index 000..4ab8679 --- /dev/null +++ b/contrib/citext/citext--1.2--1.3.sql @@ -0,0 +1,7 @@ +/* contrib/citext/citext--1.2--1.3.sql */ + +-- complain if script is sourced in psql, rather than via ALTER EXTENSION +\echo Use "ALTER EXTENSION citext UPDATE TO '1.3'" to load this file. \quit + +UPDATE pg_aggregate SET aggcombinefn = 'citext_smaller' +WHERE aggfnoid = 'min(citext)'::pg_catalog.regprocedure; diff --git a/contrib/citext/citext--1.2.sql b/contrib/citext/citext--1.2.sql deleted file mode 100644 index c2d0c0c..000 --- a/contrib/citext/citext--1.2.sql +++ /dev/null @@ -1,493 +0,0 @@ -/* contrib/citext/citext--1.2.sql */ - --- complain if script is sourced in psql, rather than via CREATE EXTENSION -\echo Use "CREATE EXTENSION citext" to load this file. \quit - --- --- PostgreSQL code for CITEXT. --- --- Most I/O functions, and a few others, piggyback on the "text" type --- functions via the implicit cast to text. --- - --- --- Shell type to keep things a bit quieter. --- - -CREATE TYPE citext; - --- --- Input and output functions. --- -CREATE FUNCTION citextin(cstring) -RETURNS citext -AS 'textin' -LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE; - -CREATE FUNCTION citextout(citext) -RETURNS cstring -AS 'textout' -LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE; - -CREATE FUNCTION citextrecv(internal) -RETURNS citext -AS 'textrecv' -LANGUAGE internal STABLE STRICT PARALLEL SAFE; - -CREATE FUNCTION citextsend(citext) -RETURNS bytea -AS 'textsend' -LANGUAGE internal STABLE STRICT PARALLEL SAFE; - --- --- The type itself. --- - -CREATE TYPE citext ( -INPUT = citextin, -OUTPUT = citextout, -RECEIVE= citextrecv, -SEND = citextsend, -INTERNALLENGTH = VARIABLE, -STORAGE= extended, --- make it a non-preferred member of string type category -CATEGORY = 'S', -PREFERRED = false, -COLLATABLE = true -); - --- --- Type casting functions for those situations where the I/O casts don't --- automatically kick in. --- - -CREATE FUNCTION citext(bpchar) -RETURNS citext -AS 'rtrim1' -LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE; - -CREATE FUNCTION citext(boolean) -RETURNS citext -AS 'booltext' -LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE; - -CREATE FUNCTION citext(inet) -RETURNS citext -AS 'network_show' -LANGUAGE internal IMMUTABLE STRICT PARALLEL SAFE; - --- --- Implicit and assignment t
Re: [HACKERS] bug in citext's upgrade script for parallel aggregates
On Sat, Jun 25, 2016 at 3:44 AM, Andreas Karlsson wrote: > On 06/24/2016 01:31 PM, David Rowley wrote: >> Seems there's a small error in the upgrade script for citext for 1.1 >> to 1.2 which will cause min(citext) not to be parallel enabled. >> >> max(citext)'s combinefunc is first set incorrectly, but then updated >> to the correct value. I assume it was meant to set the combine >> function for min(citext) instead. >> >> Fix attached. I've assumed that because we're still in beta that we >> can get away with this fix rather than making a 1.3 version to fix the >> issue. > > Yes, this is indeed a bug. Since we've already released beta2, I think we need to do a whole new extension version. We treated beta1 as a sufficiently-significant event to mandate a version bump, so we should do the same here. -- 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] bug in citext's upgrade script for parallel aggregates
On 06/24/2016 01:31 PM, David Rowley wrote: Seems there's a small error in the upgrade script for citext for 1.1 to 1.2 which will cause min(citext) not to be parallel enabled. max(citext)'s combinefunc is first set incorrectly, but then updated to the correct value. I assume it was meant to set the combine function for min(citext) instead. Fix attached. I've assumed that because we're still in beta that we can get away with this fix rather than making a 1.3 version to fix the issue. Yes, this is indeed a bug. Andreas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] bug in citext's upgrade script for parallel aggregates
Seems there's a small error in the upgrade script for citext for 1.1 to 1.2 which will cause min(citext) not to be parallel enabled. max(citext)'s combinefunc is first set incorrectly, but then updated to the correct value. I assume it was meant to set the combine function for min(citext) instead. Fix attached. I've assumed that because we're still in beta that we can get away with this fix rather than making a 1.3 version to fix the issue. -- David Rowley http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services citext_1.1_to_1.2_upgrade_fix.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
