Re: [HACKERS] Declarative partitioning vs. sql_inheritance
Robert Haas writes: > On Fri, Dec 23, 2016 at 11:59 AM, Tom Lane wrote: >> Do you have any particular objection to taking the next step of removing >> enum InhOption in favor of making inhOpt a bool? > No, not really. I don't feel like it's an improvement, but you and > Alvaro obviously do, so have at it. OK, will do. 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] Declarative partitioning vs. sql_inheritance
On Fri, Dec 23, 2016 at 11:59 AM, Tom Lane wrote: > Robert Haas writes: >> Great, committed. I realize just now that I forgot to credit anyone >> as a reviewer, but hopefully nobody's going to mind that too much >> considering this is a purely mechanical patch I wrote in 20 minutes. > > Do you have any particular objection to taking the next step of removing > enum InhOption in favor of making inhOpt a bool? It seems to me that > stuff like > > - boolrecurse = interpretInhOption(rv->inhOpt); > + boolrecurse = (rv->inhOpt == INH_YES); > > just begs the question of why it's not simply > > boolrecurse = rv->inh; > > Certainly a reader who did not know the history would be confused at > the useless-looking complexity. No, not really. I don't feel like it's an improvement, but you and Alvaro obviously do, so have at it. -- 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] Declarative partitioning vs. sql_inheritance
Robert Haas writes: > Great, committed. I realize just now that I forgot to credit anyone > as a reviewer, but hopefully nobody's going to mind that too much > considering this is a purely mechanical patch I wrote in 20 minutes. Do you have any particular objection to taking the next step of removing enum InhOption in favor of making inhOpt a bool? It seems to me that stuff like - boolrecurse = interpretInhOption(rv->inhOpt); + boolrecurse = (rv->inhOpt == INH_YES); just begs the question of why it's not simply boolrecurse = rv->inh; Certainly a reader who did not know the history would be confused at the useless-looking complexity. 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] Declarative partitioning vs. sql_inheritance
On Mon, Dec 19, 2016 at 12:25 AM, Amit Langote wrote: >> I agree. Patch attached, just removing the GUC and a fairly minimal >> amount of the supporting infrastructure. > > +1 to removing the sql_inheritance GUC. The patch looks good to me. Great, committed. I realize just now that I forgot to credit anyone as a reviewer, but hopefully nobody's going to mind that too much considering this is a purely mechanical patch I wrote in 20 minutes. -- 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] Declarative partitioning vs. sql_inheritance
On Mon, Dec 19, 2016 at 11:48 AM, Alvaro Herrera wrote: > Any particular reason not to change inhOpt to be a simple boolean, and > remove the enum? No, no particular reason. I thought about it, but I didn't really see any advantage in getting rid of the typedef. -- 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] Declarative partitioning vs. sql_inheritance
Robert Haas wrote: > On Fri, Dec 16, 2016 at 7:39 PM, Tom Lane wrote: > > Peter Eisentraut writes: > >> On 12/16/16 11:05 AM, Robert Haas wrote: > >>> If we were going to do anything about this, > >>> my vote would be to remove sql_inheritance. > > > >> Go for it. > > > >> Let's also remove the table* syntax then. > > > > Meh --- that might break existing queries, to what purpose? > > > > We certainly shouldn't remove query syntax without a deprecation period. > > I'm less concerned about that for GUCs. > > I agree. Patch attached, just removing the GUC and a fairly minimal > amount of the supporting infrastructure. Any particular reason not to change inhOpt to be a simple boolean, and remove the enum? -- Álvaro Herrerahttps://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] Declarative partitioning vs. sql_inheritance
On 2016/12/17 10:40, Robert Haas wrote: > On Fri, Dec 16, 2016 at 7:39 PM, Tom Lane wrote: >> Peter Eisentraut writes: >>> On 12/16/16 11:05 AM, Robert Haas wrote: If we were going to do anything about this, my vote would be to remove sql_inheritance. >> >>> Go for it. >> >>> Let's also remove the table* syntax then. >> >> Meh --- that might break existing queries, to what purpose? >> >> We certainly shouldn't remove query syntax without a deprecation period. >> I'm less concerned about that for GUCs. > > I agree. Patch attached, just removing the GUC and a fairly minimal > amount of the supporting infrastructure. +1 to removing the sql_inheritance GUC. The patch looks good to me. Thanks, Amit -- 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] Declarative partitioning vs. sql_inheritance
On Fri, Dec 16, 2016 at 7:39 PM, Tom Lane wrote: > Peter Eisentraut writes: >> On 12/16/16 11:05 AM, Robert Haas wrote: >>> If we were going to do anything about this, >>> my vote would be to remove sql_inheritance. > >> Go for it. > >> Let's also remove the table* syntax then. > > Meh --- that might break existing queries, to what purpose? > > We certainly shouldn't remove query syntax without a deprecation period. > I'm less concerned about that for GUCs. I agree. Patch attached, just removing the GUC and a fairly minimal amount of the supporting infrastructure. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 0fc4e57..605910f 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -7366,36 +7366,6 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir' - - sql_inheritance (boolean) - - sql_inheritance configuration parameter - - inheritance - - - -This setting controls whether undecorated table references are -considered to include inheritance child tables. The default is -on, which means child tables are included (thus, -a * suffix is assumed by default). If turned -off, child tables are not included (thus, an -ONLY prefix is assumed). The SQL standard -requires child tables to be included, so the off setting -is not spec-compliant, but it is provided for compatibility with -PostgreSQL releases prior to 7.1. -See for more information. - - - -Turning sql_inheritance off is deprecated, because that -behavior has been found to be error-prone as well as contrary to SQL -standard. Discussions of inheritance behavior elsewhere in this -manual generally assume that it is on. - - - - standard_conforming_strings (boolean) stringsstandard conforming diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml index 7e1bc0e..d7117cb 100644 --- a/doc/src/sgml/ddl.sgml +++ b/doc/src/sgml/ddl.sgml @@ -2529,11 +2529,9 @@ SELECT name, altitude WHERE altitude > 500; - Writing * is not necessary, since this behavior is - the default (unless you have changed the setting of the -configuration option). - However writing * might be useful to emphasize that - additional tables will be searched. + Writing * is not necessary, since this behavior is always + the default. However, this syntax is still supported for + compatibility with older releases where the default could be changed. diff --git a/doc/src/sgml/queries.sgml b/doc/src/sgml/queries.sgml index 5cc6dbc..0f84c12 100644 --- a/doc/src/sgml/queries.sgml +++ b/doc/src/sgml/queries.sgml @@ -145,11 +145,9 @@ FROM table_reference , table_r Instead of writing ONLY before the table name, you can write * after the table name to explicitly specify that descendant -tables are included. Writing * is not necessary since that -behavior is the default (unless you have changed the setting of the configuration option). However writing -* might be useful to emphasize that additional tables will be -searched. +tables are included. There is no real reason to use this syntax any more, +because searching descendent tables is now always the default behavior. +However, it is supported for compatibility with older releases. diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c index 9e62e00..ba1414b 100644 --- a/src/backend/commands/lockcmds.c +++ b/src/backend/commands/lockcmds.c @@ -54,7 +54,7 @@ LockTableCommand(LockStmt *lockstmt) foreach(p, lockstmt->relations) { RangeVar *rv = (RangeVar *) lfirst(p); - bool recurse = interpretInhOption(rv->inhOpt); + bool recurse = (rv->inhOpt == INH_YES); Oid reloid; reloid = RangeVarGetRelidExtended(rv, lockstmt->mode, false, diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 7a574dc..075b68b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1183,7 +1183,7 @@ ExecuteTruncate(TruncateStmt *stmt) { RangeVar *rv = lfirst(cell); Relation rel; - bool recurse = interpretInhOption(rv->inhOpt); + bool recurse = (rv->inhOpt == INH_YES); Oid myrelid; rel = heap_openrv(rv, AccessExclusiveLock); @@ -2654,7 +2654,7 @@ renameatt(RenameStmt *stmt) renameatt_internal(relid, stmt->subname, /* old att name */ stmt->newname, /* new att name */ - interpretInhOption(stmt->relation->inhOpt), /* recursive? */ + (stmt->relation->inhOpt == INH_YES), /* recursive? */ false, /* recursing? */ 0, /* expected inhcount */ stmt->behavior); @@ -2806,7 +2806,7 @@ RenameConstraint(RenameStmt *
Re: [HACKERS] Declarative partitioning vs. sql_inheritance
Peter Eisentraut writes: > On 12/16/16 11:05 AM, Robert Haas wrote: >> If we were going to do anything about this, >> my vote would be to remove sql_inheritance. > Go for it. > Let's also remove the table* syntax then. Meh --- that might break existing queries, to what purpose? We certainly shouldn't remove query syntax without a deprecation period. I'm less concerned about that for GUCs. 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] Declarative partitioning vs. sql_inheritance
On 12/16/16 11:05 AM, Robert Haas wrote: > If we were going to do anything about this, > my vote would be to remove sql_inheritance. +1. This option is long past the intended shelf life. -- -David da...@pgmasters.net -- 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] Declarative partitioning vs. sql_inheritance
On 12/16/16 11:05 AM, Robert Haas wrote: > If we were going to do anything about this, > my vote would be to remove sql_inheritance. Go for it. Let's also remove the table* syntax then. -- Peter Eisentraut http://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] Declarative partitioning vs. sql_inheritance
On Fri, Dec 16, 2016 at 2:34 PM, David Fetter wrote: > It occurs to me this probably isn't the only GUC that's basically just > a foot gun at this point. > > Is 10 a good time to sweep and clear them? We never make any progress trying to do these things "in bulk". If you think there are other GUCs that need to be removed, start a thread for each one, or closely related group, and let's talk about it on the merits. On this thread, let's just decide whether or not to remove sql_inheritance. -- 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] Declarative partitioning vs. sql_inheritance
On Fri, Dec 16, 2016 at 11:05:21AM -0500, Robert Haas wrote: > On Thu, Dec 15, 2016 at 10:40 AM, Dmitry Ivanov > wrote: > > Hi everyone, > > > > Looks like "sql_inheritance" GUC is affecting partitioned tables: > > > > [breaks literally everything] > > > > I might be wrong, but IMO this should not happen. Queries involving update, > > delete etc on partitioned tables are basically broken. Moreover, there's no > > point in performing such operations on a parent table that's supposed to be > > empty at all times. > > An earlier version of Amit's patches tried to handle this by forcing > sql_inheritance on for partitioned tables, but it wasn't > well-implemented and I don't see the point anyway. Sure, turning > off sql_inheritance off for partitioned tables produces stupid > results. But turning off sql_inheritance for inheritance > hierarchies also produces stupid results. If we were going to do > anything about this, my vote would be to remove sql_inheritance. +1 It occurs to me this probably isn't the only GUC that's basically just a foot gun at this point. Is 10 a good time to sweep and clear them? Best, David. -- David Fetter http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Declarative partitioning vs. sql_inheritance
Robert Haas writes: > An earlier version of Amit's patches tried to handle this by forcing > sql_inheritance on for partitioned tables, but it wasn't > well-implemented and I don't see the point anyway. Sure, turning off > sql_inheritance off for partitioned tables produces stupid results. > But turning off sql_inheritance for inheritance hierarchies also > produces stupid results. If we were going to do anything about this, > my vote would be to remove sql_inheritance. +1. If memory serves, we invented that GUC as a backwards-compatibility hack, because once upon a time the default behavior was equivalent to sql_inheritance = off. But that was a long time ago; a bit of digging in the git history suggests we changed it in 2000. It's hard to believe that anybody still relies on being able to turn it off. 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] Declarative partitioning vs. sql_inheritance
On Thu, Dec 15, 2016 at 10:40 AM, Dmitry Ivanov wrote: > Hi everyone, > > Looks like "sql_inheritance" GUC is affecting partitioned tables: > > explain (costs off) select * from test; > QUERY PLAN -- > Append > -> Seq Scan on test > -> Seq Scan on test_1 > -> Seq Scan on test_2 > -> Seq Scan on test_1_1 > -> Seq Scan on test_1_2 > -> Seq Scan on test_1_1_1 > -> Seq Scan on test_1_2_1 > (8 rows) > > > set sql_inheritance = off; > > > explain (costs off) select * from test; >QUERY PLAN-- > Seq Scan on test > (1 row) > > > I might be wrong, but IMO this should not happen. Queries involving update, > delete etc on partitioned tables are basically broken. Moreover, there's no > point in performing such operations on a parent table that's supposed to be > empty at all times. An earlier version of Amit's patches tried to handle this by forcing sql_inheritance on for partitioned tables, but it wasn't well-implemented and I don't see the point anyway. Sure, turning off sql_inheritance off for partitioned tables produces stupid results. But turning off sql_inheritance for inheritance hierarchies also produces stupid results. If we were going to do anything about this, my vote would be to remove sql_inheritance. -- 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