Range Searches

Here's the patch. It has been tested with other data types which can use range filters.

@Kirk Brown, thank you for the contribution!

I committed your code to a merge request to make it easier for me to review.

I took a very quick look at your patch. It looks good. As you mention in the Issue Summary, it needs tests.

Could you also update the Issue Summary's "Steps to reproduce" to include the error you get on an unpatched module? People often find issues by searching for the error message, so this is often helpful.

Updating issue status

I notice that Testbot is detecting some failing tests.

The failures in \Drupal\Tests\elasticsearch_connector\Unit\SearchAPI\Query\FilterBuilderTest are expected, given the changes you made. Basically, FilterBuilderTest looks at the queries that FilterBuilder generates for given inputs, and since your patch changes how the queries are built, test failures here are expected. The upshot is that to fix the tests, you need to change what FilterBuilderTest expects.

The test failures in \Drupal\Tests\elasticsearch_connector\Kernel\ElasticSearchBackendTest might be a bit harder to resolve.

ElasticSearchBackendTest runs queries against an Elasticsearch instance running in the test environment, and checks that the query results make sense. We inherit most of the tests in that file from Search API's \Drupal\Tests\search_api\Kernel\BackendTestBase; which creates, indexes, and searches content created by \Drupal\Tests\search_api\Functional\ExampleContentTrait.

It looks like the part of the test that is failing in your patch is in ElasticSearchBackendTest::searchSuccess(), which is a part that deals with ranges. In particular, it's looking at which entities are returned when querying the date ranges that the entity was created (i.e.: queries on the created field, which are set in \Drupal\Tests\elasticsearch_connector\Kernel\ElasticSearchBackendTest::addTestEntity()). Note that this is completely different than the original test it overrides in BackendTestBase::searchSuccess(), which was testing ranges on the entities' width field (a decimal or float number).

As a bit of historical context, when we ported elasticsearch_connector-8.x-7.x to elasticsearch_connector-8.0.x, I remember several failing tests in that class, and I believe that we overrode some of the failing test functions when we had exhausted all other ideas for why they were failing. Based on the fact that we overrode searchSuccess(), it is likely that range searches weren't working for decimal/float numbers — but if that is true, then I would have expected range queries for created-date-field values to fail as well.

Anyway, the specific part of the failing test is supposed to find entities that were created strictly-after (gt) 2008-06-28; and the test after your changes is returning additional test entities (i.e.: with created-field dates of 2003-07-10 and 2008-06-28), which seems wrong.

On the other hand, it seems like deleting our overrides (i.e.: ElasticSearchBackendTest::searchSuccess() and ElasticSearchBackendTest::addTestEntity(), i.e.: and going back to the original BackendTestBase::searchSuccess() range-searches-by-weight) fixes things? All that being said, because I don't remember the exact circumstances that lead us to overriding searchSuccess() in that way, I would want to know why range queries on the created-date-field seemed to be working before I would be comfortable with deleting our override.

(sorry for the noise: I rebased onto the latest 8.0.x, so that the phpstan pipeline wouldn't warn us about a deprecation that was fixed in 8.0.x)