On Mon, Mar 19, 2018 at 7:57 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
> On Mon, Mar 19, 2018 at 12:50 PM, Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>> On Fri, Mar 16, 2018 at 10:24 AM, Alvaro Herrera
>> <alvherre@alvh.no-ip.org> wrote:
>>> Masahiko Sawada wrote:
>>>> On Thu, Mar 15, 2018 at 9:41 AM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>>>
>>>> > 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.
>>>
>>> Something similar to PostgresNode::issues_sql_like() perhaps?
>>>
>>
>> Yeah, I didn't know that but I think it's a good idea. Unlike
>> issues_sql_like() we don't issue anything to the subscriber. So maybe
>> we need truncate logfile before insertion and verify logfile of
>> particular period. The test code would be like follows.
>>
>> $node_subscriber->safe_psql('postgres', 'CREATE SUBSCRIPTION...");
>> truncate $node_subscriber->logfile, 0;
>> $node_publisher->safe_psql('postgres', 'INSERT .. ')
>> my $log = TestLib::slurp_file($node_subscriber->logfile);
>>
>> # Verify logs
>> like($log, qr/processing remote data for replication target relation
>> "public.test" column "b", remote type dummyint, local type dummyint/,
>> 'callback function of datatype conversion1');
>> like($log, qr/processing remote data for replication target relation
>> "public.test" column "a", remote type dummytext, local type
>> dummytext/, 'callback function of datatype conversion2');
>>
>> Thoughts?
>>
>
> Attached an updated test patch added the above test(0002 patch). Since
> for this test case it's enough to use existing test functions I didn't
> create new test functions. Also I found that the local data type name
> in log for data type conversion isn't qualified whereas the remote
> data type is always qualified. Attached 0001 patch fixes that.
>
The original issue has been fixed and this entry on CommitFest has
been marked as "Committed" but there are still works for improving
testing. Perhaps I should register a new entry of remaining patches to
next CommitFest.
Regards,
--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center