serhiy-bzhezytskyy opened a new pull request, #69:
URL: https://github.com/apache/solr-orbit/pull/69
### Description
`_translate_query_node` folded `gt` into `gte` and `lt` into `lte`, then
always emitted square brackets:
```python
lo = bounds.get("gte", bounds.get("gt", "*"))
hi = bounds.get("lte", bounds.get("lt", "*"))
...
return f"{field}:[{lo} TO {hi}]"
```
So every translated range came out inclusive at both ends, and an exclusive
bound in the source workload
silently became an inclusive one. Solr supports both spellings — `[a TO b]`
and `[a TO b}` — so nothing
about the engine required this.
Effect on a shipped workload. `nyc_taxis`'s `range` operation asks upstream
for `{"gte": 5, "lt": 15}`,
and the conversion produces `total_amount:[5 TO 15]`, which also matches
`total_amount == 15`:
| corpus | `[5 TO 15]` | `lt: 15` | diff |
|---|---:|---:|---:|
| `documents-1k.json` (what `--test-mode` downloads) | 576 | 573 | 3 |
| Bernoulli sample of 300,649 documents | 191,735 | 191,441 | 294 |
The three extra documents in the 1k corpus are exactly those with
`total_amount == 15`:
```
total_amount 15.0, trip_distance 0.13, pickup 2015-01-01T00:35:03Z
total_amount 15.0, trip_distance 19.79, pickup 2015-01-01T00:36:51Z
total_amount 15.0, trip_distance 3.63, pickup 2015-01-01T00:38:45Z
```
The same difference appears in OpenSearch on its own request body — changing
`lt` to `lte` returns
191,735 against 191,441 — so it is a property of the bound, not of either
engine.
This is not about timing: 12 runs of each query on the 1k corpus give 0–4 ms
either way. It is that the
converted operation matches documents the operation it was converted from
excludes, which makes a
Solr/OpenSearch comparison a comparison of two different questions. Note the
direction — the conversion
matches *more*, so Solr does work the operation does not ask for.
The workload file that carries the current bound is fixed separately in
`apache/solr-orbit-workloads`;
each change stands alone, and only this one keeps the next conversion from
reintroducing it.
### Issues Resolved
Resolves #68
### Testing
- [x] New functionality includes testing
- `test_range_query` covered only `gte`/`lte` — the one combination the old
code got right — and
asserted with `assertIn("fare_amount", …)` and `assertIn("TO", …)`, which
pass for `[5 TO 100]`,
`[5 TO 100}` and `{5 TO 100}` alike. It is an `assertEqual` now.
- Two new tests: `test_exclusive_bounds_use_curly_brackets` covers all four
bound combinations, and
`test_a_missing_bound_is_open_and_inclusive` covers the open-ended forms.
- Verified red without the fix: with `main`'s `query.py` restored under the
new tests, both fail —
`'fare_amount:[5 TO 100}' != 'fare_amount:[5 TO 100]'` and
`'fare_amount:{5 TO *]' != 'fare_amount:[5 TO *]'`. With the fix, both
pass.
- Full unit suite: `196 passed`.
Worth flagging on its own: the existing test was green over this defect for
two independent reasons, and
either alone was enough. It exercised the only correct combination, and its
assertions could not see the
brackets even if it had exercised a wrong one.
---
By submitting this pull request, I confirm that my contribution is made
under the terms of the Apache
2.0 license.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
