drift icon indicating copy to clipboard operation
drift copied to clipboard

Please produce "comparators" for more complex queries.

Open RoarGronmo opened this issue 1 year ago • 3 comments

Hi !

In this post https://github.com/simolus3/drift/issues/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.

RoarGronmo avatar Nov 05 '24 10:11 RoarGronmo

(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.

Mike278 avatar Nov 05 '24 17:11 Mike278

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.

simolus3 avatar Nov 06 '24 00:11 simolus3

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.

RoarGronmo avatar Nov 06 '24 08:11 RoarGronmo