ElasticsearchDSL icon indicating copy to clipboard operation
ElasticsearchDSL copied to clipboard

Field Extra Parameters are wrong on some query

Open homersimpsons opened this issue 6 years ago • 2 comments

For some query like the RangeQuery or the TermQuery an extra parameter like _name (used for Named Queries) won't be at the top level.

Example:

$range = new RangeQuery('field', ['gt' => 0]);
$range->addParameter('_name', 'myname');
$range->toArray();
/*
Result:
[
    'range' => [
        'field' => [
            'gt' => 0,
            '_name' => 'myname'
        ]
    ]
]
Instead of expected:
[
    'range' => [
        'field' => [
            'gt' => 0
        ],
        '_name' => 'myname'
    ]
]
*/

This is due to the use of parameters as query parameters and not extra ones https://github.com/ongr-io/ElasticsearchDSL/blob/197eeddacccf2195bd7a9e0572ef24bb50a700c8/src/Query/TermLevel/RangeQuery.php#L72

It's the same for TermQuery https://github.com/ongr-io/ElasticsearchDSL/blob/197eeddacccf2195bd7a9e0572ef24bb50a700c8/src/Query/TermLevel/TermQuery.php#L70 I think here the processArray() should be called after...

This works with a Terms query:

$terms = new TermsQuery('field', ['values']);
$terms->addParameter('_name', 'myname');
$terms->toArray();
/*
Result:
[
    'terms' => [
        'field' => [
            'values'
        ]
        '_name' => 'myname'
    ]
]
*/

homersimpsons avatar Apr 30 '19 15:04 homersimpsons

An excellent catch @homersimpsons. Will write a test for those cases and will double check if repositioning processArray() fix the issue.

saimaz avatar May 02 '19 05:05 saimaz

@saimaz Thank you.

Moving the processArray() won't give the expected result, for example if one use 'boost' => 2.0 he expect it to be at the FieldLevel (i.e. 'field' => ['boost' => 2.0, ...]) so this processArray() should stay here...

I guess the best is to get 2 array of parameters:

  • FieldParameters: That will be used at FieldLevel, and remains in constructor
  • QueryParameters: That would be used at QueryLevel (and get accessed only via accessors ? or last param in constructor ?)

Then in the process of building the array this will be generated

[
    $this->field => $this->fieldParameters,
    ...$this->queryParameters // (via array_merge / processArray())
]

So the resulting Query would be

[
    $query->getType() => [
        'field' => [FieldParameters],
        ...QueryParameters
    ]
]

homersimpsons avatar May 02 '19 08:05 homersimpsons