Masahiko Sawada wrote:
> After more thought, I think since the errors in
> logicalrep_typmap_gettypname are not relevant with the actual error
> (i.g. type conversion error) it would not be good idea to show the
> error message of "could not found data type entry" in errcontext.
> It might be more appropriate if we return a substitute value
> ("unrecognized type" or "unrecognized built-in type") without raising
> neither an error nor a warning. Thoughts?
Yeah, seems a reasonable idea to me. Here's a tidied-up version of your
patch, minus the regression test changes (I may end up committing that
one separately). But I now hesitate to push it because I'm unsure of
how does type mapping actually work, and whether it's still working
after this patch -- for example if we create two user-defined datatypes
in opposite orders in the nodes (so they get different OIDs), are we
able to replicate data correctly from one side to the other? If there's
code to support this case, it is not at all obvious where it is.
> Regarding to regression test, I added a new test module
> test_subscription that creates a new user-defined data type.
I think this is a good thing to add. I wonder if we could have the
module create two extensions, and have the TAP test create first A then
B in one node, and first B in the other node, replicate a table that
uses type A, see how it fails, then create type A and ensure replication
works correctly. (Ideally the types would not be binary compatible, so
that it'd not work by accident.)
> 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.
This is bad. I haven't read it.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
