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
windowxmlmap 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
windowxmlmap 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
