Add support for join relations in facet and numeric filters#260
Add support for join relations in facet and numeric filters#260OllieJennings wants to merge 7 commits intotypesense:masterfrom
Conversation
* Initial plan * Initial plan for adding join relations support Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Add join relations support to facet and numeric filters Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Remove temporary jest config and update gitignore Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Revert formatting change in FacetSearchResponseAdapter.js Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Remove jest.unit.config.js from .gitignore Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Refactor to eliminate code duplication in join relation handling Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Fix FacetSearchResponseAdapter quotes and eliminate duplicate if statement Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Revert FacetSearchResponseAdapter to single quotes Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Build lib and dist files with join relations support Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for join relations in facet and numeric filters to prepare the adapter for Typesense v30's new join relations feature. The implementation enables filtering on related data using a special syntax like $product_prices(field).
- Introduces regex pattern matching for joined relation filter syntax
- Adds a helper method to build filter strings for both regular and joined relation filters
- Updates numeric and facet filter adapters to handle joined relation syntax
Reviewed Changes
Copilot reviewed 3 out of 8 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| test/SearchRequestAdpater.test.js | Adds comprehensive test coverage for joined relation filters in both numeric and facet filter scenarios |
| src/SearchRequestAdapter.js | Implements core logic for parsing and adapting joined relation filters with new regex pattern and helper method |
| lib/SearchRequestAdapter.js | Generated/compiled version of the source changes with updated variable scoping |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
Could you also add some integration tests under the testground folder? |
Yeah of course |
* Initial plan * Add integration tests for numeric refinement list, pagination/sorting, and range filters Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Fix linting issues in integration tests Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Add documentation for new integration tests Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Add comprehensive PR summary documentation Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Remove incorrectly placed test files and revert FacetSearchResponseAdapter Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Add integration tests for joins functionality in testground Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Rename products_with_prices collection to products Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com>
|
@tharropoulos Updated |
…e filter handling (#3)
…perator (#4) * Initial plan * Initial plan: Refactor ternary operator to check isExcluded first Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Refactor: Check isExcluded first in _buildFacetFilterString ternary operator Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Build: Update dist files after refactoring Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Revert: Undo quote style change in FacetSearchResponseAdapter Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Add DOM tests for joins page Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Revert: Fix quote style in src/FacetSearchResponseAdapter.js Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com>
* fix: configure babel to preserve esm modules instead of converting to cjs • set modules: false in @babel/preset-env to maintain esm syntax • prevents commonjs interop issues when importing from esm contexts • resolves publint warning about __esModule and exports.default pattern * fix(test): let jest still interpret files as cjs --------- Co-authored-by: Fanis Tharropoulos <ftharropoulos@gmail.com> Co-authored-by: Jason Bosco <jason@typesense.org>
|
@tharropoulos Can you look again? Can confirm this was tested and all running fine against my local, and QA branch |
|
@jasonbosco lgtm |
* Initial plan * Initial exploration and planning Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Implement join filter grouping by collection Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Revert lib/Configuration.js and lib/support/utils.js to use CommonJS format Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Move join filter grouping to _adaptFilters for cross-filter grouping Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Revert quote style change in src/FacetSearchResponseAdapter.js Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Fix test expectations for _adaptFacetFilters to expect ungrouped output Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Regenerate all lib files from source Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> * Fully regenerate lib files with ESM format Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: OllieJennings <1575448+OllieJennings@users.noreply.github.com>
|
|
||
| filters.forEach((filter) => { | ||
| // Match pattern like "$collection(field:=value)" or "$collection(field:>=value)" | ||
| const joinMatch = filter.match(/^(\$[^(]+)\((.*)\)$/); |
There was a problem hiding this comment.
This regex syntax is starting to get super complicated. It should have a dedicated test file for it. I'd advise starting to just parse things out manually, token by token.
There was a problem hiding this comment.
Yep i feel you, its getting a bit complex, let me do that
|
This PR is blocking testing of Typesense v30 RC joins feature for us. Any timeline for merging? Ideally would like to test before the GA release of v30 to get everything up and running in time. |
|
If @OllieJennings doesn't have the time, I'll take this refactoring on myself on Monday |
@tharropoulos apologies for not getting back sooner on this but yes, recently lost a lot of free time with work commitments and start of the new year, so apologies for not getting on to this, any help would be appreciated |
Change Summary
Add support for join relations in the facet filters (standard & numeric).
Join relations are a new feature being introduced by typesense v30, this PR gets the adapter ready to support this new feature.
PR Checklist