Come together with the global Drupal community in Rotterdam, 28 Sept – 1 Oct 2026. Sessions, contribution, connection, and Early Bird savings until 8 June.Problem/Motivation
Past Drupal 8 entity IDs can be strings (which is why Paragraph's paragraphs_item_field_data's parent_id column is a varchar(255)).
When importing to an entity using a string ID, a warning is thrown about truncated data for the fields_clean_list table.
Steps to reproduce
Import to an entity using a string ID.
Proposed resolution
Update the schema to use a varchar for entity_id.
Issue fork feeds-3414632
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #3
karlsheaComment #4
kevinvb commentedI can confirm that this fixes the issue "Invalid datetime format: 1292 Truncated incorrect DOUBLE value: 'xxx': DELETE FROM feeds_clean_list..."
I had to be able to import & update from a CSV into custom tokens (https://www.drupal.org/project/token_custom)
This module provides a content entity which has a machine name (string) as id.
Just importing didn't trigger the error but once you want to update an existing item it fails to import and gives that nasty error.
With the patch the module is able to perform the import & update.
Comment #5
ptmkenny commentedThis is tricky because an entity id should be an int but it might be a string. Adding one of the main core issues about this topic for reference.
Comment #6
karlsheaI'm not sure that core issue is a good one for reference because it's talking about a case where the id should be an int for those entity types but isn't, e.g. the database column is an
intbut due to some circumstance a string is returned ("1"instead of1).However
EntityInterfacedoes not limit the id to an int, the typehint isstring|int|nullso the statement "an entity id should be an int" is not true for all Drupal Entity types.This issue is trying to fix the case where the entity id is intentionally a string (and the database column backing it is a
varchar). Think about a table of US states where the id is the two-letter acronym—we're not trying to coerce"1"to1, but instead trying to store the value"NY"as an int which is impossible.I don't think
EntityInterfacesupporting this is widely known so it's been catching out some other module assumptions (like Dynamic Entity Reference which has some weird release versions to fix the same issue).Check out the
EntityTestStringIdcontent entity type in core which tests this, and the issue that created it: #2205215: {comment} and {comment_entity_statistics} only support integer entity ids, or the issue also using that same type to test core's Entity Reference field type making the same assumption: #2107249: Don't assume that content entities have numeric IDs in EntityReferenceItem.Comment #7
megachrizIt would be good to have an automated test that tests importing and logging entities with a non-numerical ID. In core, in the test module "entity_test", there is a content entity type called "entity_test_string_id" (\Drupal\entity_test\Entity\EntityTestStringId). That entity type would probably be suitable for such a test. Do you want to write tests for this issue, @KarlShea?
Comment #8
karlshea@MegaChriz I added a test but I'm not sure that it's in the right spot, or that I'm putting a test for feeds_log outside of the submodule. I can make any changes here you'd like, just let me know the direction you're thinking.
When I remove the schema change for
feeds_import_log_entry, the assertion fails checking for the log entry count, and when I remove the schema change forfeeds_clean_listthe test fails with an "Incorrect integer value for column" error.Comment #9
ankitv18 commentedFor next minor PHPunit is failing: https://git.drupalcode.org/issue/feeds-3414632/-/jobs/1639583#L460
We can log a separate issue to fix this and the changes looks fine hence marking this RTBC
Comment #10
megachriz@KarlShea
I think it would be good to have one test focussed on Feeds Log, in feeds/modules/log/tests/Kernel. I think the other one could be added to the existing \Drupal\Tests\feeds\Kernel\UpdateNonExistentTest test class.
Comment #11
megachrizI think I'll give this a go myself, so I can hopefully merge this for the upcoming release.
Comment #13
karlsheaChanges makes sense, thanks!
Comment #15
megachrizThanks for checking, @KarlShea! I merged the code.
Comment #17
megachrizI got one issue when updating a site:
feeds_import_log_entry.entity_id is allowed to be NULL.
Fixed that on commit.
Comment #18
megachriz@KarlShea
The database update
feeds_update_8007()caused an error on MySQL 8.0:It looks like to have something to do with the MySQL setting "sql_generate_invisible_primary_key". It is said that if that setting is turned off, the error should disappear.
Do you know if the database update could be rewritten in a way that it doesn't cause an error when the MySQL setting "sql_generate_invisible_primary_key" is turned on?
See #3454534: SQL error after upgrading to 8.x-3.0-beta5
Comment #19
karlsheaHmm my understanding is the only way to change a primary key is to drop it first and then recreate it. I asked in #contribute on Slack, let's see if anything comes from that.