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

Support for script condition #111

Closed
wants to merge 1 commit into from
Closed

Support for script condition #111

wants to merge 1 commit into from

Conversation

tibee
Copy link

@tibee tibee commented Oct 25, 2016

Q A
Is bugfix? no
New feature? yes
Breaks BC? no
Tests pass? yes
Fixed issues

@cebe
Copy link
Member

cebe commented Oct 27, 2016

can you show an example code on how you'd use it please?

@tibee
Copy link
Author

tibee commented Oct 27, 2016

Assume I have a decimal field named˛flagfield, in which I store many binary flags as bitwise summary.
For example, 101110 is stored as 46 decimal.

$query->andWhere([
    "script",
    "(doc['flagfield'].value & 4) == 4",
]);

@beowulfenator
Copy link
Collaborator

beowulfenator commented Oct 27, 2016

I think it's a good idea to add support for script filters, but this isn't the best way of doing so. This implementation creates the same unfixable problem as the addAggregation method introduced earlier. Consider the structure of ES script filter:

{"script" : {
    "inline" : "doc['num1'].value > params.param1",
    "lang"   : "painless",
    "params" : {
        "param1" : 5
    }
}}

Let's extend your example slightly:

$query->andWhere([
    "script",
    "(doc['flagfield'].value & 4) == myval",
    ["myval" => 4]
]);

First operand is the string "script", second operand is the script itself. You are essentially taking the third operand (["myval" => 4]) and passing it as params to ElasticSearch.

As you see, there is no way to specify which language the script is in (the "lang" param). Sure, you could add the fourth optional operand. Also, there's no way to reference a script file. Sure, you could add the fifth optional operand. And keep adding more operands as ElasticSearch extends its DSL. This is not the way to go.

If you are so keen on having this simplfied implementation, let's do it this way. You can check whether the second operand (the script itself) is not an array. If it is not an array, do it your way. If it is an array, pass it as the object for the script filter:

$query->andWhere([
    "script",
    [
        "inline" => "doc['num1'].value > params.param1",
        "lang" => "painless",
        "params" => [
            "param1" => 5,
        ],
    ],
]);

becomes

{"script" : {
    "inline" : "doc['num1'].value > params.param1",
    "lang"   : "painless",
    "params" : {
        "param1" : 5
    }
}}

In any case, this is a worthwhile update, but it needs tests and docs too.

@tibee
Copy link
Author

tibee commented Oct 27, 2016

I see, the script operator can have different number of operands.

What do you think of having only 1 operand, an array, and you could pass any elements you want / ES supports. Inline, lang, params, whatever.

It would be even more simple than my commit.

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

Successfully merging this pull request may close these issues.

4 participants