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

dev/core#183 CRM/Dedupe/BAO/QueryBuilder/IndividualUnsupervised.php report to using CRM_Utils… #15826

Merged
merged 1 commit into from
Nov 17, 2019

Conversation

seamuslee001
Copy link
Contributor

…_SQL_TempTable

Overview

This converts temporary table syntax to using CRM_Utils_SQL_TempTable interface in CRM/Dedupe files

Before

Legacy format used

After

CRM_Utils_SQL_TempTable used

ping @eileenmcnaughton @totten @monishdeb

@civibot civibot bot added the master label Nov 11, 2019
@civibot
Copy link

civibot bot commented Nov 11, 2019

(Standard links)

@eileenmcnaughton eileenmcnaughton changed the title dev/core#183 Convert Logging report summary report to using CRM_Utils… dev/core#183 CRM/Dedupe/BAO/QueryBuilder/IndividualUnsupervised.php report to using CRM_Utils… Nov 11, 2019
@eileenmcnaughton
Copy link
Contributor

Using $this when not in object context :-(

Screen Shot 2019-11-12 at 12 49 56 PM

@eileenmcnaughton
Copy link
Contributor

@seamuslee001 the reason the above doesn't cause a test fail is it is never hit I believe - testBatchMergeEmailOnHold should otherwise fail

@eileenmcnaughton
Copy link
Contributor

OK - the function comments re-inforce it not being called

"An alternative version which might perform a lot better

  • than the above. Will need to do some testing"

And we can see the only place the class is called

Screen Shot 2019-11-12 at 12 56 45 PM

…roup to CRM_Utils_SQL_TemporaryTable and remove unused function internalOptimized
@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton ok pushed up to remove that function now

@eileenmcnaughton
Copy link
Contributor

OK - trying to test to see if the other is hit #15827 -

@eileenmcnaughton
Copy link
Contributor

Well that's bad news - the tests don't hit it. I'm good with the function removal but need to dig more on the other

seamuslee001 added a commit to seamuslee001/civicrm-core that referenced this pull request Nov 14, 2019
seamuslee001 added a commit to seamuslee001/civicrm-core that referenced this pull request Nov 14, 2019
seamuslee001 added a commit to seamuslee001/civicrm-core that referenced this pull request Nov 17, 2019
seamuslee001 added a commit that referenced this pull request Nov 17, 2019
[NFC] Add in unit test of code being altered by #15826
@seamuslee001
Copy link
Contributor Author

Jenkins re test this please

@seamuslee001
Copy link
Contributor Author

@eileenmcnaughton now that we have merged #15849 are you good with this now?

@eileenmcnaughton
Copy link
Contributor

Ok - merge on pass as the code now gets a test pass-through

@seamuslee001
Copy link
Contributor Author

Test fail unrelated

@seamuslee001 seamuslee001 merged commit a2a44af into civicrm:master Nov 17, 2019
@seamuslee001 seamuslee001 deleted the dev_core_183_dedupe branch November 17, 2019 23:28
@magnolia61
Copy link
Contributor

magnolia61 commented Dec 28, 2019

I hit a fatal error with this PR applied.
Screenshot from 2019-12-28 10-10-57

[code] => -5
[message] => DB Error: already exists
[mode] => 16
[debug_info] => CREATE TEMPORARY TABLE `civicrm_tmp_e_dedupe_7f4acdb4ca55e07e7c552114c5a75b12` ENGINE=InnoDB DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci AS SELECT * FROM civicrm_tmp_e_dedupe_ad86ee0bed80712470ca464e8d7108a4 WHERE weight >= 20 [nativecode=1050 ** Table 'civicrm_tmp_e_dedupe_7f4acdb4ca55e07e7c552114c5a75b12' already exists]
[type] => DB_Error
[user_info] => CREATE TEMPORARY TABLE `civicrm_tmp_e_dedupe_7f4acdb4ca55e07e7c552114c5a75b12` ENGINE=InnoDB DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci AS SELECT * FROM civicrm_tmp_e_dedupe_ad86ee0bed80712470ca464e8d7108a4 WHERE weight >= 20 [nativecode=1050 ** Table 'civicrm_tmp_e_dedupe_7f4acdb4ca55e07e7c552114c5a75b12' already exists]
[to_string] => [db_error: message="DB Error: already exists" code=-5 mode=callback callback=CRM_Core_Error::handle prefix="" info="CREATE TEMPORARY TABLE `civicrm_tmp_e_dedupe_7f4acdb4ca55e07e7c552114c5a75b12` ENGINE=InnoDB DEFAULT CHARACTER SET utf8 COLLATE utf8_unicode_ci AS SELECT * FROM civicrm_tmp_e_dedupe_ad86ee0bed80712470ca464e8d7108a4 WHERE weight >= 20 [nativecode=1050 ** Table 'civicrm_tmp_e_dedupe_7f4acdb4ca55e07e7c552114c5a75b12' already exists]"]

)

@seamuslee001
Copy link
Contributor Author

are there any other associated errors?

@magnolia61
Copy link
Contributor

magnolia61 commented Dec 28, 2019

I was able to retrieve a fatal error on the sandbox as well.

Screenshot from 2019-12-28 11-15-27
Screenshot from 2019-12-28 11-15-47

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

Successfully merging this pull request may close these issues.

3 participants