Hello, The new patch looks good for me.

The usage of this is a bit irregular as a (extension) module but
it is the nature of this 'contrib'. The rearranged page type
detection logic seems neater and keeps to work as intended. This
logic will be changed after the new page type detection scheme
becomes ready by the another patch.


I have some additional comments, which should be the last
ones. All of the comments are about test.sh.

- A question mark seems missing from the end of the message "Has
  build been done with -DBUFFER_CAPTURE included in CFLAGS" in
  test.sh.

- "make check" runs "regress --use-existing" but IMHO the make
  target which is expected to do that is installcheck. I had
  fooled to convince that it should run the postgres which is
  built dedicatedly:(

- postgres processes are left running after
  test_default(custom).sh has stopped halfway. This can be fixed
  with the attached patch, but, to be honest, this seems too
  much. I'll follow your decision whether or not to do this.
  (bufcapt_test_sh_20140710.patch)

- test_default.sh is not only an example script which will run
  while utilize this facility, but the test script for this
  facility itself.

  So I think it would be better be added some queries so that all
  possible page types available for the default testing. What do
  you think about the attached patch?  (hash index is unlogged
  but I dared to put it for clarity.)

  (bufcapt_test_default_sh_20140710.patch)

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center
diff --git a/contrib/buffer_capture_cmp/test.sh b/contrib/buffer_capture_cmp/test.sh
index 89740bb..ba5e290 100644
--- a/contrib/buffer_capture_cmp/test.sh
+++ b/contrib/buffer_capture_cmp/test.sh
@@ -117,16 +117,27 @@ pg_ctl -w -D $TEST_STANDBY start
 # Check the presence of custom tests and kick them in priority. If not,
 # fallback to the default tests. Tests need only to be run on the master
 # node.
+
 if [ -f ./test-custom.sh ]; then
-	. ./test-custom.sh
+	TEST_SCRIPT=test-custom.sh
 else
-	. ./test-default.sh
+	TEST_SCRIPT=test-default.sh
 fi
 
+set +e
+bash -e $TEST_SCRIPT
+EXITSTATUS=$?
+set -e
+
 # Time to stop the nodes as tests have run
 pg_ctl -w -D $TEST_MASTER stop
 pg_ctl -w -D $TEST_STANDBY stop
 
+if [ $EXITSTATUS != 0 ]; then
+	echo "$TEST_SCRIPT exited by error"
+	exit 1;
+fi
+
 DIFF_FILE=capture_differences.txt
 
 # Check if the capture files exist. If not, build may have not been
diff --git a/contrib/buffer_capture_cmp/test-default.sh b/contrib/buffer_capture_cmp/test-default.sh
index 5bec503..24091ff 100644
--- a/contrib/buffer_capture_cmp/test-default.sh
+++ b/contrib/buffer_capture_cmp/test-default.sh
@@ -11,4 +11,16 @@
 # cd $ROOT_DIR
 
 # Create a simple table
-psql -c 'CREATE TABLE aa AS SELECT generate_series(1, 10) AS a'
+psql -c 'CREATE TABLE tbtree AS SELECT generate_series(1, 10) AS a'
+psql -c 'CREATE INDEX i_tbtree ON tbtree USING btree(a)'
+psql -c 'CREATE TABLE thash AS SELECT generate_series(1, 10) AS a'
+psql -c 'CREATE INDEX i_thash ON thash USING hash(a)'
+psql -c 'CREATE TABLE tgist AS SELECT POINT(a, a) AS p1 FROM generate_series(0, 10) a'
+psql -c 'CREATE INDEX i_tgist ON tgist USING gist(p1)'
+psql -c 'CREATE TABLE tspgist AS SELECT POINT(a, a) AS p1 FROM generate_series(0, 10) a'
+psql -c 'CREATE INDEX i_tspgist ON tspgist USING spgist(p1)'
+psql -c 'CREATE TABLE tgin AS SELECT ARRAY[a/10, a%10] as a1 from generate_series(0, 10) a'
+psql -c 'CREATE INDEX i_tgin ON tgin USING gin(a1)'
+psql -c 'CREATE SEQUENCE sq1'
+
+
-- 
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