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

[Elasticsearch] Combine SearchCondition and EqualCondition resulting in erroneous ELS request #313

Closed
dhirtzbruch opened this issue Dec 12, 2023 · 3 comments
Labels
Adapter: Elasticsearch Elasticsearch Adapter releated issue bug Something isn't working

Comments

@dhirtzbruch
Copy link
Contributor

I am not sure if this might be an Elastic version or some other issue. We are currently using ELS 7.0, but I setup a blank ELS 8.0 too, which resultet in the same issue.

The following code works perfect:

        $raw = $this->engine->createSearchBuilder()
                            ->addIndex(self::INDEX_NAME)
                            ->addFilter(new SearchCondition($search))
                            ->limit($limit)
                            ->offset($offset)
                            ->getResult();

Resulting JSON body:

{
    sort: [ ],
    query: {
        query_string: {
            query: "TES*"
        }
    }
}

Changing the search configuration to the following code also works:

        $raw = $this->engine->createSearchBuilder()
                            ->addIndex(self::INDEX_NAME)
                            ->addFilter(new EqualCondition('branch', $branch->getShortname()))
                            ->getResult();

Resulting JSON body:

{
    sort: [ ],
    query: {
        bool: {
            filter: [
                {
                    term: {
                        branch.raw: {
                        value: "DE"
                        }
                    }
                }
            ]
        }
    }
}

However, combining both:

        $raw = $this->engine->createSearchBuilder()
                            ->addIndex(self::INDEX_NAME)
                            ->addFilter(new SearchCondition($search))
                            ->addFilter(new EqualCondition('branch', $branch->getShortname()))
                            ->getResult();

results in the following JSON body:

{
    sort: [ ],
        query: {
        query_string: {
            query: "TES*"
        },
        bool: {
            filter: [
                {
                    term: {
                        branch.raw: {
                            value: "DE"
                        }
                    }
                }
            ]
        }
    }
}

which at least both available ELS instances (7.0 and 8.0) don't seem to like...

{"error":{"root_cause":[{"type":"parsing_exception","reason":"[query_string] malformed query, expected [END_OBJECT] but found [FIELD_NAME]","line":1,"col":53}],"type":"parsing_exception","reason":"[query_string] malformed query, expected [END_OBJECT] but found [FIELD_NAME]","line":1,"col":53},"status":400}

As far as I can see, all three variants work when changing the code in ElasticsearchSearcher:

// old
//                $filter instanceof Condition\SearchCondition => $query['query_string']['query'] = $filter->query,
// new
                $filter instanceof Condition\SearchCondition => $query['bool']['must']['query_string']['query'] = $filter->query,
@alexander-schranz
Copy link
Member

alexander-schranz commented Dec 12, 2023

Hello @dhirtzbruch,

Thx for the report. So the issue is to use a EqualCondition and a SearchCondition together. Can you try to provide a failing test case before we have a deeper look how to integrate a fix here, so we first know which search engines fail in this case.

You could copy the following test https://github.com/schranz-search/schranz-search/blob/c91a3b7032785c91d541628714cd7ee2a75cb3bc/packages/seal/src/Testing/AbstractSearcherTestCase.php#L238 and add additional matching SearchCondition to it.

@alexander-schranz alexander-schranz added the bug Something isn't working label Dec 12, 2023
@dhirtzbruch
Copy link
Contributor Author

Of course, I'll check that tomorrow.

@alexander-schranz
Copy link
Member

fixed via #314

@alexander-schranz alexander-schranz added the Adapter: Elasticsearch Elasticsearch Adapter releated issue label Jan 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Adapter: Elasticsearch Elasticsearch Adapter releated issue bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants