Re: [PATCH] Teach how many attributes there should be per DATA() line

From: ilmari(at)ilmari(dot)org (Dagfinn Ilmari =?utf-8?Q?Manns=C3=A5ker?=)
To: David Christensen <david(at)endpoint(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PATCH] Teach how many attributes there should be per DATA() line
Date: 2017-03-06 15:14:28
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Hi David,

Here's a review of your patch.

David Christensen <david(at)endpoint(dot)com> writes:

> Throws a build error if we encounter a different number of fields in a
> DATA() line than we expect for the catalog in question.

The patch is a good idea, and as-is implements the suggested feature.
Tested by removing an attribute from a line in catalog/pg_proc.h and
observing the expected failure. With the attribute in place, it builds and
passes make check-world.

Detailed code review:

> diff --git a/src/backend/catalog/ b/src/backend/catalog/
> + check_natts($filename, $catalog{natts},$3) or
> + die sprintf "Wrong number of Natts in DATA() line %s:%d\n", $input_file,INPUT_FILE->input_line_number;

Including the expected/actual number of attributes in the error message
would be nice. In fact, the 'die' could be moved into check_natts(),
where the actual number is already available, and would clutter the main
loop less.

> + unless ($natts)
> + {
> + die "Could not find definition for Natts_${catname} before start of DATA()\n";
> + }

More idiomatically written as:

die ....
unless $natts;

> diff --git a/src/backend/utils/ b/src/backend/utils/

The changes to this file are redundant, since it calls Catalog::Catalogs(),
which already checks that the number of attributes matches.

Attached is a suggested revised patch.

- ilmari

- Twitter seems more influential [than blogs] in the 'gets reported in
the mainstream press' sense at least. - Matt McLeod
- That'd be because the content of a tweet is easier to condense down
to a mainstream media article. - Calle Dybedahl

Attachment Content-Type Size text/x-diff 2.4 KB

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message Aleksander Alekseev 2017-03-06 15:22:17 Re: Declarative partitioning optimization for large amount of partitions
Previous Message Erik Rijkers 2017-03-06 15:10:19 Re: Logical replication existing data copy