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

Fix duplicate records after match_all / match_any filter #508

Merged
merged 9 commits into from
Nov 8, 2018

Conversation

unnawut
Copy link
Contributor

@unnawut unnawut commented Nov 5, 2018

Closes #507

Overview

Fixes the issue where match_all / match_any filters return duplicate records.

Changes

  • Added distinct to match_all and match_any queries
  • Added tests to catch duplicate records
  • Aligned MatchAllParser code with the newer MatchAnyParser

Implementation Details

MatchAnyParser came after MatchAllParser and had proper support for multiple associations filtering. This PR, in addition to adding the distinct query, also aligns MatchAllParser with the newer MatchAnyParser so it gets a more proper advanced filtering code.

Usage

For match_all, Querying multiple associations that are associated with a single record should return only one distinct record. For example, the request params may look like this.

        "match_all" => [
          %{
            "field" => "tokens.symbol",
            "comparator" => "eq",
            "value" => token_1.symbol
          },
          %{
            "field" => "keys.access_key",
            "comparator" => "eq",
            "value" => key_1.access_key
          }
        ]

where token_1.symbol and key_1.access_key leads to the same record. It should return distinct records.


For match_any, Querying multiple associations that are associated with a single record should return only one distinct record. For example, the request params may look like this.

          %{
            "field" => "tokens.symbol",
            "comparator" => "eq",
            "value" => token_1.symbol
          },
          %{
            "field" => "tokens.symbol",
            "comparator" => "eq",
            "value" => token_2.symbol
          }

where token_1.symbol and token_2.symbol leads to the same record. It should return distinct records.

Impact

No changes to API or DB schema.

Copy link

@T-Dnzt T-Dnzt left a comment

Choose a reason for hiding this comment

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

Just one thing - some of the functions between MatchAllParser and MatchAny look identical, can't we extract those to a shared parser to avoid the duplicates?

@unnawut unnawut merged commit 8ec3fb8 into master Nov 8, 2018
@ghost ghost removed the s3/review 👀 label Nov 8, 2018
@unnawut unnawut deleted the 507-dups-in-filter branch November 8, 2018 09:30
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.

None yet

3 participants