Skip to content
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

Replace usage of in_array() in MigrateExecutable::handleMissingSourceRows #5766

Open
wants to merge 2 commits into
base: 12.x
Choose a base branch
from

Conversation

mdolnik
Copy link

@mdolnik mdolnik commented Sep 25, 2023

PR to fix #5765

…urceRows().

Adds a new method getSourceIdKey() to serialize the source ID data into a unique key for retrieval later.

Refer to drush-ops#5765
*/
protected function getSourceIdKey(array $source_id): string
{
return implode('-', $source_id);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder how safe is this

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the full extent of what is possible to appear in the $source_id array values, but from my investigation it is mostly entity IDs, revision IDs, and simple strings. Of which, most cases involve one or two values.

I am open to alternate ways of serializing the array values into a unique key.

Copy link

@debart-nathan debart-nathan Aug 31, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi.
I run into this issue while doing a diagnostic on what was slowing the import even after it was marked as finished.

And want to put my grain of salts.

The case i can see where the proposed serialisation can cause problem is very specific.
[A_B, C] has the same key than [A,B_C].

you could use hash function but you would still run into the Pigeon hole principle.

the only way to avoid that would be to let the user decide by a cli argument what separator to use, including the option to turn the feature off and use the old system.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace usage of in_array() in MigrateExecutable::handleMissingSourceRows
4 participants