Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Search does not work on postgres citext fields #13726

Closed
bamboowonder opened this issue Jun 4, 2022 · 6 comments
Closed

Search does not work on postgres citext fields #13726

bamboowonder opened this issue Jun 4, 2022 · 6 comments

Comments

@bamboowonder
Copy link

bamboowonder commented Jun 4, 2022

Describe the Bug

I've finally diagnosed why search is not working for us.
varchar fields are fine.
text fields are fine.
citext fields are not searched at all. app or api.

Previously a bug report had fixed it so that Directus treated "citext" as a "text" field instead of "unknown".

But it looks like the search mechanism doesn't include database citext fields. Even though Directus sees them as text.

To Reproduce

create a varchar, a text, a citext field in database with test data.
search on the values from any of those 3 fields using the search box in directus or the api
nothing in citext is searched

Errors Shown

no errors are shown anywhere

What version of Directus are you using?

9.12.1

What version of Node.js are you using?

12.22.9

What database are you using?

postgres 14

What browser are you using?

chrome

How are you deploying Directus?

CLI npm package

@bamboowonder
Copy link
Author

To followup. I complete repaved the Directus instance. And found a sortof related bug, in that I modified a citext field using Directus, and after commiting the changes (to make it NOT NULL), I noticed that all of a sudden the "search" worked for that field. However I noticed that Directus when it altered the table to add not null, it also changed the field type to text from citext. (Which I guess makes sense if allowing Directus to manage the database).

What was interesting, is that even with the column being text, case-INsensitive search functioned! Whereas a few versions ago it didn't function.

So I'm not sure if I should close this bug. But I sure appreciate that Directus now supports case-insensitive search even on postgres without having to specify citext fields!

@azrikahar
Copy link
Contributor

This is an interesting one. Seems like the reason why it wasn't working initially is due to the search logic checks the field type to actually add said field into the WHERE query:

if (['text', 'string'].includes(field.type)) {
this.orWhereRaw(`LOWER(??) LIKE ?`, [`${collection}.${name}`, `%${searchQuery.toLowerCase()}%`]);

however, during this time (even though the App shows text already), the field type of the citext field in this IF statement is actually unknown, so it was never added to the search query.

Tracing it back to the data_type of the column over here:

SELECT c.table_name
, c.column_name
, c.column_default as default_value
, c.data_type

will retrieve USER-DEFINED instead. This should be related to this comment here - knex/knex-schema-inspector#73 (comment), pointing out the quote from postgres:

Data type of the array elements, if it is a built-in type, else USER-DEFINED (in that case, the type is identified in udt_name and associated columns).

but as @bamboowonder described, "updating the field" via disabling then re-enabling Allow NULL value makes it properly returning text and not USER-DEFINED anymore going forward.

Feels like this is not really a Directus bug though, and could something to be explored in the knex-schema-inspector end, additionally since citext is Postgres-specific and there already is case-insensitive functionality built-in now. @rijkvanzanten thoughts on this?


  • Before "updating" the citext field by toggle off and on the nullable property, we'll get USER-DEFINED for data_type:

    chrome_80T0WBwUGP

  • After "updating" the field:

    chrome_f1k5VsonYM

@rijkvanzanten

This comment was marked as resolved.

@rijkvanzanten rijkvanzanten closed this as not planned Won't fix, can't repro, duplicate, stale Jun 6, 2022
@bamboowonder

This comment was marked as resolved.

@rijkvanzanten

This comment was marked as resolved.

@rijkvanzanten rijkvanzanten reopened this Jun 6, 2022
@rijkvanzanten
Copy link
Member

citext is a custom user-defined type injected through an extension, so I think we should pick this up as a feature request to implement support for PG custom types and how to handle the type abstraction for those, rather than trying to hack in support for potentially common ones at a time 🤔 Moving this to a discussion 👍🏻

@directus directus locked and limited conversation to collaborators Jun 6, 2022
@rijkvanzanten rijkvanzanten converted this issue into discussion #13758 Jun 6, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Projects
None yet
Development

No branches or pull requests

3 participants