SET WITHOUT OIDS and VACUUM badness?

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

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

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

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

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 TABLE
test=> BEGIN;
BEGIN
test=> ALTER TABLE foo SET WITHOUT OIDS;
INSERT INTO foo values (5);
ROLLBACK;

VACUUM FULL foo;
ALTER TABLE
test=> INSERT INTO foo values (5);
INSERT 0 1
test=> ROLLBACK;
ROLLBACK
test=>
test=> VACUUM FULL foo;
WARNING: relation "foo" TID 0/1: OID is invalid
VACUUM

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, Pennsylvania 19073

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

Bruce Momjian <pgman@candle.pha.pa.us> writes:

Anyone want to fix it?

Done.

regards, tom lane