Обсуждение: 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
Вложения
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
On Sat, Jun 25, 2016 at 3:44 AM, Andreas Karlsson <andreas@proxel.se> 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
On 30 June 2016 at 03:49, Robert Haas <robertmhaas@gmail.com> wrote: > On Sat, Jun 25, 2016 at 3:44 AM, Andreas Karlsson <andreas@proxel.se> 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
Вложения
On 07/09/2016 05:42 AM, David Rowley wrote: > On 30 June 2016 at 03:49, Robert Haas <robertmhaas@gmail.com> wrote: >> On Sat, Jun 25, 2016 at 3:44 AM, Andreas Karlsson <andreas@proxel.se> 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
On 14 July 2016 at 02:00, Andreas Karlsson <andreas@proxel.se> wrote: > On 07/09/2016 05:42 AM, David Rowley wrote: >> On 30 June 2016 at 03:49, Robert Haas <robertmhaas@gmail.com> 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
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 <robertmhaas@gmail.com> wrote: > >>On Sat, Jun 25, 2016 at 3:44 AM, Andreas Karlsson <andreas@proxel.se> 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.GA447393@tornado.leadboat.com
On Tue, Jul 26, 2016 at 3:48 AM, Noah Misch <noah@leadboat.com> 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 <robertmhaas@gmail.com> wrote: >> >>On Sat, Jun 25, 2016 at 3:44 AM, Andreas Karlsson <andreas@proxel.se> 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