querybuilder icon indicating copy to clipboard operation
querybuilder copied to clipboard

Query assignments with side effects

Open brgrz opened this issue 3 years ago • 6 comments

I only just discovered that calling most methods on the existing Query modifies/mutates the query instance instead of returning new instance. So even if no left-side assignment is made the query is modified.

For instance

var qb = query.Query;
var countQuery = qb.AsCount();

both qb and countQuery are the same after applying qb.AsCount().

I am trying to manipulate queries based on certain conditions and would like to keep the original query for later.

I found Clone() method, is that the only way to go?

brgrz avatar Aug 15 '20 08:08 brgrz

Yes this is by design, as you have mentioned .Clone() is what you want here.

ahmad-moussawi avatar Aug 15 '20 13:08 ahmad-moussawi

What about immutable query, would you consider supporting that?

brgrz avatar Aug 17 '20 11:08 brgrz

There is no plan for this, unless there is a valid reason to do this, check this thread #176 anyway, we are open to discussing more this.

ahmad-moussawi avatar Aug 17 '20 15:08 ahmad-moussawi

As the other guy pointed out EF is doing it in immutable way and the methods in SqlKata even though they are not extension methods they kinda look like extension methods and for those one typically assumes they return new instances.

I'm not sure whether memory consumption would go through the roof (I could be wrong though).

The way I found out about this is when I ran into issues when I wasn't doing the assignment and the original query mutated anyway.

I don't particularly mind the Clone() but it does introduce certain noise imo.

brgrz avatar Aug 17 '20 17:08 brgrz

I would also vote for an immutable pattern. I'm not sure why you say that memory consumption would go up, since on each assignment the old object would fall out of scope (and eventually be garbage collected).

asherber avatar Jun 23 '21 00:06 asherber

This remains open and I'd hope plans would be revealed if this is planned for any version or maybe the next major version?

I'd imagine going from mutated queries to immutable ones is a breaking change, isn't it?

brgrz avatar Jan 05 '22 21:01 brgrz

Making the query immutable requires major rewriting, and honestly I don't see the clear benefit for it, the query.Clone() provide a handy way to do that, when needed.|

so the example in question can be rewritten as

var qb = query.Query();
var countQuery = qb.Clone().AsCount();

So I will close this one, since there is no plan for it at the time being.

ahmad-moussawi avatar Oct 02 '22 09:10 ahmad-moussawi