Differential D21400

Require rows passed to "loadAllFromArray()" have unique keys
ClosedPublic
Actions

Authored by epriestley on Jul 9 2020, 5:56 PM.

Tags

None

Subscribers

None

Details

Reviewers: None
Commits: rP72f149bf399a: Require rows passed to "loadAllFromArray()" have unique keys

Summary

See PHI1809, which identified a bug in Project search where queries with a large number of slugs could paginate improperly.

This change detects problems in this category: cases where multiple rows with the same ID are passed to "loadAllFromArray()". It's likely that all cases it detects are cases where a GROUP BY is missing.

Since this might have some false positives or detect some things which aren't fundamentally problematic, I'm planning to hold it until the next release.

Test Plan

Reverted D21399, then created a project with multiple slugs and queried for one of them via "project.search". Hit this new exeception.
Browsed around a bit, didn't immediately catch any collateral damage.

Diff Detail

Repository

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

Event Timeline

epriestley created this revision.Jul 9 2020, 5:56 PM

Harbormaster completed remote builds in B24759: Diff 50940.Jul 9 2020, 5:58 PM

epriestley requested review of this revision.Jul 9 2020, 5:58 PM

This revision was not accepted when it landed; it landed in state Needs Review.Aug 12 2020, 4:05 PM

Closed by commit rP72f149bf399a: Require rows passed to "loadAllFromArray()" have unique keys (authored by epriestley). · Explain Why

This revision was automatically updated to reflect the committed changes.

epriestley added a commit: rP72f149bf399a: Require rows passed to "loadAllFromArray()" have unique keys.

epriestley mentioned this in T13581: DifferentialRevisionQuery omits GROUP BY when querying by hash.Sep 16 2020, 12:16 AM

epriestley mentioned this in D21486: Add a missing "GROUP BY" to MailQuery when querying for multiple recipients.Oct 30 2020, 7:57 PM

epriestley mentioned this in rP671986592baf: Add a missing "GROUP BY" to MailQuery when querying for multiple recipients.Oct 30 2020, 8:02 PM

Revision Contents
Changeset List

Path

Size

src/

infrastructure/

storage/

lisk/

13 lines

Diff 51039

src/infrastructure/storage/lisk/LiskDAO.php

Loading...