On 11/16/18, Andres Freund <andres@anarazel.de> wrote:
> On 2018-11-15 17:25:21 +0700, John Naylor wrote:
>> I don't see an advantage to having a different range, but maybe it
>> should error out if $maxoid reaches FirstBootstrapObjectId.
>
> Hm. Not sure I really see the point. Note we didn't have that check
> before either, and it'd not catch manual assignment of too high oids. I
> wonder if we should have a check in sanity_check.sql or such, that'd
> then catch both?
My concern is that a developer might assign a high number oid to avoid
conflicts during development, such that the auto-assigned oids can
blow past 10000. If it can be done simply in SQL, I think that'd be
fine, and maybe more robust than checking within the script.
>> This patch breaks reformat_dat_file.pl. I've attached a fix, which
>> also de-lists oid as a special key within the *.dat files. It might be
>> good to put off reformatting until feature freeze, so as not to break
>> others' patches.
>
> Thanks for catching that. I wonder if we could fix that in a way that
> doesn't move oid into the middle of the data - while it's less magic-y
> from a storage level, it's still more important than the rest. Perhaps
> we could just leave oid in metadata and skip all @metadata elements in
> format_hash()?
@metadata is passed to format_hash(), so that wouldn't work without
additional bookkeeping (if I understand you correctly). I think it'd
be simpler to filter out @metadata elements while building @attnames,
which is what we pass to format_hash() for ordinary columns. Let me
know what you think (attached).
>> @@ -193,7 +192,7 @@ sub strip_default_values
>> {
>> my $attname = $column->{name};
>> die "strip_default_values: $catname.$attname undefined\n"
>> - if !defined $row->{$attname};
>> + if !defined $row->{$attname} and $attname ne 'oid';
>>
>> if (defined $column->{default}
>> and ($row->{$attname} eq $column->{default}))
>
> Hm, why is there no column definition for oid?
(This is a sanity check that all keys referring to columns have
values. I'd like to keep it since it has found bugs during
development.) If oid is now a normal column, then catalogs like
pg_amop will have no explicit oid and the check will fail without this
change. I've added a comment -- hopefully it's clearer.
-John Naylor