Re: ask for review of MERGE - Mailing list pgsql-hackers

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