Thread: Dropped index on table preventing rule creation
Hi, I don't use rules, but in a bit of experimentation on Git master, I discovered the following behaviour: CREATE TABLE test1 (id serial primary key, things text); CREATE TABLE test2 (id serial primary key, things text); ALTER TABLE test1 DROP CONSTRAINT test1_pkey; ALTER TABLE test2 DROP CONSTRAINT test2_pkey; CREATE RULE "_RETURN" AS ON SELECT TO test1 DO INSTEAD select * from test2; This produces the error message: could not convert table "test1" to a view because it has indexes There are no indexes or primary keys on either table by the time the rule creation statement runs. If creating the same 2 tables without originally giving them primary keys, this rule creates successfully. -- Thom Brown Twitter: @darkixion IRC (freenode): dark_ixion Registered Linux user: #516935 EnterpriseDB UK: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Thom Brown <thom@linux.com> writes:
> I don't use rules, but in a bit of experimentation on Git master, I
> discovered the following behaviour:
> CREATE TABLE test1 (id serial primary key, things text);
> CREATE TABLE test2 (id serial primary key, things text);
> ALTER TABLE test1 DROP CONSTRAINT test1_pkey;
> ALTER TABLE test2 DROP CONSTRAINT test2_pkey;
> CREATE RULE "_RETURN" AS ON SELECT TO test1 DO INSTEAD select * from test2;
> This produces the error message: could not convert table "test1" to a
> view because it has indexes
IIRC, this is because the check is just checking relhasindex. You
should be able to recover and create the rule if you VACUUM the table.
We could no doubt add more code to make that more transparent, but I
don't see the point. The entire exercise of converting a table to a
view is only meant to support loading of pg_dump output from versions
that are probably ten years obsolete at this point. We don't even
document that you can do the above, do we?
(IOW, rather than "fix" this I'd prefer to rip out the code altogether.
But maybe we should wait a couple more years for that.)
regards, tom lane
On Sep 10, 2011, at 11:26 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Thom Brown <thom@linux.com> writes: >> I don't use rules, but in a bit of experimentation on Git master, I >> discovered the following behaviour: >=20 >> CREATE TABLE test1 (id serial primary key, things text); >> CREATE TABLE test2 (id serial primary key, things text); >> ALTER TABLE test1 DROP CONSTRAINT test1_pkey; >> ALTER TABLE test2 DROP CONSTRAINT test2_pkey; >> CREATE RULE "_RETURN" AS ON SELECT TO test1 DO INSTEAD select * from tes= t2; >=20 >> This produces the error message: could not convert table "test1" to a >> view because it has indexes >=20 > IIRC, this is because the check is just checking relhasindex. You > should be able to recover and create the rule if you VACUUM the table. >=20 > We could no doubt add more code to make that more transparent, but I > don't see the point. The entire exercise of converting a table to a > view is only meant to support loading of pg_dump output from versions > that are probably ten years obsolete at this point. We don't even > document that you can do the above, do we? >=20 > (IOW, rather than "fix" this I'd prefer to rip out the code altogether. > But maybe we should wait a couple more years for that.) IIRC, it's not dead code. I think you can still generate such a dump if you= use CREATE OR REPLACE VIEW to manufacture a pair of mutually recursive vie= ws. Now we should probably disallow that, but we currently don't. ...Robert
Robert Haas <robertmhaas@gmail.com> writes:
> On Sep 10, 2011, at 11:26 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> (IOW, rather than "fix" this I'd prefer to rip out the code altogether.
>> But maybe we should wait a couple more years for that.)
> IIRC, it's not dead code. I think you can still generate such a dump if you use CREATE OR REPLACE VIEW to manufacture
apair of mutually recursive views.
Oh, yeah, I'd forgotten about that. In general that's pg_dump's
strategy for breaking a circular dependency loop that involves a view.
> Now we should probably disallow that, but we currently don't.
Losing that particular case isn't problematic, but I'm not sure that
that's the only possible circularity involving a view. One idea that
comes to mind is
create table foo (list-of-columns);
create function foofunc () returns setof foo as ...;
create rule .... as select * from foofunc();
This only saves somebody from citing the list of column types twice,
so maybe we could blow off this case too; but who's to say there are
not more-useful cases that would create circularities?
regards, tom lane
On Tue, Sep 13, 2011 at 4:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Robert Haas <robertmhaas@gmail.com> writes: >> On Sep 10, 2011, at 11:26 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >>> (IOW, rather than "fix" this I'd prefer to rip out the code altogether. >>> But maybe we should wait a couple more years for that.) > >> IIRC, it's not dead code. I think you can still generate such a dump if you use CREATE OR REPLACE VIEW to manufacturea pair of mutually recursive views. > > Oh, yeah, I'd forgotten about that. In general that's pg_dump's > strategy for breaking a circular dependency loop that involves a view. > >> Now we should probably disallow that, but we currently don't. > > Losing that particular case isn't problematic, but I'm not sure that > that's the only possible circularity involving a view. One idea that > comes to mind is > > create table foo (list-of-columns); > > create function foofunc () returns setof foo as ...; > > create rule .... as select * from foofunc(); > > This only saves somebody from citing the list of column types twice, > so maybe we could blow off this case too; but who's to say there are > not more-useful cases that would create circularities? I spent some more time looking at this tonight. I am wondering if perhaps we should just get rid of relhasindex. At the time that was added - which predates our commit history - we didn't use the relcache to cache the results of scanning pg_index, and we scanned pg_index using a heap scan rather than an index scan. So at that point being able to tell from the pg_class tuple that no indexes could exist was probably a rather large win. It's likely a lot smaller now - and, really, how many tables don't have indexes anyway? Even so, I'm not 100% sure whether it's worth doing: the existing code isn't really hurting us, and I think we could fix Thom's complaint by changing DefineQueryRewrite() to call RelationGetIndexList() rather than blindly believing relhasindex, which would be maybe a five line code-change. We'd probably also want to change SetRelationRuleStatus() to clear relhasindex, which would be one more line of code. One related thing that seems worth doing is ripping out relhaspkey, which is used for absolutely nothing at all. It looks to me like doing so will save four bytes per row in pg_class due to alignment padding. Patch for that attached. (Yeah, I know... this could break client code... but is it really worth keeping this cruft around forever? Especially since the column can easily say there's a primary key when there really isn't?) -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Attachment
Robert Haas <robertmhaas@gmail.com> writes:
> I spent some more time looking at this tonight. I am wondering if
> perhaps we should just get rid of relhasindex.
-1, there is absolutely no reason to believe that's a good idea.
> ... I think we could fix Thom's complaint by changing
> DefineQueryRewrite() to call RelationGetIndexList() rather than
> blindly believing relhasindex, which would be maybe a five line
> code-change. We'd probably also want to change
> SetRelationRuleStatus() to clear relhasindex, which would be one more
> line of code.
Yeah, that's about what it would take, but what I'm asking is why
bother. The *only* case that we support here is turning a just-created,
not-fooled-with table into a view, and I don't feel a need to promise
that we will handle other cases (which are inevitably going to be poorly
tested). See for example the adjacent relhassubclass test, which has
got exactly the same issue.
> One related thing that seems worth doing is ripping out relhaspkey,
Having a hard time getting excited about that either ...
regards, tom lane
On Wed, Sep 14, 2011 at 10:58 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Yeah, that's about what it would take, but what I'm asking is why > bother. =A0The *only* case that we support here is turning a just-created, > not-fooled-with table into a view, and I don't feel a need to promise > that we will handle other cases (which are inevitably going to be poorly > tested). =A0See for example the adjacent relhassubclass test, which has > got exactly the same issue. > >> One related thing that seems worth doing is ripping out relhaspkey, > > Having a hard time getting excited about that either ... The fact that this code is poorly tested is exactly why I don't think we should be complicating it with doodads of doubtful utility. The existence of these Booleans causes people to use them. This probably doesn't save any appreciable amount of performance, but it does make it easier to write wrong code. --=20 Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company