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

feat(elasticsearch): filtering on nested fields #5835

Merged
merged 3 commits into from
Oct 17, 2023

Conversation

jonnyeom
Copy link
Contributor

@jonnyeom jonnyeom commented Sep 18, 2023

Lets try again with added tests!

A better version of 5820, which was reverted

Q A
Branch? main
Tickets api-platform/api-platform#1375 and #5642
License MIT
Doc PR WIP

It is inline with elasticsearch's usage of nested documents

The nested type is a specialized version of the object data type that allows arrays of objects to be indexed

see elasticsearch nested docs

In the case of foo.bar.baz

  1. if bar is an array of objects, we return foo.bar.
  2. if bar is not an array, it does not qualify to be a nested property (according to elasticsearch) and we return null.

This allows Filter and Match queries such as

GET /object-array-object-string/_search

{
  "query": {
    "nested": {
      "path": "object.array",
      "query": {
        "bool": {
          "must": [
            { "match": { "object.array.object.property": "search-value" } }
          ]
        }
      }
    }
  }
}

@soyuka
Copy link
Member

soyuka commented Sep 19, 2023

can you fix phpunit tests on elasticsearch? to run them:

cd src/Elasticsearch
composer update
./vendor/bin/simple-phpunit

@jonnyeom
Copy link
Contributor Author

jonnyeom commented Sep 19, 2023

can you fix phpunit tests on elasticsearch? to run them:

cd src/Elasticsearch
composer update
./vendor/bin/simple-phpunit

Yep. currently rethinking the approach.

The resulting nested match query seems to be a bit different from what elasticsearch has documented

It still works, im not sure if it is a hidden feature or a bug. So will confirm with them and proceed accordingly.

@jonnyeom jonnyeom force-pushed the feat/filtering-on-nested-fields branch from 1e2c257 to 7ea551d Compare September 20, 2023 21:00
@jonnyeom jonnyeom marked this pull request as ready for review September 21, 2023 14:22
@jonnyeom jonnyeom marked this pull request as draft September 21, 2023 14:28
@soyuka soyuka marked this pull request as ready for review October 17, 2023 10:35
@soyuka soyuka merged commit 6b79b6f into api-platform:main Oct 17, 2023
42 of 43 checks passed
@soyuka soyuka added this to the 3.3 milestone Oct 17, 2023
@Renrhaf
Copy link
Contributor

Renrhaf commented Oct 17, 2023

Great ! Thank you @jonnyeom

Fr13nzzz pushed a commit to Fr13nzzz/api-platform-core-fork that referenced this pull request Nov 23, 2023
* feat(elasticsearch): filtering on nested fields

* Add additional checks and function doc for less confusion

* Fixing tests
MariusJam pushed a commit to MariusJam/core that referenced this pull request Nov 24, 2023
* feat(elasticsearch): filtering on nested fields

* Add additional checks and function doc for less confusion

* Fixing tests
soyuka pushed a commit to anoziere/core that referenced this pull request Dec 18, 2023
* feat(elasticsearch): filtering on nested fields

* Add additional checks and function doc for less confusion

* Fixing tests
priyadi pushed a commit to priyadi/core that referenced this pull request Jan 3, 2024
* feat(elasticsearch): filtering on nested fields

* Add additional checks and function doc for less confusion

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

Successfully merging this pull request may close these issues.

3 participants