Re: [HACKERS] ask for review of MERGE

[Greg Smith]

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;





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


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:


elog.c: In function ‘write_console’:

elog.c: In function ‘write_pipe_chunks’:


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


common.c: In function ‘handle_sigint’:


--
Greg Smith, 2ndQuadrant US g...@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

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: 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 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:1698: warning: ignoring return value of ‘write’, declared with attribute warn_unused_result 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 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: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

merge_v203-rebase.patch.gz
Description: GNU Zip compressed data

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