Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Please produce "comparators" for more complex queries. #3326

Open
RoarGronmo opened this issue Nov 5, 2024 · 3 comments
Open

Please produce "comparators" for more complex queries. #3326

RoarGronmo opened this issue Nov 5, 2024 · 3 comments

Comments

@RoarGronmo
Copy link

RoarGronmo commented Nov 5, 2024

Hi !

In this post #3295 we have brought up the discussion on to use distinct as "standard" drift, which may be out of band, since distinct is bound to the stream.

(I think) the comparator in that distinct function uses the overrided == operator of the object provided by drift, but not for results that contains more complex joins or partly selections.

Example 1:

SELECT * FROM ords

will most likely produce a working comparator.

But
Example 2:

SELECT 
  o.id,
  o.name
FROM ords AS o

may not produce a comparator, and distinct may compare the object references in stead, which are different, even if the contents may be the same.

I suggest that you may enhance this so that comparators are also produced on selections like that one in Example 2 above, so they can be used by distinct.

@Mike278
Copy link
Contributor

Mike278 commented Nov 5, 2024

(I think) the comparator in that distinct function uses the overrided == operator of the object provided by drift

That's correct! Though FYI distinct also accepts an optional comparator to customize deduplication logic.

but not for results that contains more complex joins or partly selections.

That's the default behaviour, but you can ask drift to override == for those result objects using the override_hash_and_equals_in_result_sets generation option.

@simolus3
Copy link
Owner

simolus3 commented Nov 6, 2024

For custom queries with joins built at runtime, drift uses QueryRow and TypedResult, neither of which currently implement == or hashCode (so the generation option applies to compiled queries only) . That's probably something worth adding.

Applying distinct() as a default doesn't really make these queries any worse as they are at the moment - they would continue to potentially fire with equal results.
Ideally, distinct should be implemented at a pretty low level in the stack to avoid duplicate work. So e.g. instead of just calling .distinct() on streams that drift returns internally, it might be useful to skip the whole step of mapping database rows to Dart if the underlying rows didn't change.

@RoarGronmo
Copy link
Author

For the time being, I wasn't aware of the override_hash_and_equals_in_result_sets option, to generate the comparators of the ...Result sets, which "corrects" the check for theese sets.

As you say @simolus3 the best is to skip the whole step if the resulting row didn't change at all. But then there should be a compile option to enable it so you could do some extensive debugging.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants