Skip to content

Conversation

@DavidStrada
Copy link

The following address 2 issues I encounter when trying to upgrade craft 4 to the latest version.

  1. Instead of getting all records at once causing php out of memory issue, it queries records using batches (100 records) better performance and no issues when querying 5000 + records.
  2. Checks for any existing duplicates in the table, Clean up any duplicates before creating the unique index, avoiding the issue of Integrity constraint violation.

The following address 2 issues I encounter when trying to upgrade craft 4 to the latest version.

1. Instead of getting all records at once causing php out of memory issue, it queries records using batches (100 records) better performance and no issues when querying 5000 + records.
2. Checks for any existing duplicates in the table,  Clean up any duplicates before creating the unique index, avoiding the issue of Integrity constraint violation.
@DavidStrada
Copy link
Author

Hi @lindseydiloreto is there a willingness on updating the codebase to allow this performance update ?
thanks

@lindseydiloreto
Copy link
Collaborator

Hi @DavidStrada,

I could have sworn we had a conversation about this (maybe on Discord), but I can't find any record of it. Perhaps it was just a dream I had.

I'm not opposed to your PR, it's just a much lower priority item compared to a lot of things we currently have in front of us. This PR makes significant changes to an old, deeply complex migration. Like all migrations, this code will only run once per each project, which means the PR is introducing a lot of complexity for just a little bit of value.

Hopefully your fork enabled you to get past this troublesome migration.

When I eventually get a chance, I would love to take the necessary time to properly review your PR and accept it into the plugin. Thanks for your contribution. 🙏

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.

2 participants