Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

multiple termQuery not working #179

Closed
dijadev opened this issue Dec 22, 2016 · 11 comments
Closed

multiple termQuery not working #179

dijadev opened this issue Dec 22, 2016 · 11 comments
Assignees

Comments

@dijadev
Copy link

dijadev commented Dec 22, 2016

Hi !
I am trying to execute this query but it seems that it's not supported to add multiple termQuery.

<?php
       $search = new Search();
        $termQueryForTag1 = new TermQuery("tag", "wow");
        $termQueryForTag2 = new TermQuery("tag", "elasticsearch");
        $termQueryForTag3 = new TermQuery("tag", "dsl");

        $search->addQuery($termQueryForTag1);
        $search->addQuery($termQueryForTag2);
        $search->addQuery($termQueryForTag3, BoolQuery::MUST);

        $queryArray = $search->toArray();
        echo "<pre>";var_dump($queryArray);die(); 

Just the laste query was token

array(1) {
  ["query"]=>
  array(1) {
    ["term"]=>
    array(1) {
      ["tag"]=>
      string(3) "dsl"
    }
  }
} 

Is it a bug or I am doing it wrongly ?

PS: I am using ElasticSearch 2.3.3 and elasticsearch-dsl 2.2.

Thanks in advance,

@dijadev dijadev changed the title muliple termQuery not working multiple termQuery not working Dec 22, 2016
@saimaz
Copy link
Member

saimaz commented Dec 23, 2016

Hi @dijadev

The query is correct. But in your case it should be formed as 3 bool queries in the must operator.

{
 "query": {
 "bool":
  {
   "must":[
      {
         "term":{"tag":"wow"}
      },
      {
        "term":{"tag":"elasticsearch"}
      },
      { 
        "term":{"tag":"dsl"}
      }
    ]
  }
}
}

@dijadev
Copy link
Author

dijadev commented Dec 23, 2016

Hi @saimaz thank you for replying!
I tested by adding 3 bool queries but it does not work
Could you give the right query to get the array you showed ?

@saimaz
Copy link
Member

saimaz commented Dec 23, 2016

You query is right. The output should be like I posted.

        $search = new Search();
        $termQueryForTag1 = new TermQuery("tag", "wow");
        $termQueryForTag2 = new TermQuery("tag", "elasticsearch");
        $termQueryForTag3 = new TermQuery("tag", "dsl");

        $search->addQuery($termQueryForTag1);
        $search->addQuery($termQueryForTag2);
        $search->addQuery($termQueryForTag3);

        $queryArray = $search->toArray();
        echo "<pre>";json_encode($queryArray);die(); 

Generates:

{
 "query": {
 "bool":
  {
   "must":[
      {
         "term":{"tag":"wow"}
      },
      {
        "term":{"tag":"elasticsearch"}
      },
      { 
        "term":{"tag":"dsl"}
      }
    ]
  }
}
}

If not, sent me more info about your env. The exact ElasticsearchDSL version 2.2.x (what is x number), your PHP version.

@dijadev
Copy link
Author

dijadev commented Dec 23, 2016

I checked the code in the bundle and i found a key optional arg so i added for each query a different key as bellow and it works !

     $search = new Search();
      $bool = new BoolQuery();
        $termQueryForTag1 = new TermQuery("tag", "wow");
        $termQueryForTag2 = new TermQuery("tag", "elasticsearch");
        $termQueryForTag3 = new TermQuery("tag", "dsl");
        $bool->add($termQueryForTag1, BoolQuery::MUST,1);
       $bool->add($termQueryForTag2, BoolQuery::MUST,2);
       $bool->add($termQueryForTag3, BoolQuery::MUST,3);
       $search->addFilter($bool)
        $queryArray = $search->toArray();
        echo "<pre>";json_encode($queryArray);die(); 

This must be fixed or mentioned in the doc.

Thanks,

@saimaz
Copy link
Member

saimaz commented Dec 23, 2016

It's still very interesting why key is not generated and when you add the second query it drops first. I still cannot reproduce your issue :(.

The key is for case when you want to add some identifier where you later can grab that specific query from BoolQuery container. It should not be necessary for adding multiple TermQuery.

@dijadev
Copy link
Author

dijadev commented Dec 23, 2016

Ok I'll debug this as soon as i can and will keep you in touch :)

@puskic
Copy link

puskic commented Dec 24, 2016

Probably the problem is uniqid() in BoolQuery where he is assigning the key if not exists. I have the same problem on my local machine(xampp, php 7, windows 7) where microtime is just not refreshing quick enough. I solve it with hardcoded random generator

public function add(BuilderInterface $query, $type = self::MUST, $key = null)
{
    if (!in_array($type, [self::MUST, self::MUST_NOT, self::SHOULD, self::FILTER])) {
        throw new \UnexpectedValueException(sprintf('The bool operator %s is not supported', $type));
    }

    if (!$key) {
        $key = str_random(60);          //uniqid();
    }

    $this->container[$type][$key] = $query;

    return $key;
}

public static function random($length = 16)
{
    $string = '';

    while (($len = strlen($string)) < $length) {
        $size = $length - $len;

        $bytes = static::randomBytes($size);

        $string .= substr(str_replace(['/', '+', '='], '', base64_encode($bytes)), 0, $size);
    }

    return $string;
}

@saimaz
Copy link
Member

saimaz commented Dec 24, 2016

Thank you @puskic. The uniqid() is definitely a problem. Your provided solution would work, but it seems a bit overhead to calculate just a unique string. I believe there should be more optimized way to do this.

@puskic
Copy link

puskic commented Dec 24, 2016

I agree with you. I already have it, way not to use ;).
Probably

bin2hex(random_bytes(30))

will do the work.
Strange result will came out if $key overlaps, it should be unique enough...

@dijadev
Copy link
Author

dijadev commented Dec 26, 2016

Hi !

I confirm what @puskic said; this problem occurs just in windows env, and disappears when I deploy to a linux machine..
My debug on windows gives:

    public function add(BuilderInterface $query, $type = self::MUST, $key = null)
    {
        if (!in_array($type, [self::MUST, self::MUST_NOT, self::SHOULD, self::FILTER])) {
            throw new \UnexpectedValueException(sprintf('The bool operator %s is not supported', $type));
        }

        if (!$key) {
            $key = uniqid();
            echo $type.'=>'.$key.'<br>';
        }

        $this->container[$type][$key] = $query;

        return $key;
    }

It generates the same $key for the TermQuerys I added to BoolQuery

must=>5860f2aa7f977
must=>5860f2aa9fd1f
must=>5860f2aabc62e
must=>5860f2aabc62e
must=>5860f2aabcdfe

@saimaz saimaz self-assigned this Jan 4, 2017
@saimaz saimaz added the qa label Jan 4, 2017
@saimaz
Copy link
Member

saimaz commented Jan 27, 2017

Fixed in v2.2.1 release

@saimaz saimaz closed this as completed Jan 27, 2017
@saimaz saimaz removed the qa label Jan 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants