Re: [COMMITTERS] pgsql: Don't use OidIsValid to check the return value of

Lists: pgsql-committerspgsql-hackers
From: heikki(at)postgresql(dot)org (Heikki Linnakangas)
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Don't use OidIsValid to check the return value of
Date: 2008-12-20 09:40:56
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Log Message:
-----------
Don't use OidIsValid to check the return value of transformGenericOptions,
because transformGenericOptions returns an array, not an Oid. I'm not
sure if this fixes the crashes seen in buildfarm, but it should be fixed
anyway.

Modified Files:
--------------
pgsql/src/backend/commands:
foreigncmds.c (r1.1 -> r1.2)
(http://anoncvs.postgresql.org/cvsweb.cgi/pgsql/src/backend/commands/foreigncmds.c?r1=1.1&r2=1.2)


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Heikki Linnakangas <heikki(at)enterprisedb(dot)com>
Cc: pgsql-hackers(at)postgreSQL(dot)org
Subject: Re: [COMMITTERS] pgsql: Don't use OidIsValid to check the return value of
Date: 2008-12-20 15:55:56
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

heikki(at)postgresql(dot)org (Heikki Linnakangas) writes:
> Don't use OidIsValid to check the return value of transformGenericOptions,
> because transformGenericOptions returns an array, not an Oid. I'm not
> sure if this fixes the crashes seen in buildfarm, but it should be fixed
> anyway.

Definitely a necessary fix, but you missed what I think is actually
causing the crashes:

if (PointerIsValid(DatumGetPointer(datum)))
! repl_val[Anum_pg_foreign_data_wrapper_fdwoptions - 1] = ObjectIdGetDatum(datum);

should be

if (PointerIsValid(DatumGetPointer(datum)))
! repl_val[Anum_pg_foreign_data_wrapper_fdwoptions - 1] = datum;

ObjectIdGetDatum probably is zeroing the high-order half of the pointer
datum.

I committed this along with some other cosmetic fixes.

Somebody still needs to fix the MSVC build scripts...

regards, tom lane


From: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgreSQL(dot)org
Subject: Re: [COMMITTERS] pgsql: Don't use OidIsValid to check the return value of
Date: 2008-12-20 16:11:30
Message-ID: [email protected]
Views: Whole Thread | Raw Message | Download mbox | Resend email
Lists: pgsql-committers pgsql-hackers

Tom Lane wrote:
> Definitely a necessary fix, but you missed what I think is actually
> causing the crashes:
>
> if (PointerIsValid(DatumGetPointer(datum)))
> ! repl_val[Anum_pg_foreign_data_wrapper_fdwoptions - 1] = ObjectIdGetDatum(datum);
>
> should be
>
> if (PointerIsValid(DatumGetPointer(datum)))
> ! repl_val[Anum_pg_foreign_data_wrapper_fdwoptions - 1] = datum;
>
> ObjectIdGetDatum probably is zeroing the high-order half of the pointer
> datum.

Doh, how could I miss that, on the very next line :-)

> I committed this along with some other cosmetic fixes.

Thanks.

--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com