Re: Cleanup/remove/update references to OID column

Hi,

On 2019-04-10 11:59:18 -0500, Justin Pryzby wrote:
> @@ -1202,8 +1202,7 @@ CREATE TABLE circles (
> <structfield>ctid</structfield> will change if it is
> updated or moved by <command>VACUUM FULL</command>. Therefore
> <structfield>ctid</structfield> is useless as a long-term row
> - identifier. The OID, or even better a user-defined serial
> - number, should be used to identify logical rows.
> + identifier. A primary key should be used to identify logical rows.
> </para>
> </listitem>
> </varlistentry>

That works for me.

> @@ -3672,11 +3671,10 @@ VALUES ('Albany', NULL, NULL, 'NY');
> <para>
> Partitions cannot have columns that are not present in the parent. It
> is not possible to specify columns when creating partitions with
> - <command>CREATE TABLE</command>, nor is it possible to add columns to
> - partitions after-the-fact using <command>ALTER TABLE</command>. Tables may be
> - added as a partition with <command>ALTER TABLE ... ATTACH PARTITION</command>
> - only if their columns exactly match the parent, including any
> - <literal>oid</literal> column.
> + <command>CREATE TABLE</command>, to add columns to
> + partitions after-the-fact using <command>ALTER TABLE</command>, nor to
> + add a partition with <command>ALTER TABLE ... ATTACH PARTITION</command>
> + if its columns would not exactly match those of the parent.
> </para>
> </listitem>

This seems like a bigger change than necessary. I just chopped off the
"including any oid column".

> diff --git a/doc/src/sgml/ref/create_trigger.sgml b/doc/src/sgml/ref/create_trigger.sgml
> index 6456105..3339a4b 100644
> --- a/doc/src/sgml/ref/create_trigger.sgml
> +++ b/doc/src/sgml/ref/create_trigger.sgml
> @@ -465,7 +465,7 @@ UPDATE OF <replaceable>column_name1</replaceable> [, <replaceable>column_name2</
> that the <literal>NEW</literal> row seen by the condition is the current value,
> as possibly modified by earlier triggers. Also, a <literal>BEFORE</literal>
> trigger's <literal>WHEN</literal> condition is not allowed to examine the
> - system columns of the <literal>NEW</literal> row (such as <literal>oid</literal>),
> + system columns of the <literal>NEW</literal> row (such as <literal>ctid</literal>),
> because those won't have been set yet.
> </para>

Hm. Not because of your change, but this sentence seems wrong. For one,
"is not allowed" isn't really true - one can very well write a trigger
doing so. The returned values just are bogus.

CREATE OR REPLACE FUNCTION scream_sysattrs() RETURNS TRIGGER LANGUAGE
plpgsql AS $$
BEGIN
RAISE NOTICE 'inserted row: self: % xmin: % cmin: %, xmax: %, cmax: % tableoid: %', NEW.ctid, NEW.xmin, NEW.cmin, NEW.xmax, NEW.cmax, NEW.tableoid;
RETURN NEW;
END;$$;
DROP TABLE IF EXISTS foo; CREATE TABLE foo(i int);CREATE TRIGGER foo BEFORE INSERT ON foo FOR EACH ROW EXECUTE FUNCTION scream_sysattrs();
postgres[5532][1]=# INSERT INTO foo values(1);
NOTICE: 00000: inserted row: self: (0,0) xmin: 112 cmin: 2249, xmax: 4294967295, cmax: 2249 tableoid: 0
LOCATION: exec_stmt_raise, pl_exec.c:3778
INSERT 0 1

We probably should do better...

> diff --git a/doc/src/sgml/ref/insert.sgml b/doc/src/sgml/ref/insert.sgml
> index 62e142f..3e1be4c 100644
> --- a/doc/src/sgml/ref/insert.sgml
> +++ b/doc/src/sgml/ref/insert.sgml
> @@ -552,13 +552,11 @@ INSERT INTO <replaceable class="parameter">table_name</replaceable> [ AS <replac
> INSERT <replaceable>oid</replaceable> <replaceable class="parameter">count</replaceable>
> </screen>
> The <replaceable class="parameter">count</replaceable> is the
> - number of rows inserted or updated. If <replaceable
> - class="parameter">count</replaceable> is exactly one, and the
> - target table has OIDs, then <replaceable
> - class="parameter">oid</replaceable> is the <acronym>OID</acronym>
> - assigned to the inserted row. The single row must have been
> - inserted rather than updated. Otherwise <replaceable
> - class="parameter">oid</replaceable> is zero.
> + number of rows inserted or updated.
> + <replaceable>oid</replaceable> used to be the object ID of the inserted row
> + if <replaceable>rows</replaceable> was 1 and the target table had OIDs, but
> + OIDs system columns are not supported anymore; therefore
> + <replaceable>oid</replaceable> is always 0.
> </para>

I rephrased this a bit. Felt like the important bit came after
historical information:
+ The <replaceable class="parameter">count</replaceable> is the number of
+ rows inserted or updated. <replaceable>oid</replaceable> is always 0 (it
+ used to be the <acronym>OID</acronym> assigned to the inserted row if
+ <replaceable>rows</replaceable> was exactly one and the target table was
+ declared <literal>WITH OIDS</literal> and 0 otherwise, but creating a table
+ <literal>WITH OIDS</literal> is not supported anymore).

> <para>
> diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
> index 08f4bab..0e6e792 100644
> --- a/doc/src/sgml/ref/psql-ref.sgml
> +++ b/doc/src/sgml/ref/psql-ref.sgml
> @@ -3794,6 +3794,9 @@ bar
> command. This variable is only guaranteed to be valid until
> after the result of the next <acronym>SQL</acronym> command has
> been displayed.
> + <productname>PostgreSQL</productname> servers since version 12 do not
> + support OID system columns in user tables, and LASTOID will always be 0
> + following <command>INSERT</command>.
> </para>
> </listitem>
> </varlistentry>

It's not just user tables, system tables as well (it's just an ordinary
table now). I also though it might be good to clarify that LASTOID still
works for older servers.

+ <productname>PostgreSQL</productname> servers since version 12 do not
+ support OID system columns anymore, thus LASTOID will always be 0
+ following <command>INSERT</command> when targeting such servers.

Thanks for the patch!

Greetings,

Andres Freund