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

New Set() overloads for UPDATE #3798

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

New Set() overloads for UPDATE #3798

wants to merge 3 commits into from

Conversation

jods4
Copy link
Contributor

@jods4 jods4 commented Oct 1, 2022

Syntax t.Update(old => new T { A = 1, B = 2 }) is great but not always usable.
When one wants to add setters dynamically or set multiple fields with ROW (A, B) = ..., one needs to use the .Set() family of APIs.
These only allow defining setters one-by-one, so I'm adding some new overloads to make it more convenient to use.

Set a new initializer

Basically works exactly like Update above, but can be called multiple times amongst other .Set() calls.

var updated = table
  .Set(old => new () { A = 1, B = old.B + 1 })
  .Update();

// Equivalent with current apis:
var updated = table
  .Set(t => t.A, 1)
  .Set(t => t.B, old => old.B + 1)
  .Update();

Set a ROW with single query

Consider this:

table
  .Set(
    x => Sql.Row(x.A, x.B),
    old => query.Select(q => Sql.Row(1, q.C)).Single())
  .Update();

It works and it is conceptually equivalent to the generated SQL.
But that is not optimal C#:

  1. The fields names are far from their values. It's ok with 2-3 such as the example above, but it's not very clear with more.
  2. C# typing is strict for SqlRow<T1, T2>, no implicit conversions. Assigning int to int? has to be wrapped into AsNullable().
  3. Because the value must be a single SqlRow, one generally has to add .Single().
  4. Sql.Row is limited to 8 elements, it is impossible to update more fields. Larger tables are far from uncommon.

I'm removing all of those 4 pain points with the following new overload that accepts a IQueryable<T>, that should select an initializer like the basic Update() example above:

// Equivalent to previous example
table
  .Set(old => query.Select(q => new T { A = 1, B = q.C }))
  .Update();

Add suspiciously missing call to ProcessSourceQueryable
It's present in 3 out of 4 identical methods...

IUpdatable has a single implementation, that it's casted to everywhere.

Add Query to the interface and stop casting.

Eliminate redundant allocations

Fix stuff
@jods4
Copy link
Contributor Author

jods4 commented Oct 1, 2022

/azp run test-all

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@jods4
Copy link
Contributor Author

jods4 commented Oct 1, 2022

To follow-up on our recent discussion about guards and throw helpers, notice how frequent this kind of short methods are in this PR:

		public static IUpdatable<T> Set<T>(
			                this IUpdatable<T>     source,
			[InstantHandle] Expression<Func<T, T>> setters)
		{
			if (source  == null) throw new ArgumentNullException(nameof(source));
			if (setters == null) throw new ArgumentNullException(nameof(setters));

			return SetInitializer(source.Query, Methods.LinqToDB.Update.SetUpdatablePrev, setters);
		}

Using guard helpers would shorten the code and after inlining if() checks would simply be dropped by JIT.
Good for readability and perf.

		public static IUpdatable<T> Set<T>(
			                this IUpdatable<T>     source,
			[InstantHandle] Expression<Func<T, T>> setters)
		{
			Guard.NotNull(source);
			Guard.NotNull(setters);

			return SetInitializer(source.Query, Methods.LinqToDB.Update.SetUpdatablePrev, setters);
		}

@jods4
Copy link
Contributor Author

jods4 commented Oct 1, 2022

@sdanyliv can you please carefully review ParseQueryableSet?
https://github.com/linq2db/linq2db/pull/3798/files#diff-97c0aa238c0f79058aa077ab71ae11f34b9f9becb7d6ea1191116c4c4537326dR741-R761

In my tests it works, but I'm really not sure it's the proper, reliable way:

  1. I couldn't find a way to get the actual initializers (esp. if Select is not the last thing called in LINQ), so I just took out the column aliases names... reliable enough?
  2. I build the SqlField expressions by simply looking up the alias names in the EntityDescriptor of the updated type (found as the 1st parameter type).

For your reference, this method works from the lambda subquery, which is a LambdaExpression returning IQueryable<Updated>. It takes a single parameter that represents the "old" table values, like other setters.

@linq2dbot
Copy link

Test baselines changed by this PR. Don't forget to merge/close baselines PR after this pr merged/closed.

@sdanyliv
Copy link
Member

sdanyliv commented Oct 2, 2022

Actually it is not correct. I suggest to postpone this PR until version 5. Query translation will be much more simpler.

@MaceWindu MaceWindu added this to the 5.0.0 milestone Dec 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants