nodes/*funcs.c inconsistencies

From: Noah Misch <noah(at)leadboat(dot)com>
To: pgsql-hackers(at)postgresql(dot)org
Subject: nodes/*funcs.c inconsistencies
Date: 2012-04-16 10:25:15
Message-ID: 20120416102515.GF22182@tornado.leadboat.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I observed these inconsistencies in node support functions:

- _copyReassignOwnedStmt() uses COPY_SCALAR_FIELD() on the string field
"newrole", and _equalReassignOwnedStmt() uses COMPARE_NODE_FIELD().

- _outCreateForeignTableStmt() calls _outCreateStmt() directly. This produces
the label "CREATEFOREIGNTABLESTMTCREATESTMT". The attached patch splits
things out the way we normally do in outfuncs.c. There's no readfuncs.c
support, so this is strictly cosmetic.

- _outColumnDef() uses WRITE_INT_FIELD for the "storage" field, a char.
Again, no readfuncs.c support to create a compatibility problem.

- _copyRenameStmt() and _equalRenameStmt() ignore the "relationType" field,
but I can't see a good reason to do so. PostgreSQL 9.1 added this field,
but only recent master (after commit 38b9693f of 3 April 2012) references
the field beyond setting it in the parser.

- _copyViewStmt() and _equalViewStmt() ignore the "options" field, and
_equalColumnDef() ignores "fdwoptions". These are new in PostgreSQL 9.2,
and I see no good reason to ignore them.

I'd suggest backpatching the ReassignOwnedStmt() bits; the wrong code could
produce crashes. The rest are for master only.

Thanks,
nm

Attachment Content-Type Size
node-support-cleanup-v1.patch text/plain 4.7 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kyotaro HORIGUCHI 2012-04-16 12:05:48 [BUG] Checkpointer on hot standby runs without looking checkpoint_segments
Previous Message Heikki Linnakangas 2012-04-16 10:09:09 Re: Why can't I use pgxs to build a plpgsql plugin?