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
Unknown Object (File)
Wed, Dec 11, 1:39 PM
Unknown Object (File)
Wed, Dec 11, 10:22 AM
Unknown Object (File)
Mon, Dec 9, 9:00 AM
Unknown Object (File)
Sat, Dec 7, 5:41 PM
Unknown Object (File)
Sat, Dec 7, 12:17 AM
Unknown Object (File)
Fri, Dec 6, 12:48 PM
Unknown Object (File)
Wed, Dec 4, 11:46 PM
Unknown Object (File)
Sun, Dec 1, 4:47 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.