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

Field Extra Parameters are wrong on some query #280

Open
homersimpsons opened this issue Apr 30, 2019 · 2 comments
Open

Field Extra Parameters are wrong on some query #280

homersimpsons opened this issue Apr 30, 2019 · 2 comments

Comments

@homersimpsons
Copy link

homersimpsons commented Apr 30, 2019

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

$this->field => $this->getParameters(),

It's the same for TermQuery

$this->field => $query,

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'
    ]
]
*/
@saimaz
Copy link
Member

saimaz commented May 2, 2019

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

@homersimpsons
Copy link
Author

@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
    ]
]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants