Fujii Masao
masao****@gmail*****
2013年 10月 1日 (火) 01:59:25 JST
On Mon, Sep 30, 2013 at 4:28 PM, Beena Emerson <memis****@gmail*****> wrote: > On Mon, Sep 30, 2013 at 12:34 PM, Fujii Masao <masao****@gmail*****> wrote: >> >> On Mon, Sep 30, 2013 at 3:54 PM, Beena Emerson <memis****@gmail*****> >> wrote: >> > On Mon, Sep 30, 2013 at 8:45 AM, Fujii Masao <masao****@gmail*****> >> > wrote: >> >> >> >> >> >> > - pg_trgm.sql is a copy of the regression test file of pg_trgm and is >> >> > used >> >> > to test the trgm functions. >> >> >> >> I don't feel inclined to add the pg_trgm regression test into pg_bigm. >> >> >> >> Is it really worth testing the pg_trgm in that situation? I was >> >> thinking >> >> that >> >> checking whether we can register pg_bigm after registering pg_trgm is >> >> sufficient as the test of co-existence of them. You're thinking that >> >> changes >> >> in pg_bigm can affect the behavior of pg_trgm? >> >> >> > When we applied the similarity patch,the pg_trgm functions did not give >> > expected values in some scenarios even when pg_bigm tests passed. So I >> > added >> > them. >> >> Could you elaborate this? I was thinking that if we can register >> pg_bigm and pg_trgm >> at the same time, it ensures that there is no conflicting function or >> operator name in them. >> Then, if there is no conflicting one, we can ensure that pg_bigm >> doesn't affect the >> behavior of pg_trgm at all. Maybe I'm missing something. > > > Yes if there are same SQL functions and operators declared, then we cannot > register both the modules at the same time. > > However, the problem was caused because the internal C functions had the > same name. > In the similarity patch it was cnt_sml. In this case, we were able to > register both pg_bigm and pg_trgm because both had different names for SQL > functions but while running the functions, the output was different. > > Eg: when using the similarity(text,text) function of pg_trgm, we got the > wrong result. > > # SELECT similarity('wow','wow'); > similarity > ------------ > 0 > (1 row) > Expected output is 1. > > This happened because the pg_bigm cnt_sml was called during the execution > instead of pg_trgm's cnt_sml. > Similarly with bigm_similarity, sometimes the pg_trgm's cnt_sml was called > and it resulted in wrong outputs. > > So, we had different SQL functions similarity and bigm_similarity which > allowed us to install both modules, however the other C functions called > during the execution had similar names and hence caused unexpected outputs. > > One more way to handle this, like Amit pointed out, is the declare the > functions as static in both modules. i.e. if the cnt_sml was declared static > in both pg_bigm and pg_trgm then this problem could have been avoided. But > since we cannot change pg_trgm code, we changed the pg_bigm function to > cnt_bigm_sml. Hmm... I was not able to reproduce this problem. I renamed cnt_bigm_sml to cnt_sml in the patch you had submitted, applied it to HEAD of pg_bigm, and executed similarity('wow', 'wow'). Its result was 1. When I applied the previous version of similarity patch (i.e., the version which Amit submitted) and compile the source, I got the following warning message. bigm_op.c:205: warning: no previous prototype for 'cnt_bigm_sml' I wonder if this is the root cause of the problem which you encountered. Thought? Regards, -- Fujii Masao
Fork