Tom Lane <t...@sss.pgh.pa.us> wrote:

> Shouldn't attribute_used() be removed from rewriteManip.h?

Yeah, I don't know how I missed that.  Thanks.

> I was a bit surprised by your removal of the
> rangeTableEntry_used() test in the hunk at
> rewriteHandler.c:1273ff.  That's probably all right, but it takes
> this out of the realm of a mechanical change.

[ also questioned by Álvaro ]

I'll leave that as it was -- it can be discussed separately from
the mechanical changes.

New patch attached.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
*** a/doc/src/sgml/catalogs.sgml
--- b/doc/src/sgml/catalogs.sgml
***************
*** 5039,5052 ****
       </row>
  
       <row>
-       <entry><structfield>ev_attr</structfield></entry>
-       <entry><type>int2</type></entry>
-       <entry></entry>
-       <entry>The column this rule is for (currently, always -1 to
-       indicate the whole table)</entry>
-      </row>
- 
-      <row>
        <entry><structfield>ev_type</structfield></entry>
        <entry><type>char</type></entry>
        <entry></entry>
--- 5039,5044 ----
*** a/src/backend/rewrite/rewriteDefine.c
--- b/src/backend/rewrite/rewriteDefine.c
***************
*** 58,64 **** static Oid
  InsertRule(char *rulname,
  		   int evtype,
  		   Oid eventrel_oid,
- 		   AttrNumber evslot_index,
  		   bool evinstead,
  		   Node *event_qual,
  		   List *action,
--- 58,63 ----
***************
*** 86,92 **** InsertRule(char *rulname,
  	namestrcpy(&rname, rulname);
  	values[Anum_pg_rewrite_rulename - 1] = NameGetDatum(&rname);
  	values[Anum_pg_rewrite_ev_class - 1] = ObjectIdGetDatum(eventrel_oid);
- 	values[Anum_pg_rewrite_ev_attr - 1] = Int16GetDatum(evslot_index);
  	values[Anum_pg_rewrite_ev_type - 1] = CharGetDatum(evtype + '0');
  	values[Anum_pg_rewrite_ev_enabled - 1] = CharGetDatum(RULE_FIRES_ON_ORIGIN);
  	values[Anum_pg_rewrite_is_instead - 1] = BoolGetDatum(evinstead);
--- 85,90 ----
***************
*** 117,123 **** InsertRule(char *rulname,
  		 * When replacing, we don't need to replace every attribute
  		 */
  		MemSet(replaces, false, sizeof(replaces));
- 		replaces[Anum_pg_rewrite_ev_attr - 1] = true;
  		replaces[Anum_pg_rewrite_ev_type - 1] = true;
  		replaces[Anum_pg_rewrite_is_instead - 1] = true;
  		replaces[Anum_pg_rewrite_ev_qual - 1] = true;
--- 115,120 ----
***************
*** 238,244 **** DefineQueryRewrite(char *rulename,
  				   List *action)
  {
  	Relation	event_relation;
- 	int			event_attno;
  	ListCell   *l;
  	Query	   *query;
  	bool		RelisBecomingView = false;
--- 235,240 ----
***************
*** 495,501 **** DefineQueryRewrite(char *rulename,
  	/*
  	 * This rule is allowed - prepare to install it.
  	 */
- 	event_attno = -1;
  
  	/* discard rule if it's null action and not INSTEAD; it's a no-op */
  	if (action != NIL || is_instead)
--- 491,496 ----
***************
*** 503,509 **** DefineQueryRewrite(char *rulename,
  		ruleId = InsertRule(rulename,
  							event_type,
  							event_relid,
- 							event_attno,
  							is_instead,
  							event_qual,
  							action,
--- 498,503 ----
*** a/src/backend/rewrite/rewriteHandler.c
--- b/src/backend/rewrite/rewriteHandler.c
***************
*** 1276,1285 **** matchLocks(CmdType event,
  		if (oneLock->event == event)
  		{
  			if (parsetree->commandType != CMD_SELECT ||
! 				(oneLock->attrno == -1 ?
! 				 rangeTableEntry_used((Node *) parsetree, varno, 0) :
! 				 attribute_used((Node *) parsetree,
! 								varno, oneLock->attrno, 0)))
  				matching_locks = lappend(matching_locks, oneLock);
  		}
  	}
--- 1276,1282 ----
  		if (oneLock->event == event)
  		{
  			if (parsetree->commandType != CMD_SELECT ||
! 				rangeTableEntry_used((Node *) parsetree, varno, 0))
  				matching_locks = lappend(matching_locks, oneLock);
  		}
  	}
***************
*** 1295,1301 **** static Query *
  ApplyRetrieveRule(Query *parsetree,
  				  RewriteRule *rule,
  				  int rt_index,
- 				  bool relation_level,
  				  Relation relation,
  				  List *activeRIRs,
  				  bool forUpdatePushedDown)
--- 1292,1297 ----
***************
*** 1309,1316 **** ApplyRetrieveRule(Query *parsetree,
  		elog(ERROR, "expected just one rule action");
  	if (rule->qual != NULL)
  		elog(ERROR, "cannot handle qualified ON SELECT rule");
- 	if (!relation_level)
- 		elog(ERROR, "cannot handle per-attribute ON SELECT rule");
  
  	if (rt_index == parsetree->resultRelation)
  	{
--- 1305,1310 ----
***************
*** 1632,1645 **** fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
  			if (rule->event != CMD_SELECT)
  				continue;
  
- 			if (rule->attrno > 0)
- 			{
- 				/* per-attr rule; do we need it? */
- 				if (!attribute_used((Node *) parsetree, rt_index,
- 									rule->attrno, 0))
- 					continue;
- 			}
- 
  			locks = lappend(locks, rule);
  		}
  
--- 1626,1631 ----
***************
*** 1664,1670 **** fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
  				parsetree = ApplyRetrieveRule(parsetree,
  											  rule,
  											  rt_index,
- 											  rule->attrno == -1,
  											  rel,
  											  activeRIRs,
  											  forUpdatePushedDown);
--- 1650,1655 ----
*** a/src/backend/rewrite/rewriteManip.c
--- b/src/backend/rewrite/rewriteManip.c
***************
*** 858,927 **** rangeTableEntry_used(Node *node, int rt_index, int sublevels_up)
  
  
  /*
-  * attribute_used -
-  *	Check if a specific attribute number of a RTE is used
-  *	somewhere in the query or expression.
-  */
- 
- typedef struct
- {
- 	int			rt_index;
- 	int			attno;
- 	int			sublevels_up;
- } attribute_used_context;
- 
- static bool
- attribute_used_walker(Node *node,
- 					  attribute_used_context *context)
- {
- 	if (node == NULL)
- 		return false;
- 	if (IsA(node, Var))
- 	{
- 		Var		   *var = (Var *) node;
- 
- 		if (var->varlevelsup == context->sublevels_up &&
- 			var->varno == context->rt_index &&
- 			var->varattno == context->attno)
- 			return true;
- 		return false;
- 	}
- 	if (IsA(node, Query))
- 	{
- 		/* Recurse into subselects */
- 		bool		result;
- 
- 		context->sublevels_up++;
- 		result = query_tree_walker((Query *) node, attribute_used_walker,
- 								   (void *) context, 0);
- 		context->sublevels_up--;
- 		return result;
- 	}
- 	return expression_tree_walker(node, attribute_used_walker,
- 								  (void *) context);
- }
- 
- bool
- attribute_used(Node *node, int rt_index, int attno, int sublevels_up)
- {
- 	attribute_used_context context;
- 
- 	context.rt_index = rt_index;
- 	context.attno = attno;
- 	context.sublevels_up = sublevels_up;
- 
- 	/*
- 	 * Must be prepared to start with a Query or a bare expression tree; if
- 	 * it's a Query, we don't want to increment sublevels_up.
- 	 */
- 	return query_or_expression_tree_walker(node,
- 										   attribute_used_walker,
- 										   (void *) &context,
- 										   0);
- }
- 
- 
- /*
   * If the given Query is an INSERT ... SELECT construct, extract and
   * return the sub-Query node that represents the SELECT part.  Otherwise
   * return the given Query.
--- 858,863 ----
*** a/src/backend/utils/adt/ruleutils.c
--- b/src/backend/utils/adt/ruleutils.c
***************
*** 3734,3740 **** make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
  	char	   *rulename;
  	char		ev_type;
  	Oid			ev_class;
- 	int16		ev_attr;
  	bool		is_instead;
  	char	   *ev_qual;
  	char	   *ev_action;
--- 3734,3739 ----
***************
*** 3761,3771 **** make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
  	Assert(!isnull);
  	ev_class = DatumGetObjectId(dat);
  
- 	fno = SPI_fnumber(rulettc, "ev_attr");
- 	dat = SPI_getbinval(ruletup, rulettc, fno, &isnull);
- 	Assert(!isnull);
- 	ev_attr = DatumGetInt16(dat);
- 
  	fno = SPI_fnumber(rulettc, "is_instead");
  	dat = SPI_getbinval(ruletup, rulettc, fno, &isnull);
  	Assert(!isnull);
--- 3760,3765 ----
***************
*** 3820,3829 **** make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
  
  	/* The relation the rule is fired on */
  	appendStringInfo(buf, " TO %s", generate_relation_name(ev_class, NIL));
- 	if (ev_attr > 0)
- 		appendStringInfo(buf, ".%s",
- 						 quote_identifier(get_relid_attribute_name(ev_class,
- 																   ev_attr)));
  
  	/* If the rule has an event qualification, add it */
  	if (ev_qual == NULL)
--- 3814,3819 ----
***************
*** 3925,3931 **** make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
  	Query	   *query;
  	char		ev_type;
  	Oid			ev_class;
- 	int16		ev_attr;
  	bool		is_instead;
  	char	   *ev_qual;
  	char	   *ev_action;
--- 3915,3920 ----
***************
*** 3943,3951 **** make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
  	fno = SPI_fnumber(rulettc, "ev_class");
  	ev_class = (Oid) SPI_getbinval(ruletup, rulettc, fno, &isnull);
  
- 	fno = SPI_fnumber(rulettc, "ev_attr");
- 	ev_attr = (int16) SPI_getbinval(ruletup, rulettc, fno, &isnull);
- 
  	fno = SPI_fnumber(rulettc, "is_instead");
  	is_instead = (bool) SPI_getbinval(ruletup, rulettc, fno, &isnull);
  
--- 3932,3937 ----
***************
*** 3965,3971 **** make_viewdef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
  
  	query = (Query *) linitial(actions);
  
! 	if (ev_type != '1' || ev_attr >= 0 || !is_instead ||
  		strcmp(ev_qual, "<>") != 0 || query->commandType != CMD_SELECT)
  	{
  		appendStringInfo(buf, "Not a view");
--- 3951,3957 ----
  
  	query = (Query *) linitial(actions);
  
! 	if (ev_type != '1' || !is_instead ||
  		strcmp(ev_qual, "<>") != 0 || query->commandType != CMD_SELECT)
  	{
  		appendStringInfo(buf, "Not a view");
*** a/src/backend/utils/cache/relcache.c
--- b/src/backend/utils/cache/relcache.c
***************
*** 682,688 **** RelationBuildRuleLock(Relation relation)
  		rule->ruleId = HeapTupleGetOid(rewrite_tuple);
  
  		rule->event = rewrite_form->ev_type - '0';
- 		rule->attrno = rewrite_form->ev_attr;
  		rule->enabled = rewrite_form->ev_enabled;
  		rule->isInstead = rewrite_form->is_instead;
  
--- 682,687 ----
***************
*** 798,805 **** equalRuleLocks(RuleLock *rlock1, RuleLock *rlock2)
  				return false;
  			if (rule1->event != rule2->event)
  				return false;
- 			if (rule1->attrno != rule2->attrno)
- 				return false;
  			if (rule1->enabled != rule2->enabled)
  				return false;
  			if (rule1->isInstead != rule2->isInstead)
--- 797,802 ----
*** a/src/include/catalog/pg_rewrite.h
--- b/src/include/catalog/pg_rewrite.h
***************
*** 35,41 **** CATALOG(pg_rewrite,2618)
  {
  	NameData	rulename;
  	Oid			ev_class;
- 	int16		ev_attr;
  	char		ev_type;
  	char		ev_enabled;
  	bool		is_instead;
--- 35,40 ----
***************
*** 57,70 **** typedef FormData_pg_rewrite *Form_pg_rewrite;
   *		compiler constants for pg_rewrite
   * ----------------
   */
! #define Natts_pg_rewrite				8
  #define Anum_pg_rewrite_rulename		1
  #define Anum_pg_rewrite_ev_class		2
! #define Anum_pg_rewrite_ev_attr			3
! #define Anum_pg_rewrite_ev_type			4
! #define Anum_pg_rewrite_ev_enabled		5
! #define Anum_pg_rewrite_is_instead		6
! #define Anum_pg_rewrite_ev_qual			7
! #define Anum_pg_rewrite_ev_action		8
  
  #endif   /* PG_REWRITE_H */
--- 56,68 ----
   *		compiler constants for pg_rewrite
   * ----------------
   */
! #define Natts_pg_rewrite				7
  #define Anum_pg_rewrite_rulename		1
  #define Anum_pg_rewrite_ev_class		2
! #define Anum_pg_rewrite_ev_type			3
! #define Anum_pg_rewrite_ev_enabled		4
! #define Anum_pg_rewrite_is_instead		5
! #define Anum_pg_rewrite_ev_qual			6
! #define Anum_pg_rewrite_ev_action		7
  
  #endif   /* PG_REWRITE_H */
*** a/src/include/rewrite/prs2lock.h
--- b/src/include/rewrite/prs2lock.h
***************
*** 25,31 **** typedef struct RewriteRule
  {
  	Oid			ruleId;
  	CmdType		event;
- 	AttrNumber	attrno;
  	Node	   *qual;
  	List	   *actions;
  	char		enabled;
--- 25,30 ----
*** a/src/include/rewrite/rewriteManip.h
--- b/src/include/rewrite/rewriteManip.h
***************
*** 49,56 **** extern void IncrementVarSublevelsUp_rtable(List *rtable,
  
  extern bool rangeTableEntry_used(Node *node, int rt_index,
  					 int sublevels_up);
- extern bool attribute_used(Node *node, int rt_index, int attno,
- 			   int sublevels_up);
  
  extern Query *getInsertSelectQuery(Query *parsetree, Query ***subquery_ptr);
  
--- 49,54 ----
*** a/src/tools/pgindent/typedefs.list
--- b/src/tools/pgindent/typedefs.list
***************
*** 1955,1961 **** adjust_appendrel_attrs_context
  allocfunc
  array_unnest_fctx
  assign_collations_context
- attribute_used_context
  autovac_table
  av_relation
  avl_dbase
--- 1955,1960 ----
-- 
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