On Fri, Dec 16, 2016 at 7:39 PM, Tom Lane <t...@sss.pgh.pa.us> wrote:
> Peter Eisentraut <peter.eisentr...@2ndquadrant.com> 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'
       </listitem>
      </varlistentry>
 
-     <varlistentry id="guc-sql-inheritance" xreflabel="sql_inheritance">
-      <term><varname>sql_inheritance</varname> (<type>boolean</type>)
-      <indexterm>
-       <primary><varname>sql_inheritance</> configuration parameter</primary>
-      </indexterm>
-      <indexterm><primary>inheritance</></>
-      </term>
-      <listitem>
-       <para>
-        This setting controls whether undecorated table references are
-        considered to include inheritance child tables.  The default is
-        <literal>on</>, which means child tables are included (thus,
-        a <literal>*</> suffix is assumed by default).  If turned
-        <literal>off</>, child tables are not included (thus, an
-        <literal>ONLY</literal> prefix is assumed).  The SQL standard
-        requires child tables to be included, so the <literal>off</> setting
-        is not spec-compliant, but it is provided for compatibility with
-        <productname>PostgreSQL</> releases prior to 7.1.
-        See <xref linkend="ddl-inherit"> for more information.
-       </para>
-
-       <para>
-        Turning <varname>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 <literal>on</>.
-       </para>
-      </listitem>
-     </varlistentry>
-
      <varlistentry id="guc-standard-conforming-strings" xreflabel="standard_conforming_strings">
       <term><varname>standard_conforming_strings</varname> (<type>boolean</type>)
       <indexterm><primary>strings</><secondary>standard 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 &gt; 500;
 </programlisting>
 
-   Writing <literal>*</> is not necessary, since this behavior is
-   the default (unless you have changed the setting of the
-   <xref linkend="guc-sql-inheritance"> configuration option).
-   However writing <literal>*</> might be useful to emphasize that
-   additional tables will be searched.
+   Writing <literal>*</> 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.
   </para>
 
   <para>
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 <replaceable>table_reference</replaceable> <optional>, <replaceable>table_r
    <para>
     Instead of writing <literal>ONLY</> before the table name, you can write
     <literal>*</> after the table name to explicitly specify that descendant
-    tables are included.  Writing <literal>*</> is not necessary since that
-    behavior is the default (unless you have changed the setting of the <xref
-    linkend="guc-sql-inheritance"> configuration option).  However writing
-    <literal>*</> 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.
    </para>
 
    <sect3 id="queries-join">
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 *stmt)
 		rename_constraint_internal(relid, typid,
 								   stmt->subname,
 								   stmt->newname,
-		 stmt->relation ? interpretInhOption(stmt->relation->inhOpt) : false,	/* recursive? */
+		 (stmt->relation && stmt->relation->inhOpt == INH_YES),	/* recursive? */
 								   false,		/* recursing? */
 								   0 /* expected inhcount */ );
 
@@ -3049,7 +3049,7 @@ AlterTable(Oid relid, LOCKMODE lockmode, AlterTableStmt *stmt)
 	CheckTableNotInUse(rel, "ALTER TABLE");
 
 	ATController(stmt,
-				 rel, stmt->cmds, interpretInhOption(stmt->relation->inhOpt),
+				 rel, stmt->cmds, (stmt->relation->inhOpt == INH_YES),
 				 lockmode);
 }
 
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index 20e2dbd..b64f7c6 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -423,7 +423,7 @@ makeRangeVar(char *schemaname, char *relname, int location)
 	r->catalogname = NULL;
 	r->schemaname = schemaname;
 	r->relname = relname;
-	r->inhOpt = INH_DEFAULT;
+	r->inhOpt = INH_YES;
 	r->relpersistence = RELPERSISTENCE_PERMANENT;
 	r->alias = NULL;
 	r->location = location;
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 5e65fe7..601e22a 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -380,7 +380,7 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt)
 
 	/* set up range table with just the result rel */
 	qry->resultRelation = setTargetTable(pstate, stmt->relation,
-								  interpretInhOption(stmt->relation->inhOpt),
+								  (stmt->relation->inhOpt == INH_YES),
 										 true,
 										 ACL_DELETE);
 
@@ -2177,7 +2177,7 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt)
 	}
 
 	qry->resultRelation = setTargetTable(pstate, stmt->relation,
-								  interpretInhOption(stmt->relation->inhOpt),
+								  (stmt->relation->inhOpt == INH_YES),
 										 true,
 										 ACL_UPDATE);
 
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 2ed7b52..931bc9a 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -28,12 +28,11 @@
  *	  current transaction and are just parsing commands to find the next
  *	  ROLLBACK or COMMIT.  If you make use of SET variables, then you
  *	  will do the wrong thing in multi-query strings like this:
- *			SET SQL_inheritance TO off; SELECT * FROM foo;
+ *			SET constraint_exclusion TO off; SELECT * FROM foo;
  *	  because the entire string is parsed by gram.y before the SET gets
  *	  executed.  Anything that depends on the database or changeable state
  *	  should be handled during parse analysis so that it happens at the
- *	  right time not the wrong time.  The handling of SQL_inheritance is
- *	  a good example.
+ *	  right time not the wrong time.
  *
  * WARNINGS
  *	  If you use a list, make sure the datum is a node so that the printing
@@ -11249,9 +11248,9 @@ join_qual:	USING '(' name_list ')'					{ $$ = (Node *) $3; }
 relation_expr:
 			qualified_name
 				{
-					/* default inheritance */
+					/* inheritance query, implicitly */
 					$$ = $1;
-					$$->inhOpt = INH_DEFAULT;
+					$$->inhOpt = INH_YES;
 					$$->alias = NULL;
 				}
 			| qualified_name '*'
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 751de4b..a96b3f9 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -229,30 +229,6 @@ setTargetTable(ParseState *pstate, RangeVar *relation,
 }
 
 /*
- * Simplify InhOption (yes/no/default) into boolean yes/no.
- *
- * The reason we do things this way is that we don't want to examine the
- * SQL_inheritance option flag until parse_analyze() is run.    Otherwise,
- * we'd do the wrong thing with query strings that intermix SET commands
- * with queries.
- */
-bool
-interpretInhOption(InhOption inhOpt)
-{
-	switch (inhOpt)
-	{
-		case INH_NO:
-			return false;
-		case INH_YES:
-			return true;
-		case INH_DEFAULT:
-			return SQL_inheritance;
-	}
-	elog(ERROR, "bogus InhOption value: %d", inhOpt);
-	return false;				/* keep compiler quiet */
-}
-
-/*
  * Given a relation-options list (of DefElems), return true iff the specified
  * table/result set should be created with OIDs. This needs to be done after
  * parsing the query string because the return value can depend upon the
@@ -437,7 +413,7 @@ transformTableEntry(ParseState *pstate, RangeVar *r)
 
 	/* We need only build a range table entry */
 	rte = addRangeTableEntry(pstate, r, r->alias,
-							 interpretInhOption(r->inhOpt), true);
+							 (r->inhOpt == INH_YES), true);
 
 	return rte;
 }
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index a025117..946ba9e 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -440,7 +440,6 @@ char	   *event_source;
 bool		row_security;
 bool		check_function_bodies = true;
 bool		default_with_oids = false;
-bool		SQL_inheritance = true;
 
 int			log_min_error_statement = ERROR;
 int			log_min_messages = WARNING;
@@ -1322,15 +1321,6 @@ static struct config_bool ConfigureNamesBool[] =
 		NULL, NULL, NULL
 	},
 	{
-		{"sql_inheritance", PGC_USERSET, COMPAT_OPTIONS_PREVIOUS,
-			gettext_noop("Causes subtables to be included by default in various commands."),
-			NULL
-		},
-		&SQL_inheritance,
-		true,
-		NULL, NULL, NULL
-	},
-	{
 		{"transform_null_equals", PGC_USERSET, COMPAT_OPTIONS_CLIENT,
 			gettext_noop("Treats \"expr=NULL\" as \"expr IS NULL\"."),
 			gettext_noop("When turned on, expressions of the form expr = NULL "
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 7f9acfd..46a7c17 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -605,7 +605,6 @@
 #lo_compat_privileges = off
 #operator_precedence_warning = off
 #quote_all_identifiers = off
-#sql_inheritance = on
 #standard_conforming_strings = on
 #synchronize_seqscans = on
 
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 65510b0..d11f112 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -45,8 +45,7 @@ typedef struct Alias
 typedef enum InhOption
 {
 	INH_NO,						/* Do NOT scan child tables */
-	INH_YES,					/* DO scan child tables */
-	INH_DEFAULT					/* Use current SQL_inheritance option */
+	INH_YES						/* DO scan child tables */
 } InhOption;
 
 /* What to do at commit time for temporary relations */
diff --git a/src/include/parser/parse_clause.h b/src/include/parser/parse_clause.h
index d04ce11..549bde4 100644
--- a/src/include/parser/parse_clause.h
+++ b/src/include/parser/parse_clause.h
@@ -19,7 +19,6 @@
 extern void transformFromClause(ParseState *pstate, List *frmList);
 extern int setTargetTable(ParseState *pstate, RangeVar *relation,
 			   bool inh, bool alsoSource, AclMode requiredPerms);
-extern bool interpretInhOption(InhOption inhOpt);
 extern bool interpretOidsOption(List *defList, bool allowOids);
 
 extern Node *transformWhereClause(ParseState *pstate, Node *clause,
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 0bf9f21..66a3915 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -244,7 +244,6 @@ extern bool log_btree_build_stats;
 
 extern PGDLLIMPORT bool check_function_bodies;
 extern bool default_with_oids;
-extern bool SQL_inheritance;
 
 extern int	log_min_error_statement;
 extern int	log_min_messages;
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to