Masahiko Sawada wrote:
> Regarding to regression test, I added a new test module
> test_subscription that creates a new user-defined data type. In a
> subscription regression test, using test_subscription we make
> subscriber call slot_store_error_callback and check if the subscriber
> can correctly look up both remote and local data type strings. One
> downside of this regression test is that in a failure case, the
> duration of the test will be long (up to 180sec) because it has to
> wait for the polling timeout.
> Attached an updated patch with a regression test.
Pushed the fix to pg10 and master. Thanks to all involved for the
report, patches and review.
Here's the regression test patch. The problem with it is that the TAP
test is not verifying much -- I tried applying it before the fix commit,
and it succeeds! The only funny is that the errcontext messages are
wrong, they look like this:
2018-03-14 20:31:03.564 -03 [763018] LOG: input int: 1
2018-03-14 20:31:03.564 -03 [763018] CONTEXT: processing remote data for replication target relation "public.test"
column"b", remote type dummytext, local type dummyint
2018-03-14 20:31:03.564 -03 [763018] LOG: input text: "one"
2018-03-14 20:31:03.564 -03 [763018] CONTEXT: processing remote data for replication target relation "public.test"
column"a", remote type dummyint, local type dummytext
but I think it would be better to verify them. (With your version I
think you were trusting that the OID would not match anything, giving
raise to the "cache lookup failed" error before the patch. I'm not sure
that that's very trustworthy either.)
I think this is a worthwhile test, but IMO it should be improved a bit
before we include it. Also, we can come up with a better name for the
test surely, not just refer to this particular bug. Maybe "typemap".
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services