Skip to content
This repository has been archived by the owner on Jul 1, 2021. It is now read-only.

Enable filter parameter during search #179

Merged
merged 2 commits into from
Jun 14, 2021
Merged

Enable filter parameter during search #179

merged 2 commits into from
Jun 14, 2021

Conversation

MarinPostma
Copy link
Contributor

@MarinPostma MarinPostma commented May 4, 2021

This pr makes the necessary changes to transplant in accordance with the specification on filters.

More precisely, it:

  • Removes the filters parameter
  • Renames facetFilters to filter
  • Allows either a string or an array to be passed to the filter param.

It doesn't allow the mixed syntax, that needs to be handled by milli.

close #81
close #140

@MarinPostma MarinPostma marked this pull request as ready for review May 6, 2021 09:28
@MarinPostma MarinPostma requested review from curquiza and irevoire May 6, 2021 09:28
@MarinPostma MarinPostma self-assigned this May 6, 2021
Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works on string 🚀

I got a panic I was not able to reproduce when using filter with number:

"filter": "price = 10"

with

"attributesForFaceting": {
        "genre": "string",
        "price": "number"
 },

Sorry I cannot give you the steps, here are the logs:

[2021-05-12T08:33:34Z INFO  milli::update::index_documents] Transform output indexed in 145.26ms
[2021-05-12T08:33:42Z INFO  actix_web::middleware::logger] 127.0.0.1:58853 "POST /indexes/indexUID/search HTTP/1.1" 400 169 "-" "PostmanRuntime/7.28.0" 0.001598
[2021-05-12T08:34:38Z INFO  actix_web::middleware::logger] 127.0.0.1:58854 "POST /indexes/indexUID/search HTTP/1.1" 200 263 "-" "PostmanRuntime/7.28.0" 0.011417
[2021-05-12T08:34:43Z INFO  actix_web::middleware::logger] 127.0.0.1:58856 "POST /indexes/indexUID/search HTTP/1.1" 200 241 "-" "PostmanRuntime/7.28.0" 0.010508
[2021-05-12T08:35:01Z INFO  actix_web::middleware::logger] 127.0.0.1:58858 "POST /indexes/indexUID/search HTTP/1.1" 200 561 "-" "PostmanRuntime/7.28.0" 0.010646
[2021-05-12T08:36:39Z INFO  actix_web::middleware::logger] 127.0.0.1:58863 "POST /indexes/indexUID/settings HTTP/1.1" 202 14 "-" "PostmanRuntime/7.28.0" 0.002318
[2021-05-12T08:36:44Z INFO  actix_web::middleware::logger] 127.0.0.1:58864 "GET /indexes/indexUID/settings HTTP/1.1" 200 253 "-" "PostmanRuntime/7.28.0" 0.001595
[2021-05-12T08:37:49Z INFO  actix_web::middleware::logger] 127.0.0.1:58867 "POST /indexes/indexUID/settings HTTP/1.1" 202 14 "-" "PostmanRuntime/7.28.0" 0.001841
[2021-05-12T08:37:51Z INFO  actix_web::middleware::logger] 127.0.0.1:58867 "GET /indexes/indexUID/settings HTTP/1.1" 200 253 "-" "PostmanRuntime/7.28.0" 0.001277
[2021-05-12T08:38:01Z INFO  actix_web::middleware::logger] 127.0.0.1:58869 "GET /indexes/indexUID/updates HTTP/1.1" 200 2027 "-" "PostmanRuntime/7.28.0" 0.003429
[2021-05-12T08:38:13Z INFO  actix_web::middleware::logger] 127.0.0.1:58871 "POST /indexes/indexUID/settings HTTP/1.1" 202 14 "-" "PostmanRuntime/7.28.0" 0.002393
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: IndexDocuments { documents_seen: 6, total_documents: 6 }
[2021-05-12T08:38:13Z INFO  milli::update::index_documents::store] We have seen 0.0 documents so far (21.28µs).
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: IndexDocuments { documents_seen: 0, total_documents: 6 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: IndexDocuments { documents_seen: 6, total_documents: 6 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: IndexDocuments { documents_seen: 6, total_documents: 6 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: IndexDocuments { documents_seen: 6, total_documents: 6 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: IndexDocuments { documents_seen: 6, total_documents: 6 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: IndexDocuments { documents_seen: 6, total_documents: 6 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: IndexDocuments { documents_seen: 6, total_documents: 6 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: IndexDocuments { documents_seen: 6, total_documents: 6 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: IndexDocuments { documents_seen: 6, total_documents: 6 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: IndexDocuments { documents_seen: 6, total_documents: 6 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: IndexDocuments { documents_seen: 6, total_documents: 6 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: IndexDocuments { documents_seen: 6, total_documents: 6 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: MergeDataIntoFinalDatabase { databases_seen: 0, total_databases: 8 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: MergeDataIntoFinalDatabase { databases_seen: 1, total_databases: 8 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: MergeDataIntoFinalDatabase { databases_seen: 2, total_databases: 8 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: MergeDataIntoFinalDatabase { databases_seen: 3, total_databases: 8 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: MergeDataIntoFinalDatabase { databases_seen: 4, total_databases: 8 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: MergeDataIntoFinalDatabase { databases_seen: 5, total_databases: 8 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: MergeDataIntoFinalDatabase { databases_seen: 6, total_databases: 8 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: MergeDataIntoFinalDatabase { databases_seen: 7, total_databases: 8 }
[2021-05-12T08:38:13Z INFO  meilisearch_http::index::updates] update 5: MergeDataIntoFinalDatabase { databases_seen: 8, total_databases: 8 }
[2021-05-12T08:38:13Z INFO  milli::update::index_documents] Transform output indexed in 118.30ms
thread 'tokio-runtime-worker' panicked at 'range end index 24 out of range for slice of length 6', /Users/curquiza/.cargo/git/checkouts/milli-00376cd5db949a15/792225e/milli/src/heed_codec/facet/facet_level_value_f64_codec.rs:18:24
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
[2021-05-12T08:38:31Z INFO  actix_web::middleware::logger] 127.0.0.1:58873 "POST /indexes/indexUID/search HTTP/1.1" 400 35 "-" "PostmanRuntime/7.28.0" 0.001595
thread 'tokio-runtime-worker' panicked at 'range end index 24 out of range for slice of length 6', /Users/curquiza/.cargo/git/checkouts/milli-00376cd5db949a15/792225e/milli/src/heed_codec/facet/facet_level_value_f64_codec.rs:18:24
[2021-05-12T08:38:43Z INFO  actix_web::middleware::logger] 127.0.0.1:58875 "POST /indexes/indexUID/search HTTP/1.1" 400 35 "-" "PostmanRuntime/7.28.0" 0.001444
thread 'tokio-runtime-worker' panicked at 'range end index 24 out of range for slice of length 6', /Users/curquiza/.cargo/git/checkouts/milli-00376cd5db949a15/792225e/milli/src/heed_codec/facet/facet_level_value_f64_codec.rs:18:24
[2021-05-12T08:39:01Z INFO  actix_web::middleware::logger] 127.0.0.1:58876 "POST /indexes/indexUID/search HTTP/1.1" 400 35 "-" "PostmanRuntime/7.28.0" 0.001532
[2021-05-12T08:39:07Z INFO  actix_web::middleware::logger] 127.0.0.1:58877 "GET /indexes/indexUID/settings HTTP/1.1" 200 270 "-" "PostmanRuntime/7.28.0" 0.001344

Still investigating on it!

@curquiza
Copy link
Member

I'm sorry, I did succeed to reproduce the panic 😢

@MarinPostma
Copy link
Contributor Author

bors try

bors bot added a commit that referenced this pull request Jun 2, 2021
@bors
Copy link
Contributor

bors bot commented Jun 2, 2021

@irevoire
Copy link
Member

irevoire commented Jun 3, 2021

bors merge

bors bot added a commit that referenced this pull request Jun 3, 2021
179: enable filter r=irevoire a=MarinPostma

This pr makes the necessary changes to transplant in accordance with the specification on filters.

More precisely, it:
- Removes the `filters` parameter
- Renames `facetFilters` to `filter`
- Allows either a string or an array to be passed to the filter param.

It doesn't allow the mixed syntax, that needs to be handled by milli.

close #81
close #140


Co-authored-by: Marin Postma <postma.marin@protonmail.com>
@irevoire
Copy link
Member

irevoire commented Jun 3, 2021

bors cancel

@bors
Copy link
Contributor

bors bot commented Jun 3, 2021

Canceled.

@irevoire
Copy link
Member

irevoire commented Jun 3, 2021

good bors

Copy link
Member

@curquiza curquiza left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did not succeed to reproduce the panic
LGTM :)

@curquiza curquiza changed the title enable filter Enable filter paramater during search Jun 3, 2021
@MarinPostma
Copy link
Contributor Author

bors merge

@bors
Copy link
Contributor

bors bot commented Jun 14, 2021

@bors bors bot merged commit d765397 into main Jun 14, 2021
@bors bors bot deleted the filter branch June 14, 2021 08:34
@MarinPostma MarinPostma changed the title Enable filter paramater during search Enable filter parameter during search Jun 14, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

filter syntax filters and facetsFilters
3 participants