Thread: FTI contrib
Hi, The latest patch we submitted to the fulltextindex module improved lots of things but something we could not get to work was the apparently correct use of the PG_GETARG* macros, etc. Whenever we used these macros, we always got 0 or NULL as our values. So, we reverted to the trigger->tgargs array. Looking at the macro, it's accessing fcinfo->arg[n] array. For us, the fcinfo->arg array was always empty. What's going on? Although it works, someone with more experience might want to have a quick look at it. The problem is that we suspect it will fail if someone tries to FTI a TOAST-ed column. Chris
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> The latest patch we submitted to the fulltextindex module improved lots of
> things but something we could not get to work was the apparently correct use
> of the PG_GETARG* macros, etc.
> Whenever we used these macros, we always got 0 or NULL as our values. So,
> we reverted to the trigger->tgargs array.
Trigger functions don't get their arguments the normal way. The GETARG
macros don't know anything about trigger arguments... so the original
code was correct as it was. I haven't had time to look at your patch,
but maybe I should go do that.
regards, tom lane
Has this been addressed? > "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: > > The latest patch we submitted to the fulltextindex module improved lots of > > things but something we could not get to work was the apparently correct use > > of the PG_GETARG* macros, etc. > > > Whenever we used these macros, we always got 0 or NULL as our values. So, > > we reverted to the trigger->tgargs array. > > Trigger functions don't get their arguments the normal way. The GETARG > macros don't know anything about trigger arguments... so the original > code was correct as it was. I haven't had time to look at your patch, > but maybe I should go do that. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Has this been addressed?
IIRC, I looked at the patch and decided it was okay.
regards, tom lane
>> "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> The latest patch we submitted to the fulltextindex module improved lots of
> things but something we could not get to work was the apparently correct use
> of the PG_GETARG* macros, etc.
>>
> Whenever we used these macros, we always got 0 or NULL as our values. So,
> we reverted to the trigger->tgargs array.
>>
>> Trigger functions don't get their arguments the normal way. The GETARG
>> macros don't know anything about trigger arguments... so the original
>> code was correct as it was. I haven't had time to look at your patch,
>> but maybe I should go do that.
>>
>> regards, tom lane
Well, the FTI code that was committed works perfectly - it compiles fine against 7.0.3 and 7.1.2 and is in use indexing 2 columns in 20000 row tables in two production and one test servers. The updated fti.pl we submitted still uses the PGConnect style functions, rather than the PG::Connect style functions. However, I don't know why there is this different in Pg.pm??? My issue with accessing args was that the docs on writing functions and triggers were a bit confusing. I got the impression that one had to access the trigger args via GETARG macros - but it turns out that is not the case. Still, someone may wish to review the fti code, and check our optimizations. Plus, since it's is 100% backwards compatible with the version in 7.1.2, you might want to back port it to the 7.1.* branch? Cheers, Chris > Has this been addressed? > > > > "Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes: > > > The latest patch we submitted to the fulltextindex module > improved lots of > > > things but something we could not get to work was the > apparently correct use > > > of the PG_GETARG* macros, etc. > > > > > Whenever we used these macros, we always got 0 or NULL as our > values. So, > > > we reverted to the trigger->tgargs array. > > > > Trigger functions don't get their arguments the normal way. The GETARG > > macros don't know anything about trigger arguments... so the original > > code was correct as it was. I haven't had time to look at your patch, > > but maybe I should go do that. > > > > regards, tom lane
"Christopher Kings-Lynne" <chriskl@familyhealth.com.au> writes:
> Plus, since it's is 100% backwards compatible with the version in 7.1.2, you
> might want to back port it to the 7.1.* branch?
Since we're hoping to go beta with 7.2 next week, I doubt there will be
any further releases in the 7.1.* branch.
regards, tom lane