Обсуждение: tsvector work with citext

Hey Hackers,

Is there a way to get tsvector_update_trigger() to work with citext columns? The attached case throws an error:

    ERROR:  column "title" is not of a character type

Is the fact that citext is a (non-preferred) member of the string category not sufficient for this to work? If not, are
thereany workarounds? 

Thanks,

David

"David E. Wheeler" <david@kineticode.com> writes:
> Is there a way to get tsvector_update_trigger() to work with citext
> columns?

Hmm ... tsvector_op.c has

/* Check if datatype is TEXT or binary-equivalent to it */
static bool
is_text_type(Oid typid)
{/* varchar(n) and char(n) are binary-compatible with text */if (typid == TEXTOID || typid == VARCHAROID || typid ==
BPCHAROID)   return true;/* Allow domains over these types, too */typid = getBaseType(typid);if (typid == TEXTOID ||
typid== VARCHAROID || typid == BPCHAROID)    return true;return false;
 
}

and a look at the caller says that "binary-equivalent to TEXT" is indeed
the requirement, because we want to apply DatumGetTextP() to the argument.

However, it does seem like this function is not implementing its
specification.  Why isn't it just "IsBinaryCoercible(typid, TEXTOID)"?
        regards, tom lane

> However, it does seem like this function is not implementing its
> specification.  Why isn't it just "IsBinaryCoercible(typid, TEXTOID)"?
Oversight, I suppose. is_text_type() was introduced by

commit 635aaab278afc1af972a4b6a55ff632ab763505d
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date:   Tue Apr 8 18:20:29 2008 +0000
    Fix tsvector_update_trigger() to be domain-friendly: it needs to allow all    the columns it works with to be
domainsover the expected type, not just    exactly the expected type.  In passing, fix ts_stat() the same way.    Per
reportfrom Markus Wollny.
 

Will fix. Suppose, is_expected_type() could be replaced to IsBinaryCoercible too.



-- 
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
  WWW: http://www.sigaev.ru/
 

> Oversight, I suppose. is_text_type() was introduced by
>
> commit 635aaab278afc1af972a4b6a55ff632ab763505d
> Author: Tom Lane <tgl@sss.pgh.pa.us>
> Date:   Tue Apr 8 18:20:29 2008 +0000
>
>      Fix tsvector_update_trigger() to be domain-friendly: it needs to allow all
>      the columns it works with to be domains over the expected type, not just
>      exactly the expected type.  In passing, fix ts_stat() the same way.
>      Per report from Markus Wollny.

I'm wrong, in this commit it was just renamed. It was originally coded by me. 
But it's still oversight.

-- 
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
  WWW: http://www.sigaev.ru/
 

On Sep 17, 2015, at 6:17 AM, Teodor Sigaev <teodor@sigaev.ru> wrote:

> I'm wrong, in this commit it was just renamed. It was originally coded by me. But it's still oversight.

Fixable?

Thanks,

David

> Fixable?

Fixed (9acb9007de30b3daaa9efc16763c3bc6e3e0a92d), but didn't backpatch because 
it isn't a critical bug.

Thank you for the report!

-- 
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
  WWW: http://www.sigaev.ru/
 

On Sep 18, 2015, at 7:29 AM, Teodor Sigaev <teodor@sigaev.ru> wrote:

>> Fixable?
>
> Fixed (9acb9007de30b3daaa9efc16763c3bc6e3e0a92d), but didn't backpatch because it isn't a critical bug.

Great, thank you!

For those on older versions, what’s the simplest workaround?

Best,

David

"David E. Wheeler" <david@kineticode.com> writes:
> On Sep 18, 2015, at 7:29 AM, Teodor Sigaev <teodor@sigaev.ru> wrote:
>> Fixed (9acb9007de30b3daaa9efc16763c3bc6e3e0a92d), but didn't backpatch because it isn't a critical bug.

> Great, thank you!
> For those on older versions, what’s the simplest workaround?

FWIW, I thought this would be a reasonable thing to back-patch.
It's not as though contrib/citext hasn't been around for awhile.
        regards, tom lane

>>> Fixed (9acb9007de30b3daaa9efc16763c3bc6e3e0a92d), but didn't backpatch because it isn't a critical bug.
>> For those on older versions, whatтАЩs the simplest workaround?
> FWIW, I thought this would be a reasonable thing to back-patch.
> It's not as though contrib/citext hasn't been around for awhile.

I'd like this idea, but does it look like a new feature in previous releses?

-- 
Teodor Sigaev                                   E-mail: teodor@sigaev.ru
  WWW: http://www.sigaev.ru/