Views JOIN condition fails in PostgreSQL when comparing entity identifiers of different datatypes

Faced that issue, this patch seems to solve it entirely when using PostgreSQL

But it breaks site when patch is used with MySQL, meaning codebase is coupled with infra, which is not cool and should be avoided

I did see initial success using the patch from #2 but later I saw very steep costs to performance due to this. This is applied to all views which can be dangerous. And it caused one of my views which originally might take up to 7 seconds to load to result in over 15 minutes to load.

Without the patch: Execution Time: 2214.311 ms, cost=52932.30
With the patch and a functional index: Execution Time: 7849.606 ms but the cost=220466848333415508344832.00 (way to big)

Updated this patch for Drupal 9.4.1

Hi All, Updated the patch #14 with few minor changes.

Tried to the syntax error by corrected the cast operator as getting syntax error in the previous patch.

Added a CAST as CHAR to solve the issue.

Removed the trailing spaces.

Complementing #12
In addition to performance issues, I got duplicated rows in different views, for example, I got duplicated results after filtering users by mail in /admin/people.

I would recommend validating the database type before doing the CAST, since this only happens with "pgsql", then validate the type of the field (if entity type exists) and validate if the field doesn't have "id" as sub-string (assuming all ids are INT)

I solve my specific issue with this https://www.drupal.org/project/flag/issues/2929733#comment-12836956

Based on my previous comment I've created this patch.

I have placed strrpos params wrong, patch was updated.

The last patch didn't work for me, using Drupal 9.5.11.
I updated to make it working.

The patch in #27 fixes this for me in Drupal 10.2. It also applies to Drupal 11.

This improved patch casts to text instead of bigint. This fixes additional failures I was having.

I have created a merge request with the patches in 27 and 29.

I have updated the tests so that they pass with the changes.

@Liam Morland, while the tests are passing on the merge request, it's not apparent which database system is being tested. It would be good to just make sure that MariaDB/Mysql/Sqlite are also passing and also all versions of PostgreSQL. With the MR test, it wasn't obvious which setup was being tested (maybe I'm missing something?).

I have run the tests on all the platforms available in the pipelines. They have all passed. If you click on the pipeline number in the MR, you can see the full results.

Can we use \Drupal\views\Plugin\views\join\CastedIntFieldJoin for this and avoid adding pgsql specific code to base plugins?

It looks like it will be deprecated: #3376464: [PP-1] Deprecate and remove Drupal\views\Plugin\views\join\CastedIntFieldJoin.

By its name, it sounds like it would cast to int. The merge request has it casting to text because entity identifiers are not always integers.

Current changes without the test updates.

Hiding patches for clarity.

Leaving in NR as seems desire is/was to move pgsql specific code out of base

Where would you suggest this code be put?

@smustgrave , the suggested approach to move pgsql specific code out of base is marked to be deprecated.
see comment #35
#3079534-35: Views JOIN condition fails in PostgreSQL when comparing entity identifiers of different datatypes

Well if there is a use for it, we can always undeprecate it (or in this case, never deprecate it in the first place), or if it needs a string version of this we should add a string version and put it in the pgsql module where this seems to belong (is PostgreSQL the exception here or is MySQL? How does SQLite handle this?)

Also, I think the general consensus is that int joins are more performant than string joins, so what performance impact does casting everything to a string have? We should check.

How is this handled outside Views? It's not like Views is the only place you can make joins. Is there maybe some method we could/should add to the database drivers that handles this?

I would like to see test coverage added that actually shows we need this, so something that would fail without this fix. All the modified test coverage changes only show that the ':text' is added and this doesn't break existing queries.

At the moment, Postgres is the exception, though I think that it is doing it more correct under the SQL standard. I don't know what SQLite does.

I ran into this problem as described in #2864440: PostgreSQL problems on entity_id of type varchar with flagging and flag_count tables. Other related issues describe how this has surfaced.

The problem is that id columns can be either string or integer. If there is a way to of knowing which it is then it could use integer casting when that is possible.

I've tried different patches and the Merge Requests (as patches) on a D11.3.3, PG17 installation. Unfortunately, none of them worked. Do I miss something?

There is a patch for this for flag module in #2929733: SQL query error on PostgreSQL 9.5 when try to list Bookmarks.

This change is reasonable IMHO and the tests have been adjusted. Client of mine is migrating from MySQL to pgsql for several reasons and we'll appreciate this improvement.

Would be good if this fix made it into 11.4.

This solution around str_contains($this->field, 'id') does not feel correct. Firstly it can produce both false positives and false negatives. For example the uuid field or any field containing the string id. Also, casting an indexed integer column to ::text in a JOIN … ON condition makes PostgreSQL unable to use the index on that column. For large tables (e.g., node_field_data, users_field_data) this could turn an index seek into a full table scan.

We need to compare the columns and only add ::text cast to the necessary column. However we don't have an API to introspect the table schema so we'll need to add one. Ideally we should file fixes in modules that cause this problem - ie. in the flagging case we should add an integer column to the table flagging_count table in in the case where we join to an entity with an integer ID we should join to that column.