Page MenuHomePhabricator

Global Advanced search should support "any()" and "not()" functions in the "Tags" field, like Maniphest's Advanced Search does
Open, LowestPublic

Description

Advanced search form differs from Subscribed edit-query form in some points. One of them is a possibility to search in projects by functions not() and any(). These functions would be useful in advanced search form too.

Steps to reproduce

  1. Open Advanced search form, e.g. by clicking on magnifier icon next to the blank search box in the top right corner of this page
  2. Imagine you want to search all tasks in VisualEditor project but exclude Patch-For-Review tasks
  3. Fill in "task" to Document type field, fill in "VisualEditor not(Patch-For-Review)" to Tags field
  4. Click "Search" button

Expected result
Phabricator should find all tasks in VisualEditor project but exclude Patch-For-Review tasks. This worked in the past and should work per Typeahead Function Help.

Current result
This doesn't work. If I want to search all tasks in VisualEditor project but exclude Patch-For-Review tasks, I have to follow the following workaround steps:

  1. Click "Tasks and bugs" in left menu on the main page
  2. Click "Subscribed" in left menu
  3. Click "Edit query" button
  4. Empty all fields and continue from step 3 in Steps to reproduce section

Event Timeline

Dvorapa created this task.Jun 16 2017, 12:40 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 16 2017, 12:40 PM
Aklapper changed the task status from Open to Stalled.Jun 16 2017, 12:59 PM

I don't understand what to see where or not. Can you please follow https://mediawiki.org/wiki/How_to_report_a_bug and provide

  • Steps to reproduce, step by step, including URLs when needed
  • Expected outcome
  • Actual outcome

Thanks!

Unrelated to the project in Labs (or did you test on Labs? Unsure, as no steps were provided...), hence removing VPS-project-Phabricator for the time being.

Dvorapa updated the task description. (Show Details)Jun 16 2017, 1:17 PM

@Aklapper I've updated the desc

Aklapper triaged this task as Lowest priority.Jun 16 2017, 1:29 PM
Aklapper edited projects, added Phabricator (Upstream); removed Phabricator.

Fill in "task" to Document type field, fill in "VisualEditor not(Patch-For-Review)"

This refers to the "Tags" field I guess. Confirming.

Restricted Application added a project: Upstream. · View Herald TranscriptJun 16 2017, 1:29 PM
Aklapper renamed this task from Advanced search should support any() and not() functions to Global Advanced search should support "any()" and "not()" functions in the "Tags" field, like Maniphest's Advanced Search does.Jun 16 2017, 1:30 PM
Aklapper changed the task status from Stalled to Open.
Dvorapa updated the task description. (Show Details)Jun 16 2017, 1:34 PM
Dvorapa updated the task description. (Show Details)
Dvorapa updated the task description. (Show Details)

Unfortunately I do not think this is very feasible. I already inquired upstream about this and was told that it's complicated...

Let me see if I can find the upstream task.

See also T190342 and T224082.

I believe that swapping this to a ProjectFunctionDatasource would "just work", provided Global search is served by Ferret. On this install, Global search is served by ElasticSearch instead of Ferret, so swapping the tokenizer would cause us to start submitting none(), not(...), any(...), etc., queries to ElasticSearch as string literals. All of these queries will fail so the net effect would be that using any tokenizer function would never match any results.

Supporting these queries against ElasticSearch would require implementing the "EdgeLogic" behavior from PhabricatorCursorPagedPolicyAwareQuery in terms that ElasticSearch can understand. Some of these constraints are very complicated -- for example, if you search for "Tags: A, B" the search really means "find tasks tagged with A, and/or any child of A, and B, and/or any child of B, but they must be tagged with at least one project in the A set and at least one project in the B set". In MySQL, we express this like this (the "ancestor" edge logic operator):

case PhabricatorQueryConstraint::OPERATOR_ANCESTOR:
  // This is tricky. We have a query which specifies multiple
  // projects, each of which may have an arbitrarily large number
  // of descendants.

  // Suppose the projects are "Engineering" and "Operations", and
  // "Engineering" has subprojects X, Y and Z.

  // We first use `FIELD(dst, X, Y, Z)` to produce a 0 if a row
  // is not part of Engineering at all, or some number other than
  // 0 if it is.

  // Then we use `IF(..., idx, NULL)` to convert the 0 to a NULL and
  // any other value to an index (say, 1) for the ancestor.

  // We build these up for every ancestor, then use `COALESCE(...)`
  // to select the non-null one, giving us an ancestor which this
  // row is a member of.

  // From there, we use `COUNT(DISTINCT(...))` to make sure that
  // each result row is a member of all ancestors.
  if (count($list) > 1) {
    $idx = 1;
    $parts = array();
    foreach ($list as $constraint) {
      $parts[] = qsprintf(
        $conn,
        'IF(FIELD(%T.dst, %Ls) != 0, %d, NULL)',
        $alias,
        (array)$constraint->getValue(),
        $idx++);
    }
    $parts = qsprintf($conn, '%LQ', $parts);

    $select[] = qsprintf(
      $conn,
      'COUNT(DISTINCT(COALESCE(%Q))) %T',
      $parts,
      $this->buildEdgeLogicTableAliasAncestor($alias));
  }
  break;

I'm not experienced enough with ElasticSearch to know if this is easy/hard/impossible to express with Elastic syntax.

Ideally, the simplest approach is to turn off Elastic search and cross our fingers; if Ferret works fine and we don't actually need the Elastic engine, we can eventually deprecate/remove Elastic support and this becomes a 1-line change.

If we need to retain Elastic support, implementing all of these functions against the Elastic engine is likely quite involved. (Another possibility is to pull this part of the query out of Elastic and execute it in the application instead, but that creates a different set of also-hard problems.)

@epriestley: I'd like to try switching off elastic and see how it goes for a while. If we don't get a bunch of complaints then it's a big win in terms of code maintenance going forward.