Query limit reached on Cases List view due to search form rendering

We recently ran into an issue that I’d like to share since we spent countless hours trying to fix it in case anyone has this issue. It was quite an adventure.

SuiteCRM Version Used
7.11.8

Where it all started
We use Cases a lot, we have over 90 000 cases. One day, the list view suddenly stopped working. It gave us an error " Query limit of 20000 reached". We then noticed it was breaking the Leads Detail view and the Documents view which both contain a Cases dashlet.

What Happened?
When looking up the error online, all the results I found simply suggested raising the limit but this seemed odd to me. 20 000 is a lot of queries for one list view so I started to log the exact queries. What I noticed was that it seemed to be doing queries on other modules that weren’t directly related to the case.

Our first assumption was that it was data related so we did our database checks, looking for a corrupted table but nothing came up. This is when I realized the issue might be in the code itself and might’ve been a pre-existing issue that just now started to surface.

Many many hours later of logging and following the exact path of how a list view is rendered I pinpointed the problematic area. In the SearchForm2.php file it loops through the module’s definition. Within that, there is a case_attachments_display_basic field which has a field type of HTML. This field does a call to a function named “case_attachments_display” Which retrieves the list of notes and puts then into HTML links.
But because it was running this function on an empty bean, the display_case_attachments functions which would normally do a get_linked_beans call to get the notes on a specific case bean was doing it on one without an id. This then caused the get_linked_beans to return over 11k note beans. Most of the other HTML fields are simple things like buttons and aren’t looking at beans which is why we hadn’t run into this issue with other modules.

How it got fixed
In the end, I went into the _build_field_defs function which was calling the display_case_attachments through “call_user_func” and modified the following:

    if (!empty($this->fieldDefs[$fvName]['function']['returns']) && $this->fieldDefs[$fvName]['function']['returns'] == 'html') {
                        $value = call_user_func($function_name, $this->seed, $name, $value, $this->view);
                        $this->fieldDefs[$fvName]['value'] = $value;
                    }

to

     if (!empty($this->fieldDefs[$fvName]['function']['returns']) && $this->fieldDefs[$fvName]['function']['returns'] == 'html') {
                    if($name == 'case_attachments_display')
                    {
                        $value = "";
                    }
                    else
                    {  
                        $value = call_user_func($function_name, $this->seed, $name, $value, $this->view);
                    }
                    $this->fieldDefs[$fvName]['value'] = $value;
                }

This stopped the function from being called and doing the thousands of queries on notes. I can’t guarantee this was the best approach but it worked well for us.

Has anyone else ran into this issue? I was going to submit a bug request but I’m not sure if this qualifies as a bug.

1 Like

Hi @sugarsuitedev,

Impressive find, well done! That is a load of cases :laughing:

@Dillon-Brown @pgr Any suggestions on changes that should be made and as to whether this should make it into core also?

Hey, @sugarsuitedev, nice debugging work :tada:

Here are a few links to see these things in context in case anybody wants to have a look:

SearchForm2.php on GitHub
case_attachents_display References

I’m curious, can you elaborate on why this was happening?

And my suggestion would be - is it possible to change this condition

if($name == 'case_attachments_display')

to something else, for example, testing for an empty bean? The reference to case_attachments_display seems very specific, but this trap could happen in the future with other modules.

I’m just asking if it’s possible, so that the bug fix can be as generic as possible.

Thanks for your work here and for this report.

Hey,
So SearchForm2.php’s _build_field_defs runs on an empty bean because it’s just trying to add more details to the field definition to create the view. Which for 99% of the fields is fine because they either just return default values or some HTML (ex. some edit buttons). This is also why I added the “if($name == ‘case_attachments_display’)”. Otherwise, the other HTML fields that actually need to run their functions would not be able to since from there, there never will be a bean.

However, unlike the other HTML type fields, when it comes to case_attachments_display field it’s function is actually meant to generate HTML links of the attachment meant for an actual bean. (This function is found in Case_Updates.php). I think that the function is most likely meant for a Detail view or something similar. In that function, it does a call to get_linked_beans which, because it’s an empty bean, just queries all the notes. (And because its getting beans, it starts to get related beans to each note).

I think a lot of people don’t notice it because they don’t have such a large amount of cases/notes or their settings are very high for queries.

Also as heads up, the edit view was doing it as well.
I changed DetailView2.php, line 647
from:


     if (
                         !empty($this->fieldDefs[$name]['function']['returns']) &&
                         $this->fieldDefs[$name]['function']['returns'] === 'html'
                     ) {
                         if (!empty($this->fieldDefs[$name]['function']['include'])) {
                             require_once($this->fieldDefs[$name]['function']['include']);
                         }
                         $value = call_user_func($function, $this->focus, $name, $value, $this->view);
                         $valueFormatted = true;
                     }

to

         if (
                         !empty($this->fieldDefs[$name]['function']['returns']) &&
                         $this->fieldDefs[$name]['function']['returns'] === 'html'
                     ) {
 
                         if($name == 'case_attachments_display')
                         {
                             $value = "";
                         }
                         else
                         {
                             if (!empty($this->fieldDefs[$name]['function']['include'])) {
                                 require_once($this->fieldDefs[$name]['function']['include']);
                             }
                             $value = call_user_func($function, $this->focus, $name, $value, $this->view);
                         }
                         $valueFormatted = true;
                     }

Thanks for the additional details!

And what about my suggestion? Do you think that, instead of testing for the function name to restrict the occasions when you skip the function, we could test for

empty($this->focus)

or something like that?

The issue with that is there are also other functions which don’t require a bean which would be stopped by that condition.

Another option would to actually edit the function call display_case_attachments in Case_Updates.php to check if the case bean has an id and if not, return an empty string. This again would only fix the one function, however, it would fix it for any other view that attempts to call it. It’s a pretty unique function and I don’t any other modules that would be similar so we might be okay with just fixing the one.

I like this last solution that you propose better.

I admit I am not fully in understanding of everything that is going on here, or that could be going on here when this code is called from other places. But I like the idea of changing things only inside that specific function.

Having a condition testing for a single function name inside our very generic SugarBean code doesn’t feel right.

Thanks for this, are you going to create a PR?

(I recently made simple instructions for someone else here)

I’ve done the fix on the function and everything seems to work. I’ll do more testing on the weekend and also make a PR, thanks for the instructions.

2 Likes