On Tue, Sep 18, 2012 at 05:52:51PM +0200, Marco Nenciarini wrote:
>     please find attached the refreshed v1 patch.

I perused this version in comparison to the last version I reviewed, finding
minor problems.  First, a warning:

tablecmds.c: In function `ATExecAddConstraint':
tablecmds.c:5898: warning: `fk_element_type' may be used uninitialized in this 
function
tablecmds.c:5898: note: `fk_element_type' was declared here

I don't see an actual bug; add a dead store to silence the compiler.

> *** a/src/test/regress/parallel_schedule
> --- b/src/test/regress/parallel_schedule
> *************** test: event_trigger
> *** 94,100 ****
>   # ----------
>   # Another group of parallel tests
>   # ----------
> ! test: select_views portals_p2 foreign_key cluster dependency guc bitmapops 
> combocid tsearch tsdicts foreign_data window xmlmap functional_deps 
> advisory_lock json
>   
>   # ----------
>   # Another group of parallel tests
> --- 94,100 ----
>   # ----------
>   # Another group of parallel tests
>   # ----------
> ! test: select_views portals_p2 foreign_key cluster dependency guc bitmapops 
> combocid tsearch tsdicts foreign_data window xmlmap functional_deps 
> advisory_lock element_foreign_key

Keep that json test.

> +                             errmsg("array ELEMENT foreign keys only support 
> NO ACTION "
> +                                        "and RESTRICT actions")));

Project style is not to break message literals; instead, let the line run
long.  There are a few more examples of this in your patch.


Those problems are isolated and do not impugn design, so committer time would
be just as well-spent on the latest version.  As such, I'm marking the patch
Ready for Committer.  Thanks to Rafal Pietrak for his helpful review.

nm


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to