Thread: Eliminating pg_catalog.pg_rewrite.ev_attr
This was previously discussed here: http://www.postgresql.org/message-id/flat/24836.1370713481@sss.pgh.pa.us#24836.1370713481@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
Attachment
Is this transformation correct? If I read this correctly, you're
missing the rangeTableEntry_used() condition, no?
> *** 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;
--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services
Kevin Grittner <kgrittn@ymail.com> writes:
> Unless someone has an objection or thinks this needs to go through
> the CF process, I will commit it tomorrow, with a catversion bump.
Shouldn't attribute_used() be removed from rewriteManip.h?
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.
regards, tom lane
Tom Lane <tgl@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
Attachment
Kevin Grittner <kgrittn@ymail.com> wrote: > New patch attached. Pushed (to master only). 277607d600fb71e25082b94302ca1716403cd0bc -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company