Page MenuHomePhabricator

Fix a log warning when searching for ranges on custom "Date" fields
ClosedPublic

Authored by epriestley on Feb 28 2019, 5:37 PM.
Tags
None
Referenced Files
F15523743: D20225.id48298.diff
Mon, Apr 21, 3:22 AM
F15509037: D20225.id48285.diff
Wed, Apr 16, 8:46 AM
F15508243: D20225.id.diff
Wed, Apr 16, 3:43 AM
F15507275: D20225.diff
Tue, Apr 15, 6:07 PM
F15419986: D20225.id.diff
Mar 21 2025, 10:22 AM
F15419911: D20225.id48285.diff
Mar 21 2025, 9:53 AM
F15406679: D20225.id48285.diff
Mar 18 2025, 1:16 PM
F15399425: D20225.id48298.diff
Mar 17 2025, 4:29 AM
Subscribers

Details

Summary

See https://discourse.phabricator-community.org/t/traceback-rendering-task-query-in-dashboard/2450/.

It looks like this blames to D19126, which added some more complex constraint logic but overlooked "range" constraints, which are handled separately.

Test Plan
  • Added a custom "date" field to Maniphest with "search": true.
  • Executed a range query against the field.

Then:

  • Before: Warnings about undefined indexes in the log.
  • After: No such warnings.

Diff Detail

Repository
rP Phabricator
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php
1186–1187

This is where the other type of constraint gets built, with these values.

1470–1471

This is where we unpack these values from all constraint types unconditionally, then only use them for constraint types which will actually have legal values.

A more tailored patch would be to do this unpacking only after we find a constraint type which we'll examine them for, but it seems reasonable that all constraints should have the same fields available.

I cherry-picked this into our fork and it fixes the problem. ✅

This revision is now accepted and ready to land.Mar 1 2019, 12:19 AM
This revision was automatically updated to reflect the committed changes.