Eliminating pg_catalog.pg_rewrite.ev_attr

Started by Kevin Grittnerover 12 years ago5 messageshackers
Jump to latest
#1Kevin Grittner
Kevin.Grittner@wicourts.gov

This was previously discussed here:

/messages/by-id/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

Attachments:

rule-attr-removal.patchtext/x-diff; name=rule-attr-removal.patchDownload+29-139
#2Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Kevin Grittner (#1)
Re: Eliminating pg_catalog.pg_rewrite.ev_attr

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Kevin Grittner (#1)
Re: Eliminating pg_catalog.pg_rewrite.ev_attr

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#4Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Tom Lane (#3)
Re: Eliminating pg_catalog.pg_rewrite.ev_attr

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

Attachments:

rule-attr-removal-v2.patchtext/x-diff; name=rule-attr-removal-v2.patchDownload+20-132
#5Kevin Grittner
Kevin.Grittner@wicourts.gov
In reply to: Kevin Grittner (#4)
Re: Eliminating pg_catalog.pg_rewrite.ev_attr

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

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers