Skip to content

Fix empty minimum_should_match in query

Terri Chu requested to merge 470046-fix into master

What does this MR do and why?

Related to #470046

For the Gitlab::Elastic::BoolExpr class, ensure that calls to_json removes any null values by calling to_h on the object first. to_h already removes null values.

Added missing specs for whole class.

Pipeline validation

I wondered why this wasn't caught during the CI pipelines when it was introduced. I think it's due to the issue happening on 7.7.0 and not later versions of Elasticsearch. Search specs include Elasticsearch 7.17.6 and 8.11.4. I forced the CI to use 7.7.0 and also had to force the ES7 specs to run to see the failure in https://gitlab.com/gitlab-org/gitlab/-/pipelines/1364860357. Those changes have been removed from the MR.

MR acceptance checklist

Please evaluate this MR against the MR acceptance checklist. It helps you analyze changes to reduce risks in quality, performance, reliability, security, and maintainability.

Screenshots or screen recordings

No UI changes

How to set up and validate locally

  1. this was reported broken on Elasticsearch version 7.7.0. Note: I tried 7.8.0 and it wasn't broken there
  2. Stop the elasticsearch running in gdk gdk stop elasticsearch
  3. Get Elasticsearch 7.7.0 version running locally or with your gdk. Note: There was not a docker image for my laptop architecture available so I had to manually download elasticsearch-7.7.0-darwin-x86_64.tar.gz and run it using ./bin/elasticsearch
  4. IF you run it manually, you will need to edit the elasticsearch-7.7.0/config/elasticsearch.yml file to add these two lines
    xpack.security.enabled: false
    discovery.type: "single-node"
  5. run the following in master branch (both should fail) and this branch (both should pass)
  6. run this spec: rspec ./ee/spec/lib/gitlab/elastic/search_results_spec.rb:962
  7. run this search against the index: http://localhost:9200/gitlab-development/_search
{
  "query": {
    "bool": {
      "must": [
        {
          "simple_query_string": {
            "_name": "blob:match:search_terms",
            "fields": [
              "blob.content",
              "blob.file_name",
              "blob.path"
            ],
            "query": "test",
            "default_operator": "and"
          }
        }
      ],
      "must_not": [],
      "should": [],
      "filter": [
        {
          "bool": {
            "should": [
              {
                "bool": {
                  "filter": [
                    {
                      "term": {
                        "visibility_level": {
                          "_name": "blob:authorized:project:any",
                          "value": 0
                        }
                      }
                    },
                    {
                      "terms": {
                        "_name": "blob:authorized:project:repository:enabled_or_private",
                        "repository_access_level": [
                          20,
                          10
                        ]
                      }
                    }
                  ]
                }
              },
              {
                "bool": {
                  "_name": "blob:authorized:project:visibility:10:repository:access_level",
                  "filter": [
                    {
                      "term": {
                        "visibility_level": {
                          "_name": "blob:authorized:project:visibility:10",
                          "value": 10
                        }
                      }
                    },
                    {
                      "terms": {
                        "_name": "blob:authorized:project:visibility:10:repository:access_level:enabled_or_private",
                        "repository_access_level": [
                          20,
                          10
                        ]
                      }
                    }
                  ]
                }
              },
              {
                "bool": {
                  "_name": "blob:authorized:project:visibility:20:repository:access_level",
                  "filter": [
                    {
                      "term": {
                        "visibility_level": {
                          "_name": "blob:authorized:project:visibility:20",
                          "value": 20
                        }
                      }
                    },
                    {
                      "terms": {
                        "_name": "blob:authorized:project:visibility:20:repository:access_level:enabled_or_private",
                        "repository_access_level": [
                          20,
                          10
                        ]
                      }
                    }
                  ]
                }
              }
            ],
            "_name": "blob:authorized:project"
          }
        },
        {
          "term": {
            "type": {
              "_name": "doc:is_a:blob",
              "value": "blob"
            }
          }
        },
        {
          "bool": {
            "_name": "non_archived",
            "should": [
              {
                "bool": {
                  "filter": {
                    "term": {
                      "archived": {
                        "value": false
                      }
                    }
                  }
                }
              },
              {
                "bool": {
                  "must_not": {
                    "exists": {
                      "field": "archived"
                    }
                  }
                }
              }
            ]
          }
        }
      ],
      "minimum_should_match": null
    }
  },
  "size": 20,
  "from": 0,
  "sort": [
    "_score"
  ],
  "highlight": {
    "pre_tags": [
      "gitlabelasticsearch→"
    ],
    "post_tags": [
      "←gitlabelasticsearch"
    ],
    "number_of_fragments": 0,
    "fields": {
      "blob.content": {},
      "blob.file_name": {}
    }
  }
}
Edited by Terri Chu

Merge request reports