-
-
Notifications
You must be signed in to change notification settings - Fork 825
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
Export merge to household - fix DB error relating to fields too long for table. #13338
Conversation
(Standard links)
|
Test failure seems to relate i think |
90c3fd0
to
63fee54
Compare
FYI, test failures seem irrelevant and would not reproduce on my local dev.
|
test this please |
related fail was fixed - we just seem to have a variety on unrelated fails now. Would be good to merge this before rc is cut on 2 Jan |
I have tested the following contact exports:
I don't think merge households is actually working. On a test database (210 contacts) I get 210 contacts exported. If I do the same on git master (but change temp table definition to MyISAM so it executes) I get 140 contacts on export. |
…for table. The key fix here is that the fields for merging to household are no longer added to the temporary table. They are fully loaded in php anyway so there is no performance gain using a temp table. Instead we iterate them in php as we process each row
63fee54
to
136f69a
Compare
@mattwire it was replacing with household details - but for every row - now it's skipping if the household is already exported |
@eileenmcnaughton Confirmed the export contacts now do the same for all 3 and merge households is exporting the correct result. @seamuslee001 Can this be merged? Have we cut the RC yet, would be good to have this in the RC so it gets a full month of testing - bonus being merge household export actually works which it hasn't for quite a long time... |
Merging based on @mattwire review |
Overview
Fixes a fatal error when trying to combine merge households and 'primary fields' on some mysql configs
Before
depending on mysql this could result in a DB error
Database Error Code: Row size too large (> 8126). Changing some columns to TEXT or BLOB may help. In current row format, BLOB prefix of 0 bytes is stored inline., 1118
After
Less fields written to the temp table so DB error not encountered
Technical Details
The key fix here is that the fields for merging to household are no longer added to the temporary table.
They are fully loaded in php anyway so there is no performance gain using a temp table.
Instead we iterate them in php as we process each row.
Comments
@mattwire @twomice @monishdeb - this is the PR that actually fixes the DB error after all the prep work.
It will need a bit of testing! I have just done very basic tests so far.