"Okay, okay", the Wise Old Elf reassured, "we can get to the bottom of this. Just run the whole thing with the DBIC_TRACE environment variable set to true. and DBIx::Class will spit out the SQL it's doing to STDERR."

No sooner had Garland run the command again than he immediately saw the problem; Perl was trying to bring back every single child from the database, and, well, that was, as you might expect, taking some time.

"I see what's going on now. You've been bitten by using the search method in list context", the Wise Old Elf explained.

Context Matters

One way that people (and elves) often get into trouble with Perl is forgetting that some functions and methods act differently when called in list or scalar context.

For example, this code creates two results sets we can use later to request children one by one:

This works because search is being called in scalar context both times, and when it's called in scalar context we get back a resultset as the names of the variables indicate. At this point these two abstract resultsets represent sets of conditions that will be run against the database as soon as we call first, next, etc on them or can be used to create further refined result sets by adding additional conditions with further calls to search.

However, this seemingly similar code does something dramatically different

While it looks like the search method here should return one value each time it's called to pair with naughty and nice, that's not what happens. search in list context actually returns all the results as a big list making $result_sets huge and filled with nonsensical data rather than just the two key-value pairs we were expecting. In other situations this can result in security risks where keys and parameters become mismatched.

One way to fix this is to force scalar context with the scalar keyword:

search_rs is exactly the same as search, except it always returns a result set object even when called in list context.

Writing a Perl Critic rule

After all this explanation, Garland was frustrated. "Okay, I give up. We should just never call the search method. We should always use search_rs"

The Wise Old Elf agreed, and he had a plan. Rather than just updating the coding standards document, he made sure that no code in the repository contained calls to the search method and made it impossible for new code to sneak in calls to search.

How did he do that? Well, the test suite for the North Pole codebase uses Test::Perl::Critic to ensure that the code in the repo meets the Perl::Critic guidelines defined in the .perlcriticrc. So all the Wise Old Elf had left to do was write a quick Perl::Critic rule that freaks out at any method call to search that isn't wrapped in a no critic type declaration.

return$self->violation('Use of the search() method is prohibited',<<'END',DBIx::Class's search() method returns a result set object in scalar context, butreturns a list of results in list context. This is unsafe when such a call isused in a constructor as it can mix up the parameter keys and values. Forexample, if search returns no results this: