This was previously discussed here:
http://www.postgresql.org/message-id/flat/24836.1370713...@sss.pgh.pa.us#24836.1370713...@sss.pgh.pa.us
The attached patch implements what I think we agreed on.
To recap, ev_attr was present in pg_rewrite at the point that
Postgres95 version 1.01 source code was imported to version
control, with a default of -1 to mean "all columns". It became
obsolete in 2002 with commit
95ef6a344821655ce4d0a74999ac49dd6af6d342, which went into
PostgreSQL version 7.3, removing the ability to define a rule on a
specific column; however, this column and over 100 lines of
vestigial code was left behind. Later code was written as though 0
was used to mean "all columns", as is done elsewhere in the code,
although pre-existing code was not changed to match. That
inconsistency didn't much matter since there was no way to define
anything which exercised the code, short of hacking the system
tables directly.
The patch removes the obsolete column from pg_rewrite, and all the
vestigial code I was able to find. The justification for the patch
is to eliminate over 100 lines of code from an area which is
confusing enough without it.
Unless someone has an objection or thinks this needs to go through
the CF process, I will commit it tomorrow, with a catversion bump.
--
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
***************
*** 1273,1287 **** 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);
! }
}
return matching_locks;
--- 1273,1280 ----
}
}
! if (oneLock->event == event && parsetree->commandType != CMD_SELECT)
! matching_locks = lappend(matching_locks, oneLock);
}
return matching_locks;
***************
*** 1295,1301 **** static Query *
ApplyRetrieveRule(Query *parsetree,
RewriteRule *rule,
int rt_index,
- bool relation_level,
Relation relation,
List *activeRIRs,
bool forUpdatePushedDown)
--- 1288,1293 ----
***************
*** 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)
{
--- 1301,1306 ----
***************
*** 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);
}
--- 1622,1627 ----
***************
*** 1664,1670 **** fireRIRrules(Query *parsetree, List *activeRIRs, bool forUpdatePushedDown)
parsetree = ApplyRetrieveRule(parsetree,
rule,
rt_index,
- rule->attrno == -1,
rel,
activeRIRs,
forUpdatePushedDown);
--- 1646,1651 ----
*** 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,55 ----
extern bool rangeTableEntry_used(Node *node, int rt_index,
int sublevels_up);
! extern bool attribute_used(Node *node, int rt_index, int sublevels_up);
extern Query *getInsertSelectQuery(Query *parsetree, Query ***subquery_ptr);
*** 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
