Re: [HACKERS] logical changeset generation v6.5
On 11/11/2013 02:06 PM, Andres Freund wrote: On 2013-11-10 14:45:17 -0500, Steve Singer wrote: Not really keen - that'd be a noticeable overhead. Note that in the cases where DEFAULT|INDEX is used, you can just use the new tuple to extract what you need for the pkey lookup since they now have the same format and since it's guaranteed that the relevant columns haven't changed if oldtup is null and there's a key. What are you actually doing with those columns? Populating a WHERE clause? Yup building a WHERE clause Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical changeset generation v6.5
On 11/9/13, 5:56 AM, Andres Freund wrote: ISTM ecpg's regression tests should be built (not run!) during $(recurse) not just during make check. Actually, I did just the opposite change some years ago. The rationale is, the build builds that which you want to install. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical changeset generation v6.5
On 2013-11-10 14:45:17 -0500, Steve Singer wrote: On 11/10/2013 09:41 AM, Andres Freund wrote: Still give me the following: update disorder.do_inventory set ii_in_stock=2 where ii_id=251; UPDATE 1 test1=# LOG: tuple in table with oid: 35122 without primary key Hm. Could it be that you still have an older test_decoding plugin lying around? The current one doesn't contain that string anymore. That'd explain the problems. In v6.4 the output plugin API was changed that plain heaptuples are passed for the old key, although with non-key columns set to NULL. Earlier it was a index tuple as defined by the indexes TupleDesc. Grrr, yah that was the problem I had compiled but not installed the newer plugin. Sorry. Heh, happened to me several times during development ;) Which I suspect means oldtuple is back to null Which is legitimate though, if you don't update the primary (or explicitly chosen candidate) key. Those only get logged if there's actual changes in those columns. Makes sense? Is the expectation that plugin writters will call RelationGetIndexAttrBitmap(relation,INDEX_ATTR_BITMAP_IDENTITY_KEY); to figure out what the identity key is. I'd expect them to check whether relreplident is FULL, NOTHING or DEFAULT|INDEX. In the latter case they can check Relation-rd_replidindex. The bitmap doesn't really seem to be helpful? How do we feel about having the decoder logic populate change.oldtuple with the identity on UPDATE statements when it is null? Not really keen - that'd be a noticeable overhead. Note that in the cases where DEFAULT|INDEX is used, you can just use the new tuple to extract what you need for the pkey lookup since they now have the same format and since it's guaranteed that the relevant columns haven't changed if oldtup is null and there's a key. What are you actually doing with those columns? Populating a WHERE clause? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical changeset generation v6.5
On 2013-11-09 20:16:20 -0500, Steve Singer wrote: When I try the test_decoding plugin on UPDATE I get rows like: table do_inventory: UPDATE: ii_id[int8]:251 ii_in_stock[int8]:1 ii_reserved[int8]:144 ii_total_sold[int8]:911 which I think is only data from the new tuple.The lack of old-key in the output makes me think the test decoding plugin also isn't getting the old tuple. (This is with your patch-set rebased ontop of ac4ab97ec05ea900db0f14d428cae2e79832e02d which includes the patches Robert committed the other day, I can't rule out that I didn't break something in the rebase). I've pushed an updated tree to git, that contains that http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=shortlog;h=refs/heads/xlog-decoding-rebasing-remapping git://git.postgresql.org/git/users/andresfreund/postgres.git and some more fixes. I'll send out an email with details sometime soon. 93c5c2a171455763995cef0afa907bcfaa405db4 Still give me the following: update disorder.do_inventory set ii_in_stock=2 where ii_id=251; UPDATE 1 test1=# LOG: tuple in table with oid: 35122 without primary key Hm. Could it be that you still have an older test_decoding plugin lying around? The current one doesn't contain that string anymore. That'd explain the problems. In v6.4 the output plugin API was changed that plain heaptuples are passed for the old key, although with non-key columns set to NULL. Earlier it was a index tuple as defined by the indexes TupleDesc. a) The table does have a primary key b) I don't get anything in the old key when I was expecting all the rows c) If I change the table to use the pkey index with alter table disorder.do_inventory replica identity using index do_inventory_pkey; The LOG message on the update goes away but the output of the test decoder plugin goes back to table do_inventory: UPDATE: ii_id[int8]:251 ii_in_stock[int8]:5 ii_reserved[int8]:144 ii_total_sold[int8]:911 Which I suspect means oldtuple is back to null Which is legitimate though, if you don't update the primary (or explicitly chosen candidate) key. Those only get logged if there's actual changes in those columns. Makes sense? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical changeset generation v6.5
On 11/10/2013 09:41 AM, Andres Freund wrote: Still give me the following: update disorder.do_inventory set ii_in_stock=2 where ii_id=251; UPDATE 1 test1=# LOG: tuple in table with oid: 35122 without primary key Hm. Could it be that you still have an older test_decoding plugin lying around? The current one doesn't contain that string anymore. That'd explain the problems. In v6.4 the output plugin API was changed that plain heaptuples are passed for the old key, although with non-key columns set to NULL. Earlier it was a index tuple as defined by the indexes TupleDesc. Grrr, yah that was the problem I had compiled but not installed the newer plugin. Sorry. a) The table does have a primary key b) I don't get anything in the old key when I was expecting all the rows c) If I change the table to use the pkey index with alter table disorder.do_inventory replica identity using index do_inventory_pkey; The LOG message on the update goes away but the output of the test decoder plugin goes back to table do_inventory: UPDATE: ii_id[int8]:251 ii_in_stock[int8]:5 ii_reserved[int8]:144 ii_total_sold[int8]:911 Which I suspect means oldtuple is back to null Which is legitimate though, if you don't update the primary (or explicitly chosen candidate) key. Those only get logged if there's actual changes in those columns. Makes sense? Is the expectation that plugin writters will call RelationGetIndexAttrBitmap(relation,INDEX_ATTR_BITMAP_IDENTITY_KEY); to figure out what the identity key is. How do we feel about having the decoder logic populate change.oldtuple with the identity on UPDATE statements when it is null? The logic I have now is to use oldtuple if it is not null, otherwise go figure out which columns from the identiy key we should be using. I think most plugins that do anything useful with an update will need to duplicate that Greetings, Andres Freund -- Andres Freundhttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical changeset generation v6.5
On 2013-11-08 17:11:58 -0500, Peter Eisentraut wrote:
On 11/8/13, 3:03 PM, Robert Haas wrote:
On Fri, Nov 8, 2013 at 12:38 PM, Robert Haas robertmh...@gmail.com wrote:
On Tue, Nov 5, 2013 at 10:21 AM, Andres Freund and...@2ndquadrant.com
wrote:
Attached to this mail and in the xlog-decoding-rebasing-remapping branch
in my git[1] repository you can find the next version of the patchset
that:
I have pushed patches #1 and #2 from this series as a single commit,
after some editing.
And I've also pushed patch #13, which is an almost-totally-unrelated
improvement that has nothing to do with logical replication, but is
useful all the same.
Please fix this new compiler warning:
pg_regress_ecpg.c: In function ‘main’:
pg_regress_ecpg.c:170:2: warning: passing argument 3 of ‘regression_main’
from incompatible pointer type [enabled by default]
In file included from pg_regress_ecpg.c:19:0:
../../../../src/test/regress/pg_regress.h:55:5: note: expected
‘init_function’ but argument is of type ‘void (*)(void)’
Hrmpf...
I usually run something akin to
# make -j3 -s (cd contrib make -j3 -s)
and then in a separate step
# make -s check-world
this is so I see compiler warnings before drowning them in check-world's
output. But ecpg/test isn't built during make in src/interfaces/ecpg,
but just during make check there.
ISTM ecpg's regression tests should be built (not run!) during
$(recurse) not just during make check. Patch towards that end attached.
Also attached is the fix for the compilation warning itself.
Greetings,
Andres Freund
--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training Services
From 99aaf866b315af21fddd858c3d2922ca21918f8c Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Sat, 9 Nov 2013 11:49:27 +0100
Subject: [PATCH 1/2] Recurse into ecpg/test during normal builds, instead just
during make check.
ecpg's make check, which currently compiles the contents of ecpg/test, is only
executed during make check-world and not make all making warnings during
the compilation of the tests hard to spot manually.
Instead build the test framework during a normal toplevel make all.
---
src/interfaces/ecpg/Makefile | 10 --
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/src/interfaces/ecpg/Makefile b/src/interfaces/ecpg/Makefile
index e397210..4f05ae4 100644
--- a/src/interfaces/ecpg/Makefile
+++ b/src/interfaces/ecpg/Makefile
@@ -2,7 +2,7 @@ subdir = src/interfaces/ecpg
top_builddir = ../../..
include $(top_builddir)/src/Makefile.global
-SUBDIRS = include pgtypeslib ecpglib compatlib preproc
+SUBDIRS = include pgtypeslib ecpglib compatlib preproc test
# Suppress parallel build of subdirectories to avoid a bug in gmake 3.82, cf
# http://savannah.gnu.org/bugs/?30653
@@ -16,15 +16,13 @@ endif
$(recurse)
-all-pgtypeslib-recurse all-ecpglib-recurse all-compatlib-recurse all-preproc-recurse: all-include-recurse
+all-pgtypeslib-recurse all-ecpglib-recurse all-compatlib-recurse all-preproc-recurse all-test-recurse: all-include-recurse
all-compatlib-recurse: all-ecpglib-recurse
all-ecpglib-recurse: all-pgtypeslib-recurse
-install-pgtypeslib-recurse install-ecpglib-recurse install-compatlib-recurse install-preproc-recurse: install-include-recurse
+install-pgtypeslib-recurse install-ecpglib-recurse install-compatlib-recurse install-preproc-recurse insta--test-recurse: install-include-recurse
install-compatlib-recurse: install-ecpglib-recurse
install-ecpglib-recurse: install-pgtypeslib-recurse
-
-clean distclean maintainer-clean:
- $(MAKE) -C test clean
+all-test-recurse: all-preproc-recurse all-ecpglib-recurse all-compatlib-recurse
check checktcp installcheck: all
$(MAKE) -C test $@
--
1.8.3.251.g1462b67
From a0aa8180a849751df6fdcf49a83ff87d906c8aed Mon Sep 17 00:00:00 2001
From: Andres Freund and...@anarazel.de
Date: Sat, 9 Nov 2013 11:53:33 +0100
Subject: [PATCH 2/2] ecpg: Adapt to recent pg_regress init_function API
changes
This fixes the warning during ecpg's test compilation introduced in
9b4d52f2095be96ca238ce41f6963ec56376491f.
---
src/interfaces/ecpg/test/pg_regress_ecpg.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/interfaces/ecpg/test/pg_regress_ecpg.c b/src/interfaces/ecpg/test/pg_regress_ecpg.c
index d01703e..740b566 100644
--- a/src/interfaces/ecpg/test/pg_regress_ecpg.c
+++ b/src/interfaces/ecpg/test/pg_regress_ecpg.c
@@ -159,7 +159,7 @@ ecpg_start_test(const char *testname,
}
static void
-ecpg_init(void)
+ecpg_init(int argc, char *argv[])
{
/* nothing to do here at the moment */
}
--
1.8.3.251.g1462b67
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical changeset generation v6.5
On 11/05/2013 10:21 AM, Andres Freund wrote: Hi, Attached to this mail and in the xlog-decoding-rebasing-remapping branch in my git[1] repository you can find the next version of the patchset that: * Fixes full table rewrites of catalog tables using the method Robert prefers (which is to log rewrite mappings to disk) * Extract the REPLICA IDENTITY as configured with ALTER TABLE for the old tuple for UPDATEs and DELETEs * Much better support for synchronous replication * Better resource cleanup (as in we need less local WAL available) * Lots of smaller fixes The change around REPLICA IDENTITY is *incompatible* to older output plugins since we now log tuples using the table's TupleDesc, not the indexes. My updated plugin is getting rows with change-tp.oldtuple as NULL on updates either with the default PRIMARY KEY identify or with a FULL identity. When I try the test_decoding plugin on UPDATE I get rows like: table do_inventory: UPDATE: ii_id[int8]:251 ii_in_stock[int8]:1 ii_reserved[int8]:144 ii_total_sold[int8]:911 which I think is only data from the new tuple.The lack of old-key in the output makes me think the test decoding plugin also isn't getting the old tuple. (This is with your patch-set rebased ontop of ac4ab97ec05ea900db0f14d428cae2e79832e02d which includes the patches Robert committed the other day, I can't rule out that I didn't break something in the rebase). Robert, I'd be very grateful if you could have a look at patch 0007 implementing what we've discussed. I kept it separate to make it easier to look at it in isolation, but I think in the end it partially should be merged into the wal_level=logical patch. I still think the wide cmin/cmax solution is more elegant and has wider applicability, but this works as well although it's about 5 times the code. Comments? [1]: http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=summary Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical changeset generation v6.5
On 2013-11-09 17:36:49 -0500, Steve Singer wrote: On 11/05/2013 10:21 AM, Andres Freund wrote: Hi, Attached to this mail and in the xlog-decoding-rebasing-remapping branch in my git[1] repository you can find the next version of the patchset that: * Fixes full table rewrites of catalog tables using the method Robert prefers (which is to log rewrite mappings to disk) * Extract the REPLICA IDENTITY as configured with ALTER TABLE for the old tuple for UPDATEs and DELETEs * Much better support for synchronous replication * Better resource cleanup (as in we need less local WAL available) * Lots of smaller fixes The change around REPLICA IDENTITY is *incompatible* to older output plugins since we now log tuples using the table's TupleDesc, not the indexes. My updated plugin is getting rows with change-tp.oldtuple as NULL on updates either with the default PRIMARY KEY identify or with a FULL identity. When I try the test_decoding plugin on UPDATE I get rows like: table do_inventory: UPDATE: ii_id[int8]:251 ii_in_stock[int8]:1 ii_reserved[int8]:144 ii_total_sold[int8]:911 which I think is only data from the new tuple.The lack of old-key in the output makes me think the test decoding plugin also isn't getting the old tuple. (This is with your patch-set rebased ontop of ac4ab97ec05ea900db0f14d428cae2e79832e02d which includes the patches Robert committed the other day, I can't rule out that I didn't break something in the rebase). I've pushed an updated tree to git, that contains that http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=shortlog;h=refs/heads/xlog-decoding-rebasing-remapping git://git.postgresql.org/git/users/andresfreund/postgres.git and some more fixes. I'll send out an email with details sometime soon. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical changeset generation v6.5
On 11/09/2013 05:42 PM, Andres Freund wrote:
On 2013-11-09 17:36:49 -0500, Steve Singer wrote:
On 11/05/2013 10:21 AM, Andres Freund wrote:
Hi,
Attached to this mail and in the xlog-decoding-rebasing-remapping branch
in my git[1] repository you can find the next version of the patchset that:
* Fixes full table rewrites of catalog tables using the method Robert
prefers (which is to log rewrite mappings to disk)
* Extract the REPLICA IDENTITY as configured with ALTER TABLE for the
old tuple for UPDATEs and DELETEs
* Much better support for synchronous replication
* Better resource cleanup (as in we need less local WAL available)
* Lots of smaller fixes
The change around REPLICA IDENTITY is *incompatible* to older output
plugins since we now log tuples using the table's TupleDesc, not the
indexes.
My updated plugin is getting rows with
change-tp.oldtuple as NULL on updates either with the default PRIMARY KEY
identify or with a FULL identity.
When I try the test_decoding plugin on UPDATE I get rows like:
table do_inventory: UPDATE: ii_id[int8]:251 ii_in_stock[int8]:1
ii_reserved[int8]:144 ii_total_sold[int8]:911
which I think is only data from the new tuple.The lack of old-key in
the output makes me think the test decoding plugin also isn't getting the
old tuple.
(This is with your patch-set rebased ontop of
ac4ab97ec05ea900db0f14d428cae2e79832e02d which includes the patches Robert
committed the other day, I can't rule out that I didn't break something in
the rebase).
I've pushed an updated tree to git, that contains that
http://git.postgresql.org/gitweb/?p=users/andresfreund/postgres.git;a=shortlog;h=refs/heads/xlog-decoding-rebasing-remapping
git://git.postgresql.org/git/users/andresfreund/postgres.git
and some more fixes. I'll send out an email with details sometime soon.
93c5c2a171455763995cef0afa907bcfaa405db4
Still give me the following:
update disorder.do_inventory set ii_in_stock=2 where ii_id=251;
UPDATE 1
test1=# LOG: tuple in table with oid: 35122 without primary key
\d disorder.do_inventory
Table disorder.do_inventory
Column | Type | Modifiers
---++---
ii_id | bigint | not null
ii_in_stock | bigint |
ii_reserved | bigint |
ii_total_sold | bigint |
Indexes:
do_inventory_pkey PRIMARY KEY, btree (ii_id)
Foreign-key constraints:
do_inventory_item_ref FOREIGN KEY (ii_id) REFERENCES
disorder.do_item(i_id) ON DELETE CASCADE
Referenced by:
TABLE disorder.do_item CONSTRAINT do_item_inventory_ref FOREIGN
KEY (i_id) REFERENCES disorder.do_inventory(ii_id) DEFERRABLE INITIALLY
DEFERRED
TABLE disorder.do_restock CONSTRAINT do_restock_inventory_ref
FOREIGN KEY (r_i_id) REFERENCES disorder.do_inventory(ii_id) ON DELETE
CASCADE
Triggers:
_disorder_replica_truncatetrigger BEFORE TRUNCATE ON
disorder.do_inventory FOR EACH STATEMENT EXECUTE PROCEDURE
_disorder_replica.log_truncate('3')
Disabled triggers:
_disorder_replica_denyaccess BEFORE INSERT OR DELETE OR UPDATE ON
disorder.do_inventory FOR EACH ROW EXECUTE PROCEDURE
_disorder_replica.denyaccess('_disorder_replica')
_disorder_replica_truncatedeny BEFORE TRUNCATE ON
disorder.do_inventory FOR EACH STATEMENT EXECUTE PROCEDURE
_disorder_replica.deny_truncate()
Replica Identity: FULL
The test decoder plugin gives me:
table do_inventory: UPDATE: old-pkey:
a) The table does have a primary key
b) I don't get anything in the old key when I was expecting all the rows
c) If I change the table to use the pkey index with
alter table disorder.do_inventory replica identity using index
do_inventory_pkey;
The LOG message on the update goes away but the output of the test
decoder plugin goes back to
table do_inventory: UPDATE: ii_id[int8]:251 ii_in_stock[int8]:5
ii_reserved[int8]:144 ii_total_sold[int8]:911
Which I suspect means oldtuple is back to null
Greetings,
Andres Freund
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical changeset generation v6.5
On Tue, Nov 5, 2013 at 10:21 AM, Andres Freund and...@2ndquadrant.com wrote: Attached to this mail and in the xlog-decoding-rebasing-remapping branch in my git[1] repository you can find the next version of the patchset that: I have pushed patches #1 and #2 from this series as a single commit, after some editing. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical changeset generation v6.5
On Fri, Nov 8, 2013 at 12:38 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Nov 5, 2013 at 10:21 AM, Andres Freund and...@2ndquadrant.com wrote: Attached to this mail and in the xlog-decoding-rebasing-remapping branch in my git[1] repository you can find the next version of the patchset that: I have pushed patches #1 and #2 from this series as a single commit, after some editing. And I've also pushed patch #13, which is an almost-totally-unrelated improvement that has nothing to do with logical replication, but is useful all the same. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] logical changeset generation v6.5
On 11/8/13, 3:03 PM, Robert Haas wrote: On Fri, Nov 8, 2013 at 12:38 PM, Robert Haas robertmh...@gmail.com wrote: On Tue, Nov 5, 2013 at 10:21 AM, Andres Freund and...@2ndquadrant.com wrote: Attached to this mail and in the xlog-decoding-rebasing-remapping branch in my git[1] repository you can find the next version of the patchset that: I have pushed patches #1 and #2 from this series as a single commit, after some editing. And I've also pushed patch #13, which is an almost-totally-unrelated improvement that has nothing to do with logical replication, but is useful all the same. Please fix this new compiler warning: pg_regress_ecpg.c: In function ‘main’: pg_regress_ecpg.c:170:2: warning: passing argument 3 of ‘regression_main’ from incompatible pointer type [enabled by default] In file included from pg_regress_ecpg.c:19:0: ../../../../src/test/regress/pg_regress.h:55:5: note: expected ‘init_function’ but argument is of type ‘void (*)(void)’ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
