Paths

Table of Contentst

Differential D20225

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

Authored by epriestley on Feb 28 2019, 5:37 PM.

Tags

None

Referenced Files

	F13078481: D20225.id.diff
	Tue, Apr 23, 4:26 PM

	Unknown Object (File)
	Sun, Apr 21, 1:57 PM

	Unknown Object (File)
	Fri, Apr 19, 3:02 AM

	Unknown Object (File)
	Fri, Apr 19, 3:02 AM

	Unknown Object (File)
	Fri, Apr 19, 3:02 AM

	Unknown Object (File)
	Thu, Apr 11, 4:13 AM

	Unknown Object (File)
	Sat, Mar 30, 7:33 AM

	Unknown Object (File)
	Fri, Mar 29, 12:53 PM

View All 89 Files

Subscribers

Details

Reviewers

Commits

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

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

Branch

range1

Lint

Lint Passed

Unit

Tests Passed

Build Status

Buildable 22147
Build 30263: Run Core Tests
Build 30262: arc lint + arc unit

Event Timeline

epriestley created this revision.Feb 28 2019, 5:37 PM

Harbormaster completed remote builds in B22147: Diff 48285.Feb 28 2019, 5:38 PM

epriestley requested review of this revision.Feb 28 2019, 5:38 PM

jbrownEP added a subscriber: jbrownEP.Feb 28 2019, 5:54 PM

epriestley added inline comments.Feb 28 2019, 6:46 PM

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. ✅

amckinley accepted this revision.Mar 1 2019, 12:19 AM

This revision is now accepted and ready to land.Mar 1 2019, 12:19 AM

Closed by commit rP27ea775fda5f: Fix a log warning when searching for ranges on custom "Date" fields (authored by epriestley). · Explain WhyMar 1 2019, 3:49 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

Path

Size

src/

infrastructure/

query/

policy/

PhabricatorCursorPagedPolicyAwareQuery.php

2 lines

Diff 48285

src/infrastructure/query/policy/PhabricatorCursorPagedPolicyAwareQuery.php

Loading...