Unexpected side-effects when using `group(…)` filter

[AugustMiller]

Description

While doing some performance work, I noticed there were some out-of-place queries related to a SimpleMap field (ultimately reproduced with SmartMap, too) that uses data from another table.

This led to a realization that within the group(…) filter, each element was having its fields normalized (or, passed through normalizeValue / normalizeFieldValue, despite not being used.

Ultimately, I don't really consider this a bug, so much as a strange side-effect of the ->toArray() method that Elements inherit… is there a backwards-compatible alternative to this behavior (casting the attributes into the string templates)?

Steps to reproduce

1. Create a Section
2. Create a SimpleMap or SmartMap field
3. Assign fields to the Entry Type
4. Create an Entry
5. Query for that Entry: {% set entries = craft.entries({ section: 'mySection' }).all() | group('id') %}
6. Observe query for SimpleMap/SmartMap data.

This should be reproducible for any kind of field that does an additional query for external data in its normalizeValue method.

Additional info

• Craft version: Pro, 3.1.3
• PHP version: 7.3.1
• Database driver & version: PostgreSQL 10.6
• Plugins & versions:

Colors: 1.0.8
Craft Commerce: 2.0.1
Entry Instructions: 1.0.4
Google Cloud Storage: 1.1
Postmark: 1.0.0
Redactor: 2.3.0
SimpleMap: 3.3.4
Stripe for Craft Commerce: 1.0.10
Typogrify: 1.1.16

[brandonkelly]

Ideally all custom fields should not be immediately running any queries from normalizeValue(). None of the built-in relational fields or Matrix do, for example – their normalizeValue() methods return element query objects, but the queries won’t actually be executed unless something in the template tells it to.

The only way we could solve this is by moving all custom fields out of the default toArray() results, so they’d only be included when explicitly requested by $extraFields. Maybe that would be better, but it would be a breaking change so not something we could do until 4.0.

[AugustMiller]

This sounds like a totally fine recommendation/principle—we'll raise with the map plugin authors, and brainstorm some PRs.

Thank you! 💞