Tom Lane <t...@sss.pgh.pa.us> wrote: > Kevin Grittner <kgri...@ymail.com> writes: >> Tom Lane <t...@sss.pgh.pa.us> wrote: >>> Actually, I think this is a bug and the right thing is to make the code >>> match the documentation not vice versa. > >> I assume that this should be a 9.3 code fix, and a doc fix prior to >> that, since it would require changing catalogs and might break >> existing user queries? Should the docs mention the value used in >> each version, or be changed to just be silent on the issue? > > I think the odds that any user queries are looking at that column are > pretty negligible, especially since nobody has complained about the > inaccurate documentation previously. I agree with only changing the > behavior in HEAD, just in case, but I don't see any strong reason to > jump through hoops here. > >> Such a change would require a catversion bump. > > Not really. There appears to be one place in ruleutils.c that would > need to be tweaked to allow either -1 or 0 (the other place already > does, so the code is inconsistent now anyhow). > >> Such a change would require mention in the release notes because >> existing user queries against pg_rewrite might fail unless >> adjusted. > > I would not bother with that either; seems like a waste of readers' > attention span. > >> Is it worth doing that now, versus when and if the hypothetical >> change to reference a column is made? > > Well, the longer we leave it as-is, the greater risk that somebody > might write code that really does depend on the bogus value.
I'll leave this alone for a day. If nobody objects, I will change the ruleutils.c code to work with either value (to support pg_upgrade) and change the code to set this to zero, for 9.3 and forward only. I will change the 9.3 docs to mention that newly created rows will have zero, but that clusters upgraded via pg_upgrade may still have some rows containing the old value of -1. For anyone casually following along without reading the code, it's not a bug in the sense that current code ever misbehaves, but the value which has been used for ev_attr to indicate "whole table" since at least Postgres95 version 1.01 is not consistent with other places we set a dummy value in attribute number to indicate "whole table". Since 2002 we have not supported column-level rules, so it has just been filled with a constant of -1. The idea is to change the constant to zero -- to make it more consistent so as to reduce confusion and the chance of future bugs should we ever decide to use the column again. If we were a little earlier in the release cycle I would argue that if we're going to do anything with this column we should drop it. -- Kevin Grittner EnterpriseDB: 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
