Re: [HACKERS] Is this a bug?
On Fri, Sep 4, 2015 at 09:40:10AM +0900, Michael Paquier wrote: > Robert Haas wrote: > > On Wed, Aug 26, 2015 at 3:31 PM, Alvaro Herrera > > wrote: > > > Fabrízio de Royes Mello wrote: > > > > > >> Why this patch was reverted one day after applied [1]? I didn't see > any > > >> discussion around it. > > > > > > Contributors whose patches are getting committed should really > subscribe > > > to pgsql-committers. > > > > I would have thought discussion of committed patches should be moved > > to -hackers. > > I agree, but it happens anyway quite frequently. Since recently, I make > a point of changing the CC from -committers to -hackers, but due to > force of habit I often forget. > > > Noted. I usually don't do that. I am thinking we should all agree if we should redirect commit comments to hackers, or not, so we are consistent. -- 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] Is this a bug?
On Fri, Sep 4, 2015 at 3:52 AM, Alvaro Herrera wrote: > Robert Haas wrote: > > On Wed, Aug 26, 2015 at 3:31 PM, Alvaro Herrera > > wrote: > > > Fabrízio de Royes Mello wrote: > > > > > >> Why this patch was reverted one day after applied [1]? I didn't see > any > > >> discussion around it. > > > > > > Contributors whose patches are getting committed should really > subscribe > > > to pgsql-committers. > > > > I would have thought discussion of committed patches should be moved > > to -hackers. > > I agree, but it happens anyway quite frequently. Since recently, I make > a point of changing the CC from -committers to -hackers, but due to > force of habit I often forget. > Noted. I usually don't do that. -- Michael
Re: [HACKERS] Is this a bug?
Robert Haas wrote: > On Wed, Aug 26, 2015 at 3:31 PM, Alvaro Herrera > wrote: > > Fabrízio de Royes Mello wrote: > > > >> Why this patch was reverted one day after applied [1]? I didn't see any > >> discussion around it. > > > > Contributors whose patches are getting committed should really subscribe > > to pgsql-committers. > > I would have thought discussion of committed patches should be moved > to -hackers. I agree, but it happens anyway quite frequently. Since recently, I make a point of changing the CC from -committers to -hackers, but due to force of habit I often forget. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, 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] Is this a bug?
On Wed, Aug 26, 2015 at 3:31 PM, Alvaro Herrera wrote: > Fabrízio de Royes Mello wrote: > >> Why this patch was reverted one day after applied [1]? I didn't see any >> discussion around it. > > Contributors whose patches are getting committed should really subscribe > to pgsql-committers. I would have thought discussion of committed patches should be moved to -hackers. The description for the -committers list says: "Notification of git commits are sent to this list. Do not post here!" So, it's understandable that people would not expect other traffic there. -- 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] Is this a bug?
On Wed, Aug 26, 2015 at 4:31 PM, Alvaro Herrera wrote: > > Fabrízio de Royes Mello wrote: > > > Why this patch was reverted one day after applied [1]? I didn't see any > > discussion around it. > > Contributors whose patches are getting committed should really subscribe > to pgsql-committers. > Ok. I sent a subscribe to pgsql-committers. Thanks, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL >> Timbira: http://www.timbira.com.br >> Blog: http://fabriziomello.github.io >> Linkedin: http://br.linkedin.com/in/fabriziomello >> Twitter: http://twitter.com/fabriziomello >> Github: http://github.com/fabriziomello
Re: [HACKERS] Is this a bug?
On Wed, Aug 26, 2015 at 4:30 PM, Andres Freund wrote: > > On 2015-08-26 16:24:31 -0300, Fabrízio de Royes Mello wrote: > > Why this patch was reverted one day after applied [1]? I didn't see any > > discussion around it. > > http://www.postgresql.org/message-id/23918.1409010...@sss.pgh.pa.us Thanks I'm not subscribed to pgsql-commiters so I didn't see it. -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL >> Timbira: http://www.timbira.com.br >> Blog: http://fabriziomello.github.io >> Linkedin: http://br.linkedin.com/in/fabriziomello >> Twitter: http://twitter.com/fabriziomello >> Github: http://github.com/fabriziomello
Re: [HACKERS] Is this a bug?
Fabrízio de Royes Mello wrote: > Why this patch was reverted one day after applied [1]? I didn't see any > discussion around it. Contributors whose patches are getting committed should really subscribe to pgsql-committers. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, 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] Is this a bug?
On 2015-08-26 16:24:31 -0300, Fabrízio de Royes Mello wrote: > Why this patch was reverted one day after applied [1]? I didn't see any > discussion around it. http://www.postgresql.org/message-id/23918.1409010...@sss.pgh.pa.us -- 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] Is this a bug?
On 26 August 2015 at 20:24, Fabrízio de Royes Mello wrote: > > On Mon, Aug 25, 2014 at 6:07 PM, Bruce Momjian wrote: > > > > On Fri, Aug 22, 2014 at 10:04:50PM -0400, Bruce Momjian wrote: > > > On Fri, Aug 22, 2014 at 03:12:47PM -0400, Robert Haas wrote: > > > > On Fri, Aug 22, 2014 at 2:33 PM, Bruce Momjian > wrote: > > > > >> Yes, you remember well. I will have to find a different way for > > > > >> pg_upgrade to call a no-op ALTER TABLE, which is fine. > > > > > > > > > > Looking at the ALTER TABLE options, I am going to put this check > in a > > > > > !IsBinaryUpgrade block so pg_upgrade can still use its trick. > > > > > > > > -1, that's really ugly. > > > > > > > > Maybe the right solution is to add a form of ALTER TABLE that is > > > > specifically defined to do only this check. This is an ongoing need, > > > > so that might not be out of line. > > > > > > Ah, seems ALTER TABLE ... DROP CONSTRAINT IF EXISTS also works --- I > > > will use that. > > > > OK, attached patch applied, with pg_upgrade adjustments. I didn't > > think the original regression tests for this were necessary. > > > > Hi, > > Why this patch was reverted one day after applied [1]? I didn't see any > discussion around it. > > Regards, > > [1] > http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=6cb74a67e26523eb2408f441bfc589c80f76c465 > The discussion was here: http://www.postgresql.org/message-id/20140826000757.ge14...@momjian.us Thom
Re: [HACKERS] Is this a bug?
On Mon, Aug 25, 2014 at 6:07 PM, Bruce Momjian wrote: > > On Fri, Aug 22, 2014 at 10:04:50PM -0400, Bruce Momjian wrote: > > On Fri, Aug 22, 2014 at 03:12:47PM -0400, Robert Haas wrote: > > > On Fri, Aug 22, 2014 at 2:33 PM, Bruce Momjian wrote: > > > >> Yes, you remember well. I will have to find a different way for > > > >> pg_upgrade to call a no-op ALTER TABLE, which is fine. > > > > > > > > Looking at the ALTER TABLE options, I am going to put this check in a > > > > !IsBinaryUpgrade block so pg_upgrade can still use its trick. > > > > > > -1, that's really ugly. > > > > > > Maybe the right solution is to add a form of ALTER TABLE that is > > > specifically defined to do only this check. This is an ongoing need, > > > so that might not be out of line. > > > > Ah, seems ALTER TABLE ... DROP CONSTRAINT IF EXISTS also works --- I > > will use that. > > OK, attached patch applied, with pg_upgrade adjustments. I didn't > think the original regression tests for this were necessary. > Hi, Why this patch was reverted one day after applied [1]? I didn't see any discussion around it. Regards, [1] http://git.postgresql.org/gitweb/?p=postgresql.git;a=commit;h=6cb74a67e26523eb2408f441bfc589c80f76c465 -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL >> Timbira: http://www.timbira.com.br >> Blog: http://fabriziomello.github.io >> Linkedin: http://br.linkedin.com/in/fabriziomello >> Twitter: http://twitter.com/fabriziomello >> Github: http://github.com/fabriziomello
Re: [HACKERS] Is this a bug?
On Fri, Aug 22, 2014 at 10:04:50PM -0400, Bruce Momjian wrote:
> On Fri, Aug 22, 2014 at 03:12:47PM -0400, Robert Haas wrote:
> > On Fri, Aug 22, 2014 at 2:33 PM, Bruce Momjian wrote:
> > >> Yes, you remember well. I will have to find a different way for
> > >> pg_upgrade to call a no-op ALTER TABLE, which is fine.
> > >
> > > Looking at the ALTER TABLE options, I am going to put this check in a
> > > !IsBinaryUpgrade block so pg_upgrade can still use its trick.
> >
> > -1, that's really ugly.
> >
> > Maybe the right solution is to add a form of ALTER TABLE that is
> > specifically defined to do only this check. This is an ongoing need,
> > so that might not be out of line.
>
> Ah, seems ALTER TABLE ... DROP CONSTRAINT IF EXISTS also works --- I
> will use that.
OK, attached patch applied, with pg_upgrade adjustments. I didn't
think the original regression tests for this were necessary.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ Everyone has their own god. +
diff --git a/contrib/pg_upgrade/dump.c b/contrib/pg_upgrade/dump.c
new file mode 100644
index e623a22..29a68c0
*** a/contrib/pg_upgrade/dump.c
--- b/contrib/pg_upgrade/dump.c
*** optionally_create_toast_tables(void)
*** 115,120
--- 115,124
"c.relkind IN ('r', 'm') AND "
"c.reltoastrelid = 0");
+ /* Suppress NOTICE output from non-existant constraints */
+ PQclear(executeQueryOrDie(conn, "SET client_min_messages = warning;"));
+ PQclear(executeQueryOrDie(conn, "SET log_min_messages = warning;"));
+
ntups = PQntuples(res);
i_nspname = PQfnumber(res, "nspname");
i_relname = PQfnumber(res, "relname");
*** optionally_create_toast_tables(void)
*** 125,137
OPTIONALLY_CREATE_TOAST_OID));
/* dummy command that also triggers check for required TOAST table */
! PQclear(executeQueryOrDie(conn, "ALTER TABLE %s.%s RESET (binary_upgrade_dummy_option);",
quote_identifier(PQgetvalue(res, rowno, i_nspname)),
quote_identifier(PQgetvalue(res, rowno, i_relname;
}
PQclear(res);
PQfinish(conn);
}
--- 129,144
OPTIONALLY_CREATE_TOAST_OID));
/* dummy command that also triggers check for required TOAST table */
! PQclear(executeQueryOrDie(conn, "ALTER TABLE %s.%s DROP CONSTRAINT IF EXISTS binary_upgrade_dummy_constraint;",
quote_identifier(PQgetvalue(res, rowno, i_nspname)),
quote_identifier(PQgetvalue(res, rowno, i_relname;
}
PQclear(res);
+ PQclear(executeQueryOrDie(conn, "RESET client_min_messages;"));
+ PQclear(executeQueryOrDie(conn, "RESET log_min_messages;"));
+
PQfinish(conn);
}
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
new file mode 100644
index e0b81b9..97a4e22
*** a/src/backend/access/common/reloptions.c
--- b/src/backend/access/common/reloptions.c
*** static void initialize_reloptions(void);
*** 307,312
--- 307,314
static void parse_one_reloption(relopt_value *option, char *text_str,
int text_len, bool validate);
+ static bool is_valid_reloption(char *name);
+
/*
* initialize_reloptions
* initialization routine, must be called before parsing
*** initialize_reloptions(void)
*** 382,387
--- 384,408
}
/*
+ * is_valid_reloption
+ * check if a reloption exists
+ *
+ */
+ static bool
+ is_valid_reloption(char *name)
+ {
+ int i;
+
+ for (i = 0; relOpts[i]; i++)
+ {
+ if (pg_strcasecmp(relOpts[i]->name, name) == 0)
+ return true;
+ }
+
+ return false;
+ }
+
+ /*
* add_reloption_kind
* Create a new relopt_kind value, to be used in custom reloptions by
* user-defined AMs.
*** transformRelOptions(Datum oldOptions, Li
*** 672,677
--- 693,703
if (isReset)
{
+ if (!is_valid_reloption(def->defname))
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("unrecognized parameter \"%s\"", def->defname)));
+
if (def->arg != NULL)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
--
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] Is this a bug?
On Fri, Aug 22, 2014 at 03:12:47PM -0400, Robert Haas wrote: > On Fri, Aug 22, 2014 at 2:33 PM, Bruce Momjian wrote: > >> Yes, you remember well. I will have to find a different way for > >> pg_upgrade to call a no-op ALTER TABLE, which is fine. > > > > Looking at the ALTER TABLE options, I am going to put this check in a > > !IsBinaryUpgrade block so pg_upgrade can still use its trick. > > -1, that's really ugly. > > Maybe the right solution is to add a form of ALTER TABLE that is > specifically defined to do only this check. This is an ongoing need, > so that might not be out of line. Ah, seems ALTER TABLE ... DROP CONSTRAINT IF EXISTS also works --- I will use that. -- 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] Is this a bug?
On Fri, Aug 22, 2014 at 2:33 PM, Bruce Momjian wrote: >> Yes, you remember well. I will have to find a different way for >> pg_upgrade to call a no-op ALTER TABLE, which is fine. > > Looking at the ALTER TABLE options, I am going to put this check in a > !IsBinaryUpgrade block so pg_upgrade can still use its trick. -1, that's really ugly. Maybe the right solution is to add a form of ALTER TABLE that is specifically defined to do only this check. This is an ongoing need, so that might not be out of line. -- 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] Is this a bug?
On August 22, 2014 8:33:57 PM CEST, Bruce Momjian wrote: >On Fri, Aug 22, 2014 at 12:53:30PM -0400, Bruce Momjian wrote: >> On Fri, Aug 22, 2014 at 10:27:02AM -0400, Robert Haas wrote: >> > On Thu, Aug 21, 2014 at 7:17 PM, Bruce Momjian >wrote: >> > > On Tue, Mar 18, 2014 at 09:11:46AM -0400, Robert Haas wrote: >> > >> On Mon, Mar 17, 2014 at 10:27 PM, Michael Paquier >> > >> wrote: >> > >> > On Tue, Mar 18, 2014 at 10:24 AM, Fabrízio de Royes Mello >> > >> > wrote: >> > >> >> >> > >> >> On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas > wrote: >> > >> >>> Well, it's fairly harmless, but it might not be a bad idea >to tighten that >> > >> >>> up. >> > >> >> The attached patch tighten that up. >> > >> > Hm... It might be interesting to include it in 9.4 IMO, >somewhat >> > >> > grouping with what has been done in a6542a4 for SET and ABORT. >> > >> >> > >> Meh. There will always be another thing we could squeeze in; I >don't >> > >> think this is particularly urgent, and it's late to the party. >> > > >> > > Do we want this patch for 9.5? It throws an error for invalid >reloption >> > > specifications. >> > >> > Fine with me. But I have a vague recollection of seeing pg_upgrade >> > doing this on purpose to create TOAST tables or something... am I >> > misremembering? >> >> Yes, you remember well. I will have to find a different way for >> pg_upgrade to call a no-op ALTER TABLE, which is fine. > >Looking at the ALTER TABLE options, I am going to put this check in a >!IsBinaryUpgrade block so pg_upgrade can still use it? Why not simply not do anything? This doesn't prevent any bugs and requiring pg-upgrade specific checks in there seems absurd. Also somebody might use it for similar purposes. --- Please excuse brevity and formatting - I am writing this on my mobile phone. -- 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] Is this a bug?
On Fri, Aug 22, 2014 at 12:53:30PM -0400, Bruce Momjian wrote: > On Fri, Aug 22, 2014 at 10:27:02AM -0400, Robert Haas wrote: > > On Thu, Aug 21, 2014 at 7:17 PM, Bruce Momjian wrote: > > > On Tue, Mar 18, 2014 at 09:11:46AM -0400, Robert Haas wrote: > > >> On Mon, Mar 17, 2014 at 10:27 PM, Michael Paquier > > >> wrote: > > >> > On Tue, Mar 18, 2014 at 10:24 AM, Fabrízio de Royes Mello > > >> > wrote: > > >> >> > > >> >> On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas > > >> >> wrote: > > >> >>> Well, it's fairly harmless, but it might not be a bad idea to > > >> >>> tighten that > > >> >>> up. > > >> >> The attached patch tighten that up. > > >> > Hm... It might be interesting to include it in 9.4 IMO, somewhat > > >> > grouping with what has been done in a6542a4 for SET and ABORT. > > >> > > >> Meh. There will always be another thing we could squeeze in; I don't > > >> think this is particularly urgent, and it's late to the party. > > > > > > Do we want this patch for 9.5? It throws an error for invalid reloption > > > specifications. > > > > Fine with me. But I have a vague recollection of seeing pg_upgrade > > doing this on purpose to create TOAST tables or something... am I > > misremembering? > > Yes, you remember well. I will have to find a different way for > pg_upgrade to call a no-op ALTER TABLE, which is fine. Looking at the ALTER TABLE options, I am going to put this check in a !IsBinaryUpgrade block so pg_upgrade can still use its trick. -- 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] Is this a bug?
On Fri, Aug 22, 2014 at 10:27:02AM -0400, Robert Haas wrote: > On Thu, Aug 21, 2014 at 7:17 PM, Bruce Momjian wrote: > > On Tue, Mar 18, 2014 at 09:11:46AM -0400, Robert Haas wrote: > >> On Mon, Mar 17, 2014 at 10:27 PM, Michael Paquier > >> wrote: > >> > On Tue, Mar 18, 2014 at 10:24 AM, Fabrízio de Royes Mello > >> > wrote: > >> >> > >> >> On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas > >> >> wrote: > >> >>> Well, it's fairly harmless, but it might not be a bad idea to tighten > >> >>> that > >> >>> up. > >> >> The attached patch tighten that up. > >> > Hm... It might be interesting to include it in 9.4 IMO, somewhat > >> > grouping with what has been done in a6542a4 for SET and ABORT. > >> > >> Meh. There will always be another thing we could squeeze in; I don't > >> think this is particularly urgent, and it's late to the party. > > > > Do we want this patch for 9.5? It throws an error for invalid reloption > > specifications. > > Fine with me. But I have a vague recollection of seeing pg_upgrade > doing this on purpose to create TOAST tables or something... am I > misremembering? Yes, you remember well. I will have to find a different way for pg_upgrade to call a no-op ALTER TABLE, which is fine. -- 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] Is this a bug?
On Thu, Aug 21, 2014 at 7:17 PM, Bruce Momjian wrote: > On Tue, Mar 18, 2014 at 09:11:46AM -0400, Robert Haas wrote: >> On Mon, Mar 17, 2014 at 10:27 PM, Michael Paquier >> wrote: >> > On Tue, Mar 18, 2014 at 10:24 AM, Fabrízio de Royes Mello >> > wrote: >> >> >> >> On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas >> >> wrote: >> >>> Well, it's fairly harmless, but it might not be a bad idea to tighten >> >>> that >> >>> up. >> >> The attached patch tighten that up. >> > Hm... It might be interesting to include it in 9.4 IMO, somewhat >> > grouping with what has been done in a6542a4 for SET and ABORT. >> >> Meh. There will always be another thing we could squeeze in; I don't >> think this is particularly urgent, and it's late to the party. > > Do we want this patch for 9.5? It throws an error for invalid reloption > specifications. Fine with me. But I have a vague recollection of seeing pg_upgrade doing this on purpose to create TOAST tables or something... am I misremembering? -- 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] Is this a bug?
On Tue, Mar 18, 2014 at 09:11:46AM -0400, Robert Haas wrote: > On Mon, Mar 17, 2014 at 10:27 PM, Michael Paquier > wrote: > > On Tue, Mar 18, 2014 at 10:24 AM, Fabrízio de Royes Mello > > wrote: > >> > >> On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas > >> wrote: > >>> Well, it's fairly harmless, but it might not be a bad idea to tighten that > >>> up. > >> The attached patch tighten that up. > > Hm... It might be interesting to include it in 9.4 IMO, somewhat > > grouping with what has been done in a6542a4 for SET and ABORT. > > Meh. There will always be another thing we could squeeze in; I don't > think this is particularly urgent, and it's late to the party. Do we want this patch for 9.5? It throws an error for invalid reloption specifications. -- 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] Is this a bug?
On Mon, Mar 17, 2014 at 10:27 PM, Michael Paquier wrote: > On Tue, Mar 18, 2014 at 10:24 AM, Fabrízio de Royes Mello > wrote: >> >> On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas wrote: >>> Well, it's fairly harmless, but it might not be a bad idea to tighten that >>> up. >> The attached patch tighten that up. > Hm... It might be interesting to include it in 9.4 IMO, somewhat > grouping with what has been done in a6542a4 for SET and ABORT. Meh. There will always be another thing we could squeeze in; I don't think this is particularly urgent, and it's late to the party. -- 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] Is this a bug?
On Tue, Mar 18, 2014 at 10:24 AM, Fabrízio de Royes Mello wrote: > > On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas wrote: >> Well, it's fairly harmless, but it might not be a bad idea to tighten that >> up. > The attached patch tighten that up. Hm... It might be interesting to include it in 9.4 IMO, somewhat grouping with what has been done in a6542a4 for SET and ABORT. -- Michael -- 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] Is this a bug?
On Thu, Mar 13, 2014 at 10:22 AM, Robert Haas wrote:
>
> On Wed, Mar 12, 2014 at 11:11 PM, Fabrízio de Royes Mello
> wrote:
> > Hi all,
> >
> > Shouldn't the "ALTER" statements below raise an exception?
> >
> > fabrizio=# CREATE TABLE foo(bar SERIAL PRIMARY KEY);
> > CREATE TABLE
> >
> > fabrizio=# SELECT relname, reloptions FROM pg_class WHERE relname ~
'^foo';
> >relname | reloptions
> > -+
> > foo |
> > foo_bar_seq |
> > foo_pkey|
> > (3 rows)
> >
> > fabrizio=# ALTER TABLE foo RESET (noname);
> > ALTER TABLE
> >
> > fabrizio=# ALTER INDEX foo_pkey RESET (noname);
> > ALTER INDEX
> >
> > fabrizio=# ALTER TABLE foo ALTER COLUMN bar RESET (noname);
> > ALTER TABLE
> >
> >
> > If I try to "SET" an option called "noname" obviously will raise an
> > exception:
> >
> > fabrizio=# ALTER TABLE foo SET (noname=1);
> > ERROR: unrecognized parameter "noname"
>
> Well, it's fairly harmless, but it might not be a bad idea to tighten
that up.
>
The attached patch tighten that up.
Grettings,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
>> Timbira: http://www.timbira.com.br
>> Blog sobre TI: http://fabriziomello.blogspot.com
>> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello
>> Twitter: http://twitter.com/fabriziomello
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 530a1ae..4a5b767 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -307,6 +307,8 @@ static void initialize_reloptions(void);
static void parse_one_reloption(relopt_value *option, char *text_str,
int text_len, bool validate);
+static bool is_valid_reloption(char *name);
+
/*
* initialize_reloptions
* initialization routine, must be called before parsing
@@ -382,6 +384,25 @@ initialize_reloptions(void)
}
/*
+ * is_valid_reloption
+ * check if a reloption exists
+ *
+ */
+static bool
+is_valid_reloption(char *name)
+{
+ int i;
+
+ for (i = 0; relOpts[i]; i++)
+ {
+ if (pg_strcasecmp(relOpts[i]->name, name) == 0)
+ return true;
+ }
+
+ return false;
+}
+
+/*
* add_reloption_kind
* Create a new relopt_kind value, to be used in custom reloptions by
* user-defined AMs.
@@ -674,6 +695,11 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
if (isReset)
{
+ if (!is_valid_reloption(def->defname))
+ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("unrecognized parameter \"%s\"", def->defname)));
+
if (def->arg != NULL)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 0f0c638..195103e 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -7,6 +7,14 @@ COMMENT ON TABLE tmp_wrong IS 'table comment';
ERROR: relation "tmp_wrong" does not exist
COMMENT ON TABLE tmp IS 'table comment';
COMMENT ON TABLE tmp IS NULL;
+ALTER TABLE tmp SET (fillfactor=70);
+ALTER TABLE tmp RESET (fillfactor, noname);
+ERROR: unrecognized parameter "noname"
+ALTER TABLE tmp RESET (fillfactor=70);
+ERROR: RESET must not include values for parameters
+ALTER TABLE tmp RESET (fillfactor);
+ALTER TABLE tmp RESET (noname);
+ERROR: unrecognized parameter "noname"
ALTER TABLE tmp ADD COLUMN xmin integer; -- fails
ERROR: column name "xmin" conflicts with a system column name
ALTER TABLE tmp ADD COLUMN a int4 default 3;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 87973c1..2cb31f2 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -9,6 +9,16 @@ COMMENT ON TABLE tmp_wrong IS 'table comment';
COMMENT ON TABLE tmp IS 'table comment';
COMMENT ON TABLE tmp IS NULL;
+ALTER TABLE tmp SET (fillfactor=70);
+
+ALTER TABLE tmp RESET (fillfactor, noname);
+
+ALTER TABLE tmp RESET (fillfactor=70);
+
+ALTER TABLE tmp RESET (fillfactor);
+
+ALTER TABLE tmp RESET (noname);
+
ALTER TABLE tmp ADD COLUMN xmin integer; -- fails
ALTER TABLE tmp ADD COLUMN a int4 default 3;
--
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] Is this a bug
fabriziomello wrote > On Thu, Mar 13, 2014 at 10:34 AM, Euler Taveira < > euler@.com > > > wrote: >> >> On 13-03-2014 00:11, Fabrízio de Royes Mello wrote: >> > Shouldn't the "ALTER" statements below raise an exception? >> > >> For consistency, yes. Who cares? I mean, there is no harm in resetting >> an unrecognized parameter. Have in mind that tighten it up could break >> scripts. In general, I'm in favor of validating things. >> > > I know this could break scripts, but I think a consistent behavior should > be raise an exception when an option doesn't exists. > >> euler@euler=# reset noname; >> ERROR: 42704: unrecognized configuration parameter "noname" >> LOCAL: set_config_option, guc.c:5220 >> > > This is a consistent behavior. > > Regards, Probably shouldn't back-patch but a fix and release comment in 9.4 is warranted. Scripts resetting invalid parameters are probably already broken, they just haven't discovered their mistake yet. Do we need an "IF EXISTS" feature on these as well? ;) David J. -- View this message in context: http://postgresql.1045698.n5.nabble.com/Is-this-a-bug-tp5795831p5795943.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] Is this a bug?
On Thu, Mar 13, 2014 at 10:34 AM, Euler Taveira wrote: > > On 13-03-2014 00:11, Fabrízio de Royes Mello wrote: > > Shouldn't the "ALTER" statements below raise an exception? > > > For consistency, yes. Who cares? I mean, there is no harm in resetting > an unrecognized parameter. Have in mind that tighten it up could break > scripts. In general, I'm in favor of validating things. > I know this could break scripts, but I think a consistent behavior should be raise an exception when an option doesn't exists. > euler@euler=# reset noname; > ERROR: 42704: unrecognized configuration parameter "noname" > LOCAL: set_config_option, guc.c:5220 > This is a consistent behavior. Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL >> Timbira: http://www.timbira.com.br >> Blog sobre TI: http://fabriziomello.blogspot.com >> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello >> Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] Is this a bug?
On 13-03-2014 00:11, Fabrízio de Royes Mello wrote: > Shouldn't the "ALTER" statements below raise an exception? > For consistency, yes. Who cares? I mean, there is no harm in resetting an unrecognized parameter. Have in mind that tighten it up could break scripts. In general, I'm in favor of validating things. euler@euler=# reset noname; ERROR: 42704: unrecognized configuration parameter "noname" LOCAL: set_config_option, guc.c:5220 -- Euler Taveira Timbira - http://www.timbira.com.br/ PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento -- 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] Is this a bug?
On Wed, Mar 12, 2014 at 11:11 PM, Fabrízio de Royes Mello wrote: > Hi all, > > Shouldn't the "ALTER" statements below raise an exception? > > fabrizio=# CREATE TABLE foo(bar SERIAL PRIMARY KEY); > CREATE TABLE > > fabrizio=# SELECT relname, reloptions FROM pg_class WHERE relname ~ '^foo'; >relname | reloptions > -+ > foo | > foo_bar_seq | > foo_pkey| > (3 rows) > > fabrizio=# ALTER TABLE foo RESET (noname); > ALTER TABLE > > fabrizio=# ALTER INDEX foo_pkey RESET (noname); > ALTER INDEX > > fabrizio=# ALTER TABLE foo ALTER COLUMN bar RESET (noname); > ALTER TABLE > > > If I try to "SET" an option called "noname" obviously will raise an > exception: > > fabrizio=# ALTER TABLE foo SET (noname=1); > ERROR: unrecognized parameter "noname" Well, it's fairly harmless, but it might not be a bad idea to tighten that up. -- 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
[HACKERS] Is this a bug?
Hi all, Shouldn't the "ALTER" statements below raise an exception? fabrizio=# CREATE TABLE foo(bar SERIAL PRIMARY KEY); CREATE TABLE fabrizio=# SELECT relname, reloptions FROM pg_class WHERE relname ~ '^foo'; relname | reloptions -+ foo | foo_bar_seq | foo_pkey| (3 rows) fabrizio=# ALTER TABLE foo RESET (noname); ALTER TABLE fabrizio=# ALTER INDEX foo_pkey RESET (noname); ALTER INDEX fabrizio=# ALTER TABLE foo ALTER COLUMN bar RESET (noname); ALTER TABLE If I try to "SET" an option called "noname" obviously will raise an exception: fabrizio=# ALTER TABLE foo SET (noname=1); ERROR: unrecognized parameter "noname" fabrizio=# ALTER INDEX foo_pkey SET (noname=1); ERROR: unrecognized parameter "noname" fabrizio=# ALTER TABLE foo ALTER COLUMN bar SET (noname=1); ERROR: unrecognized parameter "noname" Regards, -- Fabrízio de Royes Mello Consultoria/Coaching PostgreSQL >> Timbira: http://www.timbira.com.br >> Blog sobre TI: http://fabriziomello.blogspot.com >> Perfil Linkedin: http://br.linkedin.com/in/fabriziomello >> Twitter: http://twitter.com/fabriziomello
Re: [HACKERS] is this a bug?
"David E. Wheeler" writes:
> On Jan 17, 2010, at 3:47 PM, Tom Lane wrote:
>>> create type y as (c char, n int);
>>> select ('a', NULL)::y = ('a', NULL)::y; -- TRUE
>>> select ('a', NULL) = ('a', NULL); -- NULL
>> The latter gets simplified to ('a' = 'a') AND (NULL = NULL).
>> The former doesn't --- it goes through record_eq, which treats
>> two nulls as equal.
> Shouldn't this go through record_eq, then?
> try=# select row('a', NULL) = row('a', NULL);
No, the ROW keyword is just noise. It's the cast that is preventing the
expansion. We could possibly change things so that it got expanded out
even with the cast, but on the whole I'm not sure that would be an
improvement. It doesn't make things consistent, it just shifts the
boundary of inconsistency ...
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] is this a bug?
On Jan 17, 2010, at 3:47 PM, Tom Lane wrote:
>> create type y as (c char, n int);
>> select ('a', NULL)::y = ('a', NULL)::y; -- TRUE
>> select ('a', NULL) = ('a', NULL); -- NULL
>
>> I would expect those to evaluate to the same thing.
>
> The latter gets simplified to ('a' = 'a') AND (NULL = NULL).
> The former doesn't --- it goes through record_eq, which treats
> two nulls as equal.
Shouldn't this go through record_eq, then?
try=# select row('a', NULL) = row('a', NULL);
?column?
--
[null]
Best,
David
--
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] is this a bug?
On Sun, 2010-01-17 at 18:47 -0500, Tom Lane wrote: > The former might be closer to the spec's expectations but I'm not > totally sure about it. I suppose that people using NULLs should expect the unexpected ;) I don't have strong feelings about it, I just wanted to raise the issue. Regards, Jeff Davis -- 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] is this a bug?
Jeff Davis writes:
> create type y as (c char, n int);
> select ('a', NULL)::y = ('a', NULL)::y; -- TRUE
> select ('a', NULL) = ('a', NULL); -- NULL
> I would expect those to evaluate to the same thing.
The latter gets simplified to ('a' = 'a') AND (NULL = NULL).
The former doesn't --- it goes through record_eq, which treats
two nulls as equal.
The reason record_eq does that is that we have to have a total ordering
in order for record types to be indexable or sortable.
The former might be closer to the spec's expectations but I'm not
totally sure about it.
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
[HACKERS] is this a bug?
create type y as (c char, n int);
select ('a', NULL)::y = ('a', NULL)::y; -- TRUE
select ('a', NULL) = ('a', NULL); -- NULL
I would expect those to evaluate to the same thing.
Regards,
Jeff Davis
--
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] Is this a bug?
"Matthew T. O'Connor" writes: > Now, I know I can specify a constraint name inside the alter command, > but I still expected this to work. It does, in 8.0. regression=# create table foo (id1 int, id2 int, id3 int); CREATE TABLE regression=# ALTER TABLE foo ADD unique (id1,id2); NOTICE: ALTER TABLE / ADD UNIQUE will create implicit index "foo_id1_key" for table "foo" ALTER TABLE regression=# ALTER TABLE foo ADD unique (id1,id3); NOTICE: ALTER TABLE / ADD UNIQUE will create implicit index "foo_id1_key1" for table "foo" ALTER TABLE regression=# Of course there's no free lunch: it's significantly harder to guess what the index name will be... regards, tom lane ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] Is this a bug?
On Tue, Jan 25, 2005 at 12:43:16PM -0500, Matthew T. O'Connor wrote: > foo=# ALTER TABLE foo ADD unique (id1,id3); > NOTICE: ALTER TABLE / ADD UNIQUE will create implicit index > "foo_id1_key" for table "foo" > ERROR: relation "foo_id1_key" already exists 8.0.0 handles this situation better: test=> create table foo (id1 int, id2 int, id3 int); CREATE TABLE test=> ALTER TABLE foo ADD unique (id1,id2); NOTICE: ALTER TABLE / ADD UNIQUE will create implicit index "foo_id1_key" for table "foo" ALTER TABLE test=> ALTER TABLE foo ADD unique (id1,id3); NOTICE: ALTER TABLE / ADD UNIQUE will create implicit index "foo_id1_key1" for table "foo" ALTER TABLE -- Michael Fuhr http://www.fuhr.org/~mfuhr/ ---(end of broadcast)--- TIP 9: the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[HACKERS] Is this a bug?
I think this may have been discussed before but I found this a bit surprising: foo=# SELECT version(); version - PostgreSQL 7.4.3 on i686-pc-linux-gnu, compiled by GCC gcc (GCC) 3.3.2 20031022 (Red Hat Linux 3.3.2-1) (1 row) foo=# create table foo (id1 int, id2 int, id3 int); CREATE TABLE foo=# ALTER TABLE foo ADD unique (id1,id2); NOTICE: ALTER TABLE / ADD UNIQUE will create implicit index "foo_id1_key" for table "foo" ALTER TABLE foo=# ALTER TABLE foo ADD unique (id1,id3); NOTICE: ALTER TABLE / ADD UNIQUE will create implicit index "foo_id1_key" for table "foo" ERROR: relation "foo_id1_key" already exists foo=# Now, I know I can specify a constraint name inside the alter command, but I still expected this to work. Thanks, Matthew ---(end of broadcast)--- TIP 7: don't forget to increase your free space map settings
Re: [HACKERS] Is this a bug?
"Roberto Abalde" <[EMAIL PROTECTED]> writes: > I've found that some two functions in /src/backend/optimizer/plan/planner.c > have side effects. No kidding ;-). The planner is full of side-effects on data structures. Both of the changes you mention are intentional. regards, tom lane ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/users-lounge/docs/faq.html
[HACKERS] Is this a bug?
Hi people,
I've found that some two functions in /src/backend/optimizer/plan/planner.c
have side effects.
First, I've added two pprints before and after line 89-90 like this.
pprint(parse->rtable);
/* primary planning entry point (may recurse for subqueries) */
result_plan = subquery_planner(parse, -1.0 /* default case */);
pprint(parse->rtable);
Then I ran the query "select * from F1 where fk6 > 50;" and get this on the
log (I put "<<" to highlite the differences)
DEBUG: query: select * from F1 where fk6 > 50;
(
{ RTE
:relname f1
:relid 782787
:subquery <>
:alias <>
:eref
{ ATTR
:relname f1
:attrs ( "pk" "fk1" "fk2" "fk3" "fk4" "fk5" "fk6" "valu
e" )
}
:inh true <<
:inFromCl true
:checkForRead true
:checkForWrite false
:checkAsUser 0
}
)
(
{ RTE
:relname f1
:relid 782787
:subquery <>
:alias <>
:eref
{ ATTR
:relname f1
:attrs ( "pk" "fk1" "fk2" "fk3" "fk4" "fk5" "fk6" "valu
e" )
}
:inh false <<
:inFromCl true
:checkForRead true
:checkForWrite false
:checkAsUser 0
}
)
So, parse->rtable->inh changes. In the same way I've tested
"set_plan_references" (lines 97-98)
pprint(parse->jointree);
/* final cleanup of the plan */
set_plan_references(result_plan);
pprint(parse->jointree);
Again I ran the query "select * from F1 where fk6 > 50;" and get this on the
log (I put "<<" to highlite the differences)
DEBUG: query: select * from F1 where fk6 > 50;
{ FROMEXPR
:fromlist (
{ RANGETBLREF 1
}
)
:quals (
{ EXPR
:typeOid 16
:opType op
:oper
{ OPER
:opno 521
:opid 0 <
:opresulttype 16
}
...
{ FROMEXPR
:fromlist (
{ RANGETBLREF 1
}
)
:quals (
{ EXPR
:typeOid 16
:opType op
:oper
{ OPER
:opno 521
:opid 147 <
:opresulttype 16
}
Now, parse->jointre->opid changes. The odd thing is that
"set_plan_references" does not have "parse" as a parameter (!).
I need help to check this again to see if these are actual bugs. Can someone
help me with this? BTW, I'm using the 7.1.1 sources.
Saludos,
Roberto
-
A "No" uttered from deepest conviction is better and greater than a "Yes"
merely uttered to please, or what is worse, to avoid trouble. -- Mahatma
Gandhi
---(end of broadcast)---
TIP 2: you can get off all lists at once with the unregister command
(send "unregister YourEmailAddressHere" to [EMAIL PROTECTED])
[HACKERS] Is this a bug in 7.0.2
Dear all, I've migrated from RedHat6.2/PHP3.0/PostgreSQL6.5 to Mandrake/PHP4.0/Postgres7.0.2 successfully as far as pg_dump database_name is concerned. I am still running BOTH versions on two computers. PostgreSQL6.5 does not produce any error using math function "integer (float_expression)" (even "int(float_expression" is OK with 6.5) and 7.0.2 complains saying: "function integer(float8) is not found for specified types" The query is used in something like this: SELECT attr-1, attr-2, ... attr-k, integer(attr-2 * $php_variable_1 + atrr-3 * $php_variable_2) AS attr-N FROM table_1, table_2 WHERE clause_1 AND clause_2 ORDER BY attr-2, attr-4 If I made an error HERE with the sintax, I apologise - I am not close to my both Linux machines, and sintax is OK in my real SELECT queries. "$php_xxx"s are PHP variables and "attr-xxx"s are SQL table attributes. The "integer(float_expression)" is more complicated then written above BUT it all DOES work in 6.5 and DOES NOT in 7.0.2. Any ideas, please? Much obliged, Steven. -- *** Steven Vajdic (BSc/Hon, MSc) Senior Software Engineer Motorola Australia Software Centre (MASC) 2 Second Avenue, Technology Park Adelaide, South Australia 5095 email: [EMAIL PROTECTED] email: [EMAIL PROTECTED] Ph.: +61-8-8168-3435 Fax:+61-8-8168-3501 Front Office (Ph): +61-8-8168-3500 mobile: +61 (0)419 860 903 AFTER WORK email: [EMAIL PROTECTED] *** ---(end of broadcast)--- TIP 6: Have you searched our list archives? http://www.postgresql.org/search.mpl
