Add InsertWhenNotMatched extension for constant value

[dtila]

No description provided.

[viceroypenguin]

Would it be possible to add Update methods as well, so as not to limit to Insert branch of Merge()?

Also, please consider what the API should look like if one wanted to do InsertWhenNotMatchedAnd() with setting multiple columns.

[viceroypenguin]

Suggested change

	this IMergeableSource<TTarget, TSource> merge,
	[InstantHandle] Expression<Func<TTarget, TV>> extract,
	[SkipIfConstant] TV value)
	this IMergeableSource<TTarget, TSource> merge,
	[InstantHandle] Expression<Func<TTarget, TV>> extract,
	[SkipIfConstant] TV value)

[viceroypenguin]

Suggested change

	((ITable<IReviewIndex>)db.GetTable<ReviewIndex>())
	db.GetTable<ReviewIndex>()

[viceroypenguin]

I'd like to see some validation that the data actually made it into the database; see the other tests in this file for the types of validation that should be done.

[viceroypenguin]

Can you add more than one InsertWhenNotMatched() to validate that more than one field can be set?

[jods4]

Would it be possible to have an API similar to the one already existing for standalone insert/update?

src
  .Merge()
  .On(...)
  .Set(x => x.Field, 1)
  .Set(x => x.Field, y => y.OtherField)
  .InsertWhenNotMatched()
  .Set(x => x.Field, 2)
  .UpdateWhenMatched()
  .Merge();

[viceroypenguin]

Would it be possible to have an API similar to the one already existing for standalone insert/update?

src
  .Merge()
  .On(...)
  .Set(x => x.Field, 1)
  .Set(x => x.Field, y => y.OtherField)
  .InsertWhenNotMatched()
  .Set(x => x.Field, 2)
  .UpdateWhenMatched()
  .Merge();

I think I'd probably want to see the operation first and the set's after:

src
  .Merge().Using()
  .On()
  .InsertWhenNotMatchedAnd()
    .Set()
    .Set()
  .InsertWhenNotMatched()
    .Set()
    .Set()
  .UpdateWhenMatched()
    .Set()
  .Merge();

[jods4]

Doesn't that feels backward compared to existing insert, update apis that have the action as the final call?

In any case I prefer any option (before/after) that looks similar to existing .Set() rather than adding a new way to do the same thing.

[viceroypenguin]

Oh, right. I forgot that the existing methods have the action as the last one. It's backwards to my natural thinking, but I agree with keeping the behavior of existing APIs.

[dtila]

Hi guys, to be consistent with the EF implementation, I would see the API like this

src
  .Merge()
  .On(...)
  .Set(x => x.Field, 1)
  .Set(x => x.Field, y => y.OtherField)
  .InsertWhenNotMatched(entity => entity
           .Set(x.Field, 2)
           .Set(x => x.Field2, 3))
  .UpdateWhenMatched()
  .Merge();

Also this would allow using the existing Set.

Let me know and let's agree to an API

[jods4]

Generally speaking I'd say internal consistency with other existing linq2db API is more important than consistency with EF.

Let's go into the details, the existing signature of InsertWhenNotMatched is not quite right: it takes a lambda src => target, with is ok for src => new Target { X = 1 } but not for src => ??.Set(??.X, 1).

We can introduce an overload (src, target) => target for that purpose but I find it confusing with the existing UpdateWhenMatched((src, target) => target). If you're used to linq2db API, the target parameter for an insert makes no sense. Also for UpdatewhenMatched there would be no new overload and it would also be confusing what the API expects as an expression (2 different things possible in a single method, whereas InsertWhenNotMatched has 2 different overloads).

To lead users into the right direction, I think we would need a new type for the Set calls:

interface ISettable
{
  Set<V>(V property, V value)
}

// We then have 2 new overloads for dynamic setters:
void InsertWhenNotMatched(Action<ISettable, Src, Target>);
void UpdateWhenMatched(Action<ISettable, Src, Target>);

// Which could be used like so:
src
  .Merge()
  .On(...)
  .InsertWhenNotMatched((setter, src, target) => setter
    .Set(target.Field1, 2)
    .Set(target.Field2, src.Delta)
  )
  .UpdateWhenMatched((setter, src, target) => setter
    .Set(x.Field1, 0)
    .Set(x.Field2, target.Field1 - src.Delta)
  )
  .Merge()    

Personnally, I don't like this proposal much:

• The introduction of ISettable is drifting away from the existing API that linq2db uses for simple inserts/updates.
• For insert, you can't reference target in values, whereas for update you can, which is a bit inconsistent and an easy mistake.
• But the main drawback of EF-style API, in my opinion, is that it doesn't lend itself to dynamic programming. Let me explain:

One nice thing with linq2db .Set() is that it's easy to set some fields dynamically in your code, based on some condition or composable function. For example:

// Composable logic
public static IUpdatable<T> SetLastChange(this IUpdatable<T> src) 
  where T: ILastChange
{
  return src
    .Set(x => x.ChangedBy, currentUser)
    .Set(x => x.ChangedOn, DateTime.Now);
}

var query = src
  .Where(x => x.Id == 3)
  .Set(x => x.Name, "John") // Static setters
  .SetLastChange();  // Composition
  
// Dynamic setters, SET is conditionally generated
if (freezeData)
  query = query.Set(x => x.Locked, true);
  
query.Update();

And that kind of programming is much harder when all your setters must be inside an Expression<Func<Src, Target>>. Building dynamic expressions is notoriously complicated.

So for those reasons, personally I would stick to linq2db style:

// Which could be used like so:
src
  .Merge()
  .On(...)
    .Set(x => x.Field1, 2)
    .Set(x => x.Field2, src => src.Delta)
  .InsertWhenNotMatched()
    .Set(x => x.Field1, 0)
    .Set(x => x.Field2, (src, target) => target.Field1 - src.Delta)
  .UpdateWhenMatched()
  .Merge()

[dtila]

Being consistent, indeed is sometimes more important than adding a new behavior. In EF, indeed you are unable to use dynamic programming style. indeed make more sense.

Having these, I think it makes more sense to add new Set functions on the IMergeable interface.

Unfortunatelly I have a local setup that prevents me from running tests, so I am afraid I will be unable to check regressions. #4241

[igor-tkachev]

Lets add new set of Set methods, but keep linq2db style.

[Shane32]

Would it be possible to have an API similar to the one already existing for standalone insert/update?

src
  .Merge()
  .On(...)
  .Set(x => x.Field, 1)
  .Set(x => x.Field, y => y.OtherField)
  .InsertWhenNotMatched()
  .Set(x => x.Field, 2)
  .UpdateWhenMatched()
  .Merge();

I think I'd probably want to see the operation first and the set's after:

src
  .Merge().Using()
  .On()
  .InsertWhenNotMatchedAnd()
    .Set()
    .Set()
  .InsertWhenNotMatched()
    .Set()
    .Set()
  .UpdateWhenMatched()
    .Set()
  .Merge();

I think this is important to discuss a little more here. A long time ago, my dad had a HP calculator that used reverse polish notation. You type "3 enter 4 +" to find out what 3+4 is. It's really hard to follow unless you're used to it. In SQL we have a bit of the same, where SQL starts by selecting fields, but later comes what we are selecting from (although granted it's easy to read in english). Within LINQ, there is a very logical flow: you start by selecting the data table (FROM), and then you proceed with filter (WHERE), select, etc. Similarly with updates, in SQL it's backwards by stating what to modify before we specify what we are modifying.

In LINQ in general all commands follow this flow:

what -> filter/select/etc -> execute

This lets us programmatically build complex queries, and as a last step, execute them (ToList / Single / etc). Each command executes in order when order applies. This is true for linq2db set commands also.

what -> filter -> set commands -> execute

In my mind, Update() is just an indication to execute the programmed operation, similar to ExecuteNonQuery() is in ASP.NET. In the same vein, the linq2db merge command has a very similar flow:

what from/to & how -> operations -> execute

//what from
db.Table1
  //do
  .Merge()
  //what to
  .Using(db.Table2)
  //how
  .On(...)
  //operations
  .UpdateWhenMatched(...)
  .InsertWhenNotMatched(...)
  //execute
  .Merge()

I don't think there's anything about the current design that would lead users to believe that .Set should come before .UpdateWhenMatched. That would be like writing .Expression(x => x > 3).Where().

If there are conflicting thoughts here, I would remove ambiguity altogether, and simply add overloads to UpdateWhenMatched and InsertWhenNotMatched to build an operation programmatically.

.UpdateWhenMatched(x => x.Active, false)
.UpdateWhenMatched(x => x.Price, (dest, src) => dest.Price + src.Price)

Or use the builder syntax.

.UpdateWhenMatched(b => b
  .Set(x => x.Active, true)
  .Set(x => x.Price, (dest, src) => dest.Price + src.Price))

If those are not options, I would certainly put Set after UpdateWhenMatched, just like we write On after Merge and Using, and Set after From and Where.

src
  .Merge()
  .Using()
  .On()
  .InsertWhenNotMatched()
  .Set()
  .Set()
  .UpdateWhenMatched()
  .Set()
  .Merge();

The biggest reason I don't care for this is clarity, as formatting guidelines would have all the periods in a row. But I think it would be clearly wrong to have the Set before the InsertWhenNotMatched statement. There is simply no precedent for specifying what fields to update before specifying when they will be updated. For example, we cannot write db.Table1.Set(x => x.Price, 123).Update(x => x.Active == true).

Finally, there is an additional argument to support my suggestion: if Set comes before InsertWhenNotMatched / UpdateWhenMatched, there is no way to provide only the proper overloads for the method.

src
  .Merge()
  .Using()
  .On()
  .Set(x => x.Price, (dest, src) => dest.Price + src.Price)  // valid
  .UpdateWhenMatched()
  .Merge();

src
  .Merge()
  .Using()
  .On()
  .Set(x => x.Price, (dest, src) => dest.Price + src.Price)  // invalid - there isn't a dest
  .InsertWhenNotMatched()
  .Merge();

[Shane32]

I agree that mandating expression trees such as the below is not the best solution to allow for dynamic programming:

  .InsertWhenNotMatched((setter, src, target) => setter
    .Set(target.Field1, 2)
    .Set(target.Field2, src.Delta)
  )

It essentially leaves us almost right where we are now. I would use a typical delegate, like this:

  .InsertWhenNotMatched(b => b
    .Set(target => target.Field1, 2)
    .Set(target => target.Field2, (target, src) => src.Delta)
  )

Then it can be used like this:

  .InsertWhenNotMatched(b => {
    if (updatePrice) {
      b.Set(x => x.Price, newPrice);
    }
  })

We can explicitly state in the xml comments that the delegate will be executed immediately to build the operation (rather than when .Merge() executes), so users can write corresponding code for dynamic code solutions.

[viceroypenguin]

So to summarize, there are three different models we can use for this:

1. l2db Update model: .Merge().Using().On().Set(...).UpdateWhenMatched().Merge();
   • Named because this is the pattern used by .Update(); prepare, then operate
   • Advantage: Consistency with Update pattern
   • Advantage: Composable
   • Disadvantage: Cannot know whether .Set() is valid or not until .Insert() or .Update() is called; that is, no compile-time enforce of correct usage
   • Disadvantage: Logically out of order from operation (do something with the operation before picking the operation)
2. l2db Merge model: .Merge().Using().On().UpdateWhenMatched().Set(...).Merge();
   • Named because this is the pattern used by .Merge(); operation, then configure
   • Advantage: Consistency with Merge pattern
   • Advantage: Composable
   • Advantage: Set() methods can be provided specifically for each operation with appropriate parrameters; that is, source only for insert operations, source and dest for update/delete operations
   • Advantage: allows compile-time prohibition of setting values both in operation and via .Set() (.Set().Set().UpdateWhenMatched((dst, src) => new {}) can't be prohibited at compile time, but .UpdateWhenMatched((dst, src) => new {}).Set().Set() can be)
   • Advantage: Reads logically from operation to configuration of operation
   • Disadvantage: Not consistent with Update pattern
3. MS Options model (delegate): .Merge().Using().On().UpdateWhenMatched(s => s.Set(...)).Merge();
   • Named because this is the pattern used to configure options in Microsoft.Extension.Options
   • Advantage: Composable
   • Advantage: Set() methods can be provided specifically for each operation with appropriate parrameters; that is, source only for insert operations, source and dest for update/delete operations
   • Advantage: Reads logically as configuration of an operation
   • Disadvantage: Inconsistent with any existing l2db pattern so far

My vote is for 2, because it is more logical and reads better, and is consistent with the rest of the Merge operation itself (start the Merge operation with .Merge(), then do .Using(), etc.).

/to: @Shane32
/to: @igor-tkachev

[igor-tkachev]

@viceroypenguin, should we use then UpdateWhenMatchedWith(For) instead of UpdateWhenMatched to indicate that the expression is not completed?

[Shane32]

I'd vote for 2 or 3 nearly evenly, but I would favor 3 because it removes ambiguity and provides 'natural' indentation.

I vote for 3: Due to "conflict" with existing operation of UpdateWhenMatched() (in that users might think that .UpdateWhenMatched().Set() is correct, when in fact they wanted .UpdateWhenMatchedWith().Set()), I vote for 3. Granted we would probably return an interface on the former to prevent .Set(), but I prefer overloads to additional methods, so when the user finds the right method, they can just thumb through all the overloads to find the one that suits them the best.

Option 2 would be my second choice.

[viceroypenguin]

@viceroypenguin, should we use then UpdateWhenMatchedWith(For) instead of UpdateWhenMatched to indicate that the expression is not completed?

Oh right, because UpdateWhenMatched() with no parameters exists to when the source and target are the same type to automatically build the expression.

Then yes, each operation would include a XxxWith() method that would return a type that can use the .Set() methods.

[jods4]

Indeed option 3. with a delegate doesn't have the composability / dynamic drawbacks of an expression.
It means every parameter of the builder is an expression, similar to the existing Set patterns we have today.

Merge is complicated so I have no objections to any of the three remaining contenders.

I think consistency is important and the most consistent api to me would be 1 (first choice).

The builder pattern has the merit of being the most straightforward to type and build; it's also very clear what merge case the Set applies to, so option 3 is my second choice.

Option 2 I like less because it's similar to existing apis but backwards. It also requires new method names to disambiguate what you want to do and I don't like neither InsertWithWhenNotMatchedAnd nor InsertWhenNotMatchedAndWith... Both And and With call for a continuation in code, but only one can be last in the name.

FWIW I don't see what invalid usage can't be validated at compile-time in 1?
When you start using Set(..) on a merge, we can return a specific interface or struct that only includes InsertWhenMatched() and not InsertWhenMatched(setter). So mixing styles is not possible.

Note
Mixing styles could be a feature if you merge set properties.
I actually want to add Set(x => new T { .. }) as new overloads to insert/update for convenience.
I have a PR for that but it is stashed away until the parser rewrite is complete.

Likewise once you call .Set(_, Func<Src, Target, Target>) the interface could be IMergeUpdate and forbid InsertWhenMatched() (only UpdateWhenMatched()).

[igor-tkachev]

Another "consistent" way - we use MS (3rd) option and implement the similar extensions for our Insert/Update methods.