Re: ask for review of MERGE

Starting looking at the latest MERGE patch from Boxuan here tonight. The 
regression tests pass for me here, good starting sign. I expect to move 
onto trying to break it more creatively next, then onto performance 
tests. Nothing much more exciting than that to report so far.

It had suffered some bit rot, I think because of the security label 
changes. Attached is a rebased version against the new git HEAD so 
nobody else has to duplicate that to apply the patch. Also, to provide 
an alternate interface for anyone who wants to do testing/browsing of 
this patch, I've made a Github fork with a merge branch in it. I plan to 
commit intermediate stuff to there that keeps up to date with review 
changes: http://github.com/greg2ndQuadrant/postgres/tree/merge

Probably easier to read 
http://github.com/greg2ndQuadrant/postgres/compare/merge than most local 
patch viewers, so I gzip'ed the attached updated patch to save some bytes.

One compiler warning I noticed that needs to get resolved:

src/backend/commands/explain.c:

explain.c: In function ‘ExplainMergeActions’:
explain.c:1528: warning: comparison of distinct pointer types lacks a cast

That is complaining about this section:

if (mt_planstate->operation != CMD_MERGE ||
mt_planstate->mt_mergeActPstates == NIL)
return;

So presumably that comparison with NIL needs a cast. Once I get more 
familiar with the code I'll fix that myself if Boxuan doesn't offer a 
suggestion first.

The rest of the compiler warnings I saw didn't look related to his code, 
maybe stuff my picky Ubuntu compiler is noticing that was done recently 
to HEAD. I haven't checked HEAD without this patch yet to confirm, and 
am done for the night now. Here's the list if anyone is interested:

Warning in src/backend/parser/scan.c:

gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
-Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing 
-fwrapv -g -I../../../src/include -D_GNU_SOURCE -c -o index.o index.c 
-MMD -MP -MF .deps/index.Po
In file included from gram.y:12172:
scan.c: In function ‘yy_try_NUL_trans’:
scan.c:16256: warning: unused variable ‘yyg’

Warning in src/backend/utils/error/elog.c:

gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
-Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing 
-fwrapv -g -I../../../../src/include -D_GNU_SOURCE -c -o ts_cache.o 
ts_cache.c -MMD -MP -MF .deps/ts_cache.Po
elog.c: In function ‘write_console’:
elog.c:1698: warning: ignoring return value of ‘write’, declared with 
attribute warn_unused_result
elog.c: In function ‘write_pipe_chunks’:
elog.c:2388: warning: ignoring return value of ‘write’, declared with 
attribute warn_unused_result
elog.c:2397: warning: ignoring return value of ‘write’, declared with 
attribute warn_unused_result

Warning in src/bin/scripts/common.c:

gcc -O2 -Wall -Wmissing-prototypes -Wpointer-arith 
-Wdeclaration-after-statement -Wendif-labels -fno-strict-aliasing 
-fwrapv -g -I. -I. -I../../../src/interfaces/libpq 
-I../../../src/bin/pg_dump -I../../../src/include -D_GNU_SOURCE -c -o 
input.o input.c -MMD -MP -MF .deps/input.Po
common.c: In function ‘handle_sigint’:
common.c:247: warning: ignoring return value of ‘write’, declared with 
attribute warn_unused_result
common.c:250: warning: ignoring return value of ‘write’, declared with 
attribute warn_unused_result
common.c:251: warning: ignoring return value of ‘write’, declared with 
attribute warn_unused_result

-- 
Greg Smith, 2ndQuadrant US greg@2ndQuadrant.com Baltimore, MD
PostgreSQL Training, Services and Support  www.2ndQuadrant.us
Author, "PostgreSQL 9.0 High Performance"    Pre-ordering at:
https://www.packtpub.com/postgresql-9-0-high-performance/book