Re: %TYPE and array declaration patch review

From: Wojciech Muła <wojciech_mula(at)poczta(dot)onet(dot)pl>
To: Greg Smith <greg(at)2ndQuadrant(dot)com>
Cc: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: %TYPE and array declaration patch review
Date: 2011-12-10 18:07:43
Message-ID: 20111210190743.2576a1ef@cat.tac
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, 10 Dec 2011 10:22:54 -0500 Greg Smith <greg(at)2ndQuadrant(dot)com>
wrote:

> Tom's concerns about the grammar rewrite and way parsing is handled
> here seem the worst blockers for committing this, and I can't imagine
> how those could be resolved before this CommitFest is over. I'm
> going to mark this one as returned with that and Pavel's feedback.
> Perhaps Wojciech or someone else will come up with a clever way to
> address this, one that has less impact on existing code. Adding more
> parser complexity than absolutely necessary is a much bigger problem
> than the regression tests being too long.

My first attempt to fix the issue was very short & simple, I slightly
modified existing C code to handle additional suffixes, see
http://archives.postgresql.org/pgsql-hackers/2011-08/msg00998.php

Pavel reviewed this and pointed that would be great if we were fix
related issues. Thus, the "second" patch try to resolve:
1. handling array indices;
2. copying types;
3. handling %TYPE/%ROWTYPE in procedure definition.

Ad 1 - IMHO first attempt is good enough :) (except coding style)

Ad 2 - this is broken now, just few cases are handled correctly.
My code handled more cases manually, but now I see this have to
be solved with kind of generic pattern-matching procedure (maybe
with backtracking) - I hope shorter, cleaner and easier to understand
and maintain. Tom Lane wrote it would be good if resolving types were
handled in core, but IMO only PL/pgsql code will use this procedure.

Ad 3 - minor issue, could be fixed anytime

So, my proposition is to drop current approach and back to the
first proposition. Copying types have to be considered as separate
issue.

regards
Wojciech Muła

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Greg Smith 2011-12-10 18:07:59 CommitFest 2011-11 Update
Previous Message Brar Piening 2011-12-10 17:58:34 Re: Review of VS 2010 support patches