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 sort on geo_point fields #63

Open
alevani opened this issue Sep 17, 2021 · 6 comments
Open

Cannot sort on geo_point fields #63

alevani opened this issue Sep 17, 2021 · 6 comments
Labels
enhancement New feature or request

Comments

@alevani
Copy link
Contributor

alevani commented Sep 17, 2021

It seems that the "orderBy" function does not support sorting on fields of type geo_point.

The bellow function call will return every index entries within a distance of $location['distance'] of the given location coordinate.

$searcher->must(new GeoDistance('locations', [ (float) $location['lat'], (float) $location['lon']], $location['distance'] ?? '200km'));

When trying to apply a orderBy('locations'), I get this error:

{"error":{"root_cause":[{"type":"illegal_argument_exception","reason":"can't sort on geo_point field without using specific sorting feature, like geo_distance"}],"type":"search_phase_execution_exception","reason":"all shards failed","phase":"query","grouped":true,"failed_shards":[{"shard":0,"index":"offers","node":"66qPfbXCQ7Kch-EAzjHhOA","reason":{"type":"illegal_argument_exception","reason":"can't sort on geo_point field without using specific sorting feature, like geo_distance"}}],"caused_by":{"type":"illegal_argument_exception","reason":"can't sort on geo_point field without using specific sorting feature, like geo_distance","caused_by":{"type":"illegal_argument_exception","reason":"can't sort on geo_point field without using specific sorting feature, like geo_distance"}}},"status":400}

I have tried a few things but none of my attempts seem to work. Is this even out of the box supported?

What the ElasticSearch query should look like:

GET offers/_search
{
  "size": 1,
  "query": {
    "bool": {
      "filter": {
        "geo_distance": {
          "distance": "100km",
          "locations": {
                    "lat": -89.440879,
                  "lon": 124.327279
          }
        }
      }
    }
  },
  "sort": [
    {
      "_geo_distance": {
        "locations": {
                  "lat": -89.440879,
                  "lon": 124.327279
        }
      }
    }
  ]
}

Thanks!

@Jeroen-G
Copy link
Owner

Using debugging, what does the query look like that is executed but returns the Elastic error?

@Jeroen-G
Copy link
Owner

Jeroen-G commented Sep 17, 2021

Is this even out of the box supported?

Well, AFAIK GeoDistance is not something included in this package, so I wouldn't necessarily expect it to work immediately in all facets of Scout/Elastic/Explorer. But perhaps together we can make it work!

@alevani
Copy link
Contributor Author

alevani commented Sep 17, 2021

Looks like it is impossible to catch, server crashes before being able to return anything.

To query over a GeoPoint field one need to use specific sorting features (as the error yields). If I use sort on a regular field (that is ....->orderBy('discount')), the debugger yields:

.
.
.
"sort": [
    {
      "discount": "asc"
    }
 ]

What the package does when using orderBy is wrapping the field in a sort object as above. So when ordering on the locations field, it probably does the same:

.
.
.
"sort": [
    {
      "locations": "asc"
    }
 ]

Which if ran on ElasticSearch yields the same error as mentioned in the issue's description.

My wild guess to solve the problem would be to wrap field of type GeoPoint in a different way, such as:

.
.
.
"sort": [
    {
      "_geo_distance": {
        "locations": {
                  "lat": -89.440879,
                  "lon": 124.327279
        }
      }
    }
 ]

Thereby having an option to either pass the orderBy function an specific object, or by tweaking the JeroenG\Explorer\Domain\Syntax\Sort; into being able to receive "sorting feature" parameters (which I am trying to do ATM).

@alevani
Copy link
Contributor Author

alevani commented Sep 17, 2021

Note that: The function also need to be able the sorting feature, which in the _geo_distance case is an array with a lat and lon row.

I think there's potential for expending this package to include the so called sorting feature (I am not an ElasticSearch expert, pardon if I am missing something) 🔥 .

@Jeroen-G Jeroen-G added the enhancement New feature or request label Sep 19, 2021
@Jeroen-G
Copy link
Owner

Ok, I have looked at the sorting feature and the current implementation is quite naive considering everything that is possible with ES. It will take quite some work to write a new implementation and rework how sorting is now done, so I cannot promise any timeline when I would be finished (or start) with this.

I think for the time being you can circumvent it by using the Finder directly instead of your model, i.e.:

$query = Query::with(new BoolQuery());
$builder = new SearchCommand('my_index', $query);
$query->setSort([new MyCustomGeoSort(....)]);
$finder = new Finder($client, $builder);
$results = $finder->find();

(Have a look at the Finder's tests and Elastic Engine to get to know it more)

@riccardo993
Copy link

@Jeroen-G if you are working on the sort feature, it would be nice to have the Nested Sort too. Reading to your last reply I ended up with this solution. In this way I can have my data with paginations too. Hope it helps.

  1. Create a custom SortNested class which extends your Sort class (otherwise the assertion throws an exception):
namespace App\Services\Projection\Scout;

use JeroenG\Explorer\Domain\Syntax\Sort;
use Webmozart\Assert\Assert;

class SortNested extends Sort
{
    public const ASCENDING = 'asc';

    public const DESCENDING = 'desc';

    private string $path;

    private string $field;

    private string $order;

    public function __construct(string $path, string $field, string $order = self::ASCENDING)
    {
        parent::__construct($field, $order);

        $this->path = $path;
        $this->field = $field;
        $this->order = $order;
        Assert::inArray($order, [self::ASCENDING, self::DESCENDING]);
    }

    public function build(): array
    {
        return [
            $this->field => [
                'order' => $this->order,
                'nested' => [
                    'path' => $this->path,
                ],
            ],
        ];
    }
}
  1. Create a custom class that fetches the result:
namespace App\Services\Projection\Reader;

use JeroenG\Explorer\Application\DocumentAdapterInterface;
use JeroenG\Explorer\Infrastructure\Scout\ScoutSearchCommandBuilder;
use Laravel\Scout\Builder;

class GetScoutResult implements \App\Services\Contracts\Projection\Reader\GetScoutResultInterface
{
    private DocumentAdapterInterface $documentAdapter;

    public function __construct(DocumentAdapterInterface $documentAdapter)
    {
        $this->documentAdapter = $documentAdapter;
    }

    /**
     * {@inheritDoc}
     */
    public function execute(Builder $query, int $perPage, int $page, array $sort = []): array
    {
        $offset = $perPage * ($page - 1);

        $normalizedBuilder = ScoutSearchCommandBuilder::wrap($query);
        $normalizedBuilder->setOffset($offset);
        $normalizedBuilder->setLimit($perPage);
        $normalizedBuilder->setSort($sort);

        $result = $this->documentAdapter->search($normalizedBuilder);

        return [
            'hits' => $result->hits(),
            'aggregations' => $result->aggregations(),
            'count' => $result->count(),
        ];
    }
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants