Thread: SET WITHOUT OIDS and VACUUM badness?

SET WITHOUT OIDS and VACUUM badness?

From
Christopher Kings-Lynne
Date:
This is what we did:

0. BEGIN;

1. ALTER TABLE ... SET WITHOUT OIDS

2. A bunch of things are selected out of this table and inserted into 
another (using INSERT ... SELECT)

3. An index is created on a timestamp field on this table

4. Then there's an update on a related table, that selects stuff from 
this table.

5. Renames a column

6. Drops a constraint

7. Adds a foreign key

8. Drops 8 columns

9. Drops 2 indexes

10. Drops 3 triggers

11. Then a tsearch 'txtidx' field is updated, and then cancelled halfway 
through

12. ROLLBACK;

13. VACUUM FULL forums_posts;

Then we get thousands of these:

WARNING:  relation "forums_posts" TID 22763/10: OID is invalid
WARNING:  relation "forums_posts" TID 22763/11: OID is invalid
WARNING:  relation "forums_posts" TID 22763/12: OID is invalid
WARNING:  relation "forums_posts" TID 22763/13: OID is invalid
WARNING:  relation "forums_posts" TID 22763/14: OID is invalid
WARNING:  relation "forums_posts" TID 22763/15: OID is invalid
WARNING:  relation "forums_posts" TID 22763/16: OID is invalid
WARNING:  relation "forums_posts" TID 22763/17: OID is invalid
WARNING:  relation "forums_posts" TID 22764/1: OID is invalid
WARNING:  relation "forums_posts" TID 22764/2: OID is invalid
WARNING:  relation "forums_posts" TID 22764/3: OID is invalid
WARNING:  relation "forums_posts" TID 22764/4: OID is invalid

This seems to be reproducible...

Chris



Re: SET WITHOUT OIDS and VACUUM badness?

From
Dennis Bjorklund
Date:
On Wed, 21 Jan 2004, Christopher Kings-Lynne wrote:

> This seems to be reproducible...

Here is a smaller example that show the problem:

CREATE TABLE foo (a INT);

BEGIN; ALTER TABLE foo SET WITHOUT OIDS; INSERT INTO foo values (5);
ROLLBACK;

VACUUM FULL foo;

It's easy to guess what is causing this, but I'll leave that to the person 
that wants to fix it.

-- 
/Dennis Björklund



Re: SET WITHOUT OIDS and VACUUM badness?

From
Gavin Sherry
Date:
On Wed, 21 Jan 2004, Christopher Kings-Lynne wrote:

> This is what we did:
>
> 0. BEGIN;
>
> 1. ALTER TABLE ... SET WITHOUT OIDS

> 12. ROLLBACK;
>
> 13. VACUUM FULL forums_posts;

The problem here is that this conditional doesn't take into account the
change in state which the above transaction causes:
           if (onerel->rd_rel->relhasoids &&               !OidIsValid(HeapTupleGetOid(&tuple)))

Tuples inserted after step one have no (valid) OID. However, since we
rollback, the change to pg_class.relhasoids => 'f' is rolled back. The
only solution I can think of is removing the test or storing relhasoids as
a per tuple flag (argh).

Gavin


Re: SET WITHOUT OIDS and VACUUM badness?

From
Gavin Sherry
Date:
On Wed, 21 Jan 2004, Gavin Sherry wrote:

> On Wed, 21 Jan 2004, Christopher Kings-Lynne wrote:
>
> > This is what we did:
> >
> > 0. BEGIN;
> >
> > 1. ALTER TABLE ... SET WITHOUT OIDS
>
> > 12. ROLLBACK;
> >
> > 13. VACUUM FULL forums_posts;
>
> The problem here is that this conditional doesn't take into account the
> change in state which the above transaction causes:
>
>             if (onerel->rd_rel->relhasoids &&
>                 !OidIsValid(HeapTupleGetOid(&tuple)))
>
> Tuples inserted after step one have no (valid) OID. However, since we
> rollback, the change to pg_class.relhasoids => 'f' is rolled back. The
> only solution I can think of is removing the test or storing relhasoids as
> a per tuple flag (argh).

What am I talking about. Can't we test for:

(&tuple)->t_infomask & HEAP_HASOID

Instead of:

onerel->rd_rel->relhasoids

Gavin


Re: SET WITHOUT OIDS and VACUUM badness?

From
Tom Lane
Date:
Gavin Sherry <swm@linuxworld.com.au> writes:
> What am I talking about. Can't we test for:
> (&tuple)->t_infomask & HEAP_HASOID
> Instead of:
> onerel->rd_rel->relhasoids

ISTM the point of the check is to detect rows that are out of sync with
the relation's relhasoids flag, so we might as well just get rid of the
check entirely as do that.

I'm not averse to dropping the check, but if we want to keep it, I'd be
inclined to restrict it to live tuples.
        regards, tom lane


Re: SET WITHOUT OIDS and VACUUM badness?

From
Bruce Momjian
Date:
Gavin Sherry wrote:
> On Wed, 21 Jan 2004, Gavin Sherry wrote:
> 
> > On Wed, 21 Jan 2004, Christopher Kings-Lynne wrote:
> >
> > > This is what we did:
> > >
> > > 0. BEGIN;
> > >
> > > 1. ALTER TABLE ... SET WITHOUT OIDS
> >
> > > 12. ROLLBACK;
> > >
> > > 13. VACUUM FULL forums_posts;
> >
> > The problem here is that this conditional doesn't take into account the
> > change in state which the above transaction causes:
> >
> >             if (onerel->rd_rel->relhasoids &&
> >                 !OidIsValid(HeapTupleGetOid(&tuple)))
> >
> > Tuples inserted after step one have no (valid) OID. However, since we
> > rollback, the change to pg_class.relhasoids => 'f' is rolled back. The
> > only solution I can think of is removing the test or storing relhasoids as
> > a per tuple flag (argh).
> 
> What am I talking about. Can't we test for:
> 
> (&tuple)->t_infomask & HEAP_HASOID
> 
> Instead of:
> 
> onerel->rd_rel->relhasoids

I can confirm we still have this bug:test=> CREATE TABLE foo (a INT);CREATE TABLEtest=> BEGIN;BEGINtest=>   ALTER TABLE
fooSET WITHOUT OIDS;  INSERT INTO foo values (5);ROLLBACK;VACUUM FULL foo;ALTER TABLEtest=>   INSERT INTO foo values
(5);INSERT0 1test=> ROLLBACK;ROLLBACKtest=>test=> VACUUM FULL foo;WARNING:  relation "foo" TID 0/1: OID is
invalidVACUUM

Anyone want to fix it?

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073
 


Re: SET WITHOUT OIDS and VACUUM badness?

From
Gavin Sherry
Date:
On Wed, 11 Feb 2004, Bruce Momjian wrote:

> Gavin Sherry wrote:
> > On Wed, 21 Jan 2004, Gavin Sherry wrote:
> >
> > > On Wed, 21 Jan 2004, Christopher Kings-Lynne wrote:
> > >
> > > > This is what we did:
> > > >
> > > > 0. BEGIN;
> > > >
> > > > 1. ALTER TABLE ... SET WITHOUT OIDS
> > >
> > > > 12. ROLLBACK;
> > > >
> > > > 13. VACUUM FULL forums_posts;
> > >
> > > The problem here is that this conditional doesn't take into account the
> > > change in state which the above transaction causes:
> > >
> > >             if (onerel->rd_rel->relhasoids &&
> > >                 !OidIsValid(HeapTupleGetOid(&tuple)))
> > >
> > > Tuples inserted after step one have no (valid) OID. However, since we
> > > rollback, the change to pg_class.relhasoids => 'f' is rolled back. The
> > > only solution I can think of is removing the test or storing relhasoids as
> > > a per tuple flag (argh).
> >
> > What am I talking about. Can't we test for:
> >
> > (&tuple)->t_infomask & HEAP_HASOID
> >
> > Instead of:
> >
> > onerel->rd_rel->relhasoids
>
> I can confirm we still have this bug:
>

[sample]

Tom had two suggestions later in the thread:

http://archives.postgresql.org/pgsql-hackers/2004-01/msg00467.php

Gavin


Re: SET WITHOUT OIDS and VACUUM badness?

From
Tom Lane
Date:
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Anyone want to fix it?

Done.
        regards, tom lane