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#1747 don't drop temp tables on class destruct. #17438

Merged
merged 1 commit into from
Jun 2, 2020

Conversation

eileenmcnaughton
Copy link
Contributor

@eileenmcnaughton eileenmcnaughton commented May 31, 2020

Overview

Fixes a regression whereby saved searches off the Date Added search can't load as the temp table is cleaned up too aggressively - description etc copied from gitlab as it was well explained

Since upgrading to 5.24 creating smart group using 'Date Added to CiviCRM' fails with DB Error: no such table. Also when a smart group is based on 'Date Added to CiviCRM' and try to refresh the count it fails with same error message

Reproduction steps

  1. Navigate to CiviCRM >> Search >> Custom Searches >> Date Added to CiviCRM
  2. Do a search on date and include in group.
  3. Select all contact and try adding to smart group.
  4. Got an error "Fatal error: DB error".

smartGroupError

Current behaviour

Get DB error 'Database Error Code: Table 'dmastercivi_g5lis.civicrm_tmp_e_ig_6f1e2d3b300d50a856f5cb9140996451' doesn't exist, 1146' when trying to create smart group or refresh count

Expected behaviour

smart should be created with correct contacts in it and also when refreshing the group count, the group should show correct count

Technical Details

Recent code clean up means the search class is now dropped more quickly - meaning that dropping
these tables in the destructors is too aggressive. Since they are memory tables & temp tables they get
cleaned up anyway I think. Regardless the odd slow cleanup is better than the current situation

We should move this search to an extension that ships with core, not necessarily
enabled on initial install but that is up for discussion

Comments

Recent code clean up means the search class is now dropped more quickly - meaning that dropping
these tables in the destructors is too aggressive. Since they are memory tables & temp tables they get
cleaned up anyway I think. We should move this search to an extension that ships with core, not necessarily
enabled on initial install but that is up for discussion
@civibot
Copy link

civibot bot commented May 31, 2020

(Standard links)

@civibot civibot bot added the 5.26 label May 31, 2020
@seamuslee001
Copy link
Contributor

Jenkins re test this please

@seamuslee001
Copy link
Contributor

Tested and this fixes the reported issue

@seamuslee001 seamuslee001 merged commit ea04a36 into civicrm:5.26 Jun 2, 2020
@seamuslee001 seamuslee001 deleted the cust branch June 2, 2020 00:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants