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

cannot have more than 1 script sorting #795

Closed
jiajiawang opened this issue May 11, 2021 · 3 comments
Closed

cannot have more than 1 script sorting #795

jiajiawang opened this issue May 11, 2021 · 3 comments
Labels

Comments

@jiajiawang
Copy link
Contributor

In elasticsearch, you can have as many script sorting as you want.
For example, there is a test index with two keyword fields "region" and "country", to sort by region and country alphabetically, by using script sorting you can do

{
  "sort": [
    {
      "_script": {
        "type": "string",
        "script": {
          "inline": "doc['region'].value"
        },
        "order": "desc"
      }
    },
    {
      "_script": {
        "type": "string",
        "script": {
          "inline": "doc['country'].value"
        },
        "order": "desc"
      }
    }
  ]
}

(In real world, you can simply do { "sort": ["region", "country"] }, this example is just to explain the issue)
However, when I use the order method of chewy I can't add more than 1 script sorting

TestsIndex.order([
    {
      "_script": {
        "type": "string",
        "script": {
          "inline": "doc['region'].value"
        },
        "order": "desc"
      }
    },
    {
      "_script": {
        "type": "string",
        "script": {
          "inline": "doc['country'].value"
        },
        "order": "desc"
      }
    }
  ])

it returns

<TestsIndex::Query {:index=>["tests"], :type=>["user"], :body=>{:sort=>[{"_script"=>{:type=>"string", :script=>{:inline=>"doc['country'].value"}, :order=>"desc"}}]}}>

The region script sorting part is gone.


Expected behavior

Expect query to be like

<TestsIndex::Query {:index=>["tests"], :type=>["user"], :body=>{:sort=>[{"_script"=>{:type=>"string", :script=>{:inline=>"doc['region'].value"}, :order=>"desc"}}, {"_script"=>{:type=>"string", :script=>{:inline=>"doc['country'].value"}, :order=>"desc"}}]}}>

Actual behavior

Actual query is

<TestsIndex::Query {:index=>["tests"], :type=>["user"], :body=>{:sort=>[{"_script"=>{:type=>"string", :script=>{:inline=>"doc['country'].value"}, :order=>"desc"}}]}}>

Steps to reproduce the problem

Define index

class TestsIndex < Chewy::Index
  define_type User do
    field :region, type: :keyword
    field :country, type: :keyword
  end
end

Then in console do

TestsIndex.order([
    {
      "_script": {
        "type": "string",
        "script": {
          "inline": "doc['region'].value"
        },
        "order": "desc"
      }
    },
    {
      "_script": {
        "type": "string",
        "script": {
          "inline": "doc['country'].value"
        },
        "order": "desc"
      }
    }
  ])

Version Information

Chewy 5.0.0 and 6.0.0 and 7.0.0
Elasticsearch 6.4
ruby 2.6.6p146 (2020-03-31 revision 67876) [x86_64-darwin19]
Rails 5.1.7


By looking into the source code, I found that the value of Chewy::Search::Parameters::Order is a hash and the issue is because the key of script sorting must always be "_script".

=> #<Chewy::Search::Parameters:0x00007fa76fb3c950
 @storages=
  {:order=>#<Chewy::Search::Parameters::Order:0x00007fa76fb3c6a8 @value={"_script"=>{:type=>"string", :script=>{:inline=>"doc['country'].value"}, :order=>"desc"}}>,
   :types=>#<Chewy::Search::Parameters::Types:0x00007fa76fb4f438 @value=[]>,
   :none=>#<Chewy::Search::Parameters::None:0x00007fa76fb4ea60 @value=false>,
   :filter=>#<Chewy::Search::Parameters::Filter:0x00007fa76fb4ea38 @value=#<Chewy::Search::Parameters::QueryStorage::Bool:0x00007fa76fb4e7e0 @minimum_should_match=nil, @must=[], @must_not=[], @should=[]>>,
   :query=>#<Chewy::Search::Parameters::Query:0x00007fa76fb4e3d0 @value=#<Chewy::Search::Parameters::QueryStorage::Bool:0x00007fa76fb4e1c8 @minimum_should_match=nil, @must=[], @must_not=[], @should=[]>>}>
@rabotyaga
Copy link
Contributor

Hey @jiajiawang 👋 !
Thanks for the report. Are you interested in submitting a PR with a fix?

@jiajiawang
Copy link
Contributor Author

Hey @jiajiawang 👋 !
Thanks for the report. Are you interested in submitting a PR with a fix?

Hey @rabotyaga, Yes, I'm interested in it.
Not sure how hard it would be, but I'm happy to give a try.
In term of the fix, I assume we'll have to change to use array as the storage of Order param.
Would love to hear your thoughts and ideas on it first.

@rabotyaga
Copy link
Contributor

Hey @jiajiawang 👋 !

Hey @rabotyaga, Yes, I'm interested in it.

Great! And thanks!

Not sure how hard it would be, but I'm happy to give a try.
In term of the fix, I assume we'll have to change to use array as the storage of Order param.
Would love to hear your thoughts and ideas on it first.

Yep, I believe we should change the Chewy::Search::Parameters::Order implementation to use Array under the hood.

jiajiawang added a commit to jiajiawang/chewy that referenced this issue May 19, 2021
To allow multiple sorting options that may have the same key name.
For example script based sorting whose key will always be `_script`.
jiajiawang added a commit to jiajiawang/chewy that referenced this issue May 19, 2021
Use Array to store the value of Chewy::Search::Parameters::Order.
To allow multiple sorting options that may have the same key name.
For example script based sorting whose key will always be `_script`.
jiajiawang added a commit to jiajiawang/chewy that referenced this issue May 19, 2021
Use Array to store the value of Chewy::Search::Parameters::Order.
To allow multiple sorting options that may have the same key name.
For example script based sorting whose key will always be `_script`.
rabotyaga pushed a commit that referenced this issue May 19, 2021
Use Array to store the value of Chewy::Search::Parameters::Order.
To allow multiple sorting options that may have the same key name.
For example script based sorting whose key will always be `_script`.
jiajiawang added a commit to jiajiawang/chewy that referenced this issue May 20, 2021
cyucelen pushed a commit to cyucelen/chewy that referenced this issue Jan 28, 2023
* [Fix toptal#795] **(Breaking)** Change implementation of Order param

Use Array to store the value of Chewy::Search::Parameters::Order.
To allow multiple sorting options that may have the same key name.
For example script based sorting whose key will always be `_script`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants