DataObjects-NET / dataobjects-net

https://dataobjects.net
MIT License
61 stars 25 forks source link

A bunch of small improvements related to query translation #392

Closed alex-kulakov closed 4 months ago

SergeiPavlov commented 4 months ago

Please consider following Clone() optimization: https://github.com/servicetitan/dataobjects-net/pull/237/files

Return type covariance allows to avoid type casting in many cases.

alex-kulakov commented 4 months ago

This PR is for 7.0. Do you want me to copy these changes to 7.0 or just merge it to master?

SergeiPavlov commented 4 months ago

This PR is for 7.0. Do you want me to copy these changes to 7.0 or just merge it to master?

I thought you are going to apply the PR's changes to master branch too.

While I see now the 7.0 & master branches diverged and there will be many conflicts.

alex-kulakov commented 4 months ago

Well, everything which is done in 6.0/7.0/7.1 happens to appear in master some time because of merges. We need this to propagate at least bug fixes from where they were originated (if version is still supported) to the newest versions.

We have some enterprise customers (or customers of our customers) that still use 6.0 and if there is some issue which is very important for them, I'm expected to address it in 6.0.

This PR contains no breaking changes of publicly available APIs, I didn't make drastic changes deliberately, even if they are performance beneficial. If your PR has some of such changes then I'd prefer to merge it to master.

SergeiPavlov commented 4 months ago

If your PR has some of such changes then I'd prefer to merge it to master.

Changes of .Clone() in my PR are not 100% compatible For example var s = (SqlSelect) select.Clone(); in Extensions' code should be converted into (SqlSelect)((ICloneable)select).Clone();

On the other hand, that usage is rare and trivial to convert

alex-kulakov commented 4 months ago

It doesn't matter. Target runtime has no support for covariant returns. I will experiment with your suggestion in the version where it is supported by target runtime.