On Thu, Mar 15, 2018 at 9:41 AM, Alvaro Herrera <alvhe...@alvh.no-ip.org> wrote: > 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.
Thank you for committing! > > 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.) Agreed. Also, since my patch attempted to lead the error we need a long time to check if it failed, which is not good. > > 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". > It might be useful if we have the facility of TAP test to check the log message and regexp-match the message to the expected string. Regards, -- Masahiko Sawada NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center
