Attach Action Search Attribute `lower(...)` Fails In PostgreSQL

[atoff]

Package

filament/filament

Package Version

2.17.32

Laravel Version

10.12.0

Livewire Version

No response

PHP Version

8.2.7

Problem description

When I changed DB engine from MySQL to PostgreSQL, I encountered an issue with some of my relation managers, whereby on AttachActions where I defined recordSelectSearchColumns using attributes that are not text fields (e.g. IDs or other number identifiers), the following SQL exception is thrown SQLSTATE[42883]: Undefined function: 7 ERROR: function lower(bigint) does not exist.

This seems to be caused by this line (https://github.com/filamentphp/filament/blob/2.x/packages/tables/src/Actions/AttachAction.php#L198) which wraps the attribute in a lower() function. However, the global search implementation appears to cast the attribute to text before hand, meaning it works with these numeric columns (https://github.com/filamentphp/filament/blob/2.x/packages/admin/src/Resources/Resource.php#L477)

Expected behavior

Expected there to be no SQL error and the search to work as normal

Steps to reproduce

In The Repro Repo (Migrate, setup .env for postgres db, etc)

1. Login at /admin/login with joe@example.org, password testtest
2. Navigate to the Books resource index, select the seeded book
3. Click Attach on the user relation manager
4. Enter a search query

Generally

1. Use PostgreSQL (I was using v15)
2. Define a model that has a numeric-based identifier (such as a typical auto-incrementing model ID)
3. Build a relation manager that uses the AttachAction.
4. Define recordSelectSearchColumns with this attribute
5. Attempt to attach the model in the panel/table

Reproduction repository

https://github.com/atoff/filament-bug-lower-pgsql

Relevant log output

Flare Share: https://flareapp.io/share/LPlEkj27#debug

[danharrin]

Please PR a fix using ::text to all places we use lower() then

[danharrin]

Looking at this now, I would suggest we don't do that due to performance reasons. Try this:

AttachAction::make()->recordSelectSearchColumns(['id::text'])

Just FYI - we only use ::text in global search when we're dealing with translatable JSON. Otherwise, we don't use it anywhere else.

Please let me know if that doesn't work and I'll reopen the issue so we can discuss other options

[atoff]

Looking at this now, I would suggest we don't do that due to performance reasons. Try this:

AttachAction::make()->recordSelectSearchColumns(['id::text'])

Just FYI - we only use ::text in global search when we're dealing with translatable JSON. Otherwise, we don't use it anywhere else.

Please let me know if that doesn't work and I'll reopen the issue so we can discuss other options

This does work and is what I have used in the meantime to circumvent this issue. However, I think from a usability point of view someone would expect this to work without needing that shim. Especially if you are trying to keep your project database agnostic so you can use any of Laravels adapters.

On the translatable json side, that is a good point. I wondered why global search was working in this case then, and from taking a look at the source, the difference seems to be that global search uses a ilike operator but the Attach search uses lower() transformation. It appears that MySQL-type DBs default to a LIKE query being case insensitive, which is why global search finds results fine despite not having a lower() statement (e.g. where user like '%joe%' would still match user=Joe on MySQL).

I would suggest a refactor to change where lower() is used to ilike statements when on pgsql and this should avoid this issue I believe?

[danharrin]

I don't think we can use ::text everywhere since it would degrade performance, and theres not really a reliable way to only do it for numeric columns. So this is probably a good permanent solution. We have deliberately avoided ilike for the same performance reasons we initially introduced forceCaseInsensitive(), which is now enabled by default

[atoff]

Ok sounds good. If you're happy with that can you reopen this issue and I will make a PR to patch V2 and V3 to close it 👍

[danharrin]

Sorry, what are you going to patch?

[atoff]

Sorry I thought you agreed in your previous comment that using ilike for the search when using postgres would be appropriate to avoid this issue. Seeing as this is already done in global search, I would patch to use this for the attach action, and any other searches that are affected. I see your point about performance now, but I think for attach actions this would be fairly negligible no?

I just think the ::text method isn't really sustainable, and doesn't give a good DX (aside from not being documented currently, but granted that could be changed). I'd imagine that the same problem is encountered if the relation manager title field is numeric, and you'd then end up having to either manually override or set that to ::text which might cause other issues.

[danharrin]

As far as I know ilike is not currently used anywhere? ::text is only used when you're in Postgres and want to use a numeric field to search on, in which case its not a bad solution IMO. Otherwise, you're degrading the performance of all text-based search columns by using ilike instead of lower().

[atoff]

I agree that appending ::text would work in most situations, I guess for me it's more so that it constrains the app to running on postgres (I don't believe you can use ::text on SQL).

ilike is currently used for global search when on postgres: https://github.com/filamentphp/filament/blob/2.x/packages/admin/src/Resources/Resource.php#L464-L477

[danharrin]

That's v2, we don't use it for v3 at all