Fork
On 2020/02/20 21:42, torikoshia wrote:
> Thanks for your kind review!
>
> Attached a fixed patch.
Thanks for updating the patch!
> On 2020-02-20 01:46, Fujii Masao wrote:
>> Could you tell me why you used "completed" instead of "complete" when
>> "terse" is specified? IMO it's better to use the same word in both
>> "on" and "terse" cases.
>
> You're right.
> I've changed "completed" to "complete".
Thanks!
> I intended to be consistent with pgs2malloc()
> log message which used past tense 'called', but
> this message was added in this patch.
>
> Should I also fix this message?
> If so, I'll also fix it.
> However, it’s just me but I feel a little bit
> strange to change "called" to "call" here.
>
> | elog(LOG, "pgs2malloc(): called");
Or what about just reporting "pgs2malloc()" for
the consistency with the detailed information of log?
Your patch seems to forget to treat the following log message.
elog(LOG, "GetSennaQuery(): quick exit: %s", tmp);
I think that it's better to comment the reason why 'terse' is enough
for this regression test. So I'd like to add something like the following
comments based on the comments that you added. Thought?
---------------------------
Test whether recheck is skipped expectedly when keyword length is 1 or 2.
We can judge that recheck is skipped successfully if the debug messages
indicating the calls to pgs2norm() and pgs2malloc() are logged only once
respectively. So this test must run with enable_debug enabled.
Note that enable_debug must be set to 'terse' in order to stabilize the result
of this regression test. Otherwise, i.e., when enable_debug is set to 'on',
the contents dealt in function pgs2malloc() and pgs2norm() are logged,
and the order of the those debug messages varies depending on the block
size in PostgreSQL server. Those detailed information are not necessary to
judge whether recheck is skipped or not.
---------------------------
Regards,
--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters