Problem/Motivation

\Drupal\l10n_community\L10nTranslator::getStrings() breaks in common database setups (because ONLY_FULL_GROUP_BY is enabled by default nowadays).

Steps to reproduce

Attempt to use the translation form on a MySQL server with ONLY_FULL_GROUP_BY.

Proposed resolution

?

Remaining tasks

User interface changes

-

API changes

-

Data model changes

-

LLM disclosure

The issue was reproduced with LLM while running the test suite and fixed with that in human review.

Issue fork l10n_server-3543334

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

tstoeckler created an issue. See original summary.

tstoeckler opened merge request !75

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented
Status: Active » Needs review

See #3541994: Fix CI for the CI failures.

I went with the minimal fix for now by just replacing all the non-aggregated fields with a MAX(...) expression. That works because there should only be a single result anyway, but it's not functionally dependent so MySQL doesn't know that.

It's not very pretty, but it works, but I'm definitely open to suggestions.

teebeecoder’s picture

teebeecoder commented

Hi @tstoeckler,

I won’t merge this for now, since this function is only used once in L10nCommunityLanguagesController.php. I’m currently optimizing its performance, so I’ll revisit it early next week to decide whether to keep the method as-is or simplify how we retrieve this count.

tstoeckler’s picture

tstoeckler
he/him
Primary language German
Location Essen, Germany
commented

Thanks for all the merges in the other issues, that's really great!

Absolutely no worries, I just wanted to get something working locally for now, so even better if you're able to improve the code beyond what I did.

  • 0e328b7a committed on 3.0.x 
    fix #3543334: \Drupal\l10n_community\L10nTranslator::getStrings() breaks...
gábor hojtsy’s picture

gábor hojtsy
he/him
Primary language Hungarian
Location Hungary
commented
Issue summary: View changes
Status: Needs review » Fixed

Found this with my LLM when I was running tests locally. It suggested a sensible rewrite of the query as subquery instead of group by. I'll do a performance evaluation later which may undo this in some form as it sounds scary on the face of it. But keeping that for now as it works :)

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.