Thread: code question: rewriteDefine.c

code question: rewriteDefine.c

From
Neil Conway
Date:
I noticed the following code in src/backend/rewrite/rewriteDefine.c,
circa line 390:
    /*     * Are we converting a relation to a view?     *     * If so, check that the relation is empty because the
storage    * for the relation is going to be deleted.  Also insist that     * the rel not have any triggers, indexes,
orchild tables.     */    if (event_relation->rd_rel->relkind != RELKIND_VIEW)    {
 

Under what circumstances do we "convert a relation to a view"? Is this
functionality exposed to the user?

Furthermore, it seems broken: it checks the pgclass.relhassubclass
attribute for this relation to see "if it has child tables", but this
is wrong, as relhassubclass only indicates that the relation MAY have
a subclass, not that is definitely does[1]. It also doesn't drop the
relation's TOAST table, if any, as the code itself notes.

-Neil

[1] This is because relhassubclass is not updated when a table's child
table is dropped, due to concurrency concerns; see has_subclass() in
plancat.c



Re: code question: rewriteDefine.c

From
Tom Lane
Date:
Neil Conway <neilc@samurai.com> writes:
> Under what circumstances do we "convert a relation to a view"? Is this
> functionality exposed to the user?

This is a backwards-compatibility hangover.  pg_dump scripts from
somewhere back in the Dark Ages (6.something) would represent a view
asCREATE TABLE v (column-list);CREATE RULE "_RETURN" AS ON SELECT TO v DO INSTEAD ...;
and the code you are looking at is intended to convert this locution
into a genuine-per-current-representation view.

I'm not sure how important it is to continue supporting that.  But I'd
not want to break it just because someone thinks the hack is ugly.
It was ugly from day one.

> Furthermore, it seems broken: it checks the pgclass.relhassubclass
> attribute for this relation to see "if it has child tables", but this
> is wrong, as relhassubclass only indicates that the relation MAY have
> a subclass, not that is definitely does[1]. It also doesn't drop the
> relation's TOAST table, if any, as the code itself notes.

There could not be any child tables, either current or former, in the
intended application.  There could be a TOAST table, but getting rid of
it would only save some useless entries in pg_class etc, not prevent any
functional problems, so no one bothered.
        regards, tom lane


Re: code question: rewriteDefine.c

From
Neil Conway
Date:
Tom Lane <tgl@sss.pgh.pa.us> writes:
> This is a backwards-compatibility hangover.

> But I'd not want to break it just because someone thinks the hack is
> ugly.  It was ugly from day one.

I agree it shouldn't be removed -- I was just curious to see what was
using it. It's certainly ugly, though.

I'll submit a patch documenting this.

-Neil