win7user10 / Laraue.EfCoreTriggers

Library to write triggers in C# with EF.Core
MIT License
112 stars 20 forks source link

Raw sql trigger action #16

Closed zirou5055 closed 2 years ago

zirou5055 commented 3 years ago

How about adding a raw SQL trigger action?

I recently had to create a trigger that generates a notification in postgresql. (e.g.: "PERFORM pg_notify('insert_item', 'payload');")

But I can't create a custom provider because ITriggerProvider doesn't have an interface for it.

I think plain SQL is easy, but SQL with arguments won't be that easy. I would be very happy if you could provide that feature.

win7user10 commented 3 years ago

Thank you for the suggestion. I will think about how to add this feature

fritz-net commented 2 years ago

@zirou5055 is your feature ( https://github.com/zirou5055/Laraue.EfCoreTriggers/commit/04b9b3a285285c3dd12a587f46a15dd778f1d88f ) complete? It looks way more complete than my attempt and also has tests

zirou5055 commented 2 years ago

@fritz-net Yes, I am applying it to my company's project. but i am not sure if that is enough to make it public.

fritz-net commented 2 years ago

@zirou5055 this is not mine to decide, maybe you could bring in a pull request for @win7user10 to decide? I would really love raw sql trigger in the nuget package. Currently I use my implementation as submodule which sometimes gives me problems with my CI

zirou5055 commented 2 years ago

@fritz-net My implementation has limitations (ex. not support sql with arguments). so i didn't request pull. I believe @win7user10 is considering providing an official version.

win7user10 commented 2 years ago

@zirou5055, please provide an excepted syntax from the feature. I can suggest the next:

.Action(action => action
  .ExecuteRawSql("PERFORM calculate_something({0}, {1})", (old, new) => old.Name, (old, new) => new.Name)

args will be replaced to passed expression.

The syntax without args:

.Action(action => action
  .ExecuteRawSql("PERFORM calculate_something()")

I will try to find some time to implement this feature

zirou5055 commented 2 years ago

@win7user10 I agree

win7user10 commented 2 years ago

@zirou5055, please, check a new functionality

zirou5055 commented 2 years ago

Your suggestion is good enough. But looking back at what I'm using, I think it would be nice if there was a way to pass the entire row as an argument.

Here is the example I'm using. Hope this helps. Expected result:

"CREATE FUNCTION LC_TRIGGER_AFTER_INSERT_NODEHISTORY()
    RETURNS trigger
AS $LC_TRIGGER_AFTER_INSERT_NODEHISTORY$
    BEGIN PERFORM pg_notify('node_history', row_to_json(NEW)::text);
    RETURN NEW;
    END;
$LC_TRIGGER_AFTER_INSERT_NODEHISTORY$
LANGUAGE plpgsql"

My usage: _(I needed to pass the whole NEW as an argument to the row_tojson.)

modelBuilder.Entity<NodeHistory>()
                .AfterInsert(trigger => trigger
                    .Action(action => action
                        .Raw(nm => nm.Notify("node_history"))));
public class RawSqlExtensionsNotifyConverter : MethodCallConverter
    {
        public override string MethodName => nameof(RawSqlExtensions.Notify);

        public override Type ReflectedType => typeof(RawSqlExtensions);

        public override SqlBuilder BuildSql(BaseExpressionProvider provider, MethodCallExpression expression, Dictionary<string, ArgumentType> argumentTypes)
        {
            var entityName = (expression.Arguments[0] as ParameterExpression)!.Name!;
            var argumentChannelExpression = expression.Arguments[1] as ConstantExpression;
            var argumentType = argumentTypes[entityName];
            var entityPrefix = argumentType switch
            {
                ArgumentType.New => "NEW",
                ArgumentType.Old => "OLD",
                _ => throw new Exception($"Not supported argument type {argumentType}")
            };

            return new SqlBuilder($"PERFORM pg_notify('{argumentChannelExpression!.Value}', row_to_json({entityPrefix})::text);");
        }
    }

New usage:

modelBuilder.Entity<NodeHistory>()
                .AfterInsert(trigger => trigger
                    .Action(action => action
                        .ExecuteRawSql("PERFORM pg_notify('node_history', row_to_json({0})::text)", (old, new) => new);
zirou5055 commented 2 years ago

@fritz-net what do you think? could you please leave any comments?

win7user10 commented 2 years ago

@zirou5055 I think in your case, the best way is to write

.ExecuteRawSql("PERFORM pg_notify('node_history', row_to_json(NEW)::text)"
zirou5055 commented 2 years ago

@win7user10 You are absolutely right. If so, I think your suggestion would suffice.

fritz-net commented 2 years ago

@zirou5055 I did not have time yet to test it out. However for my current usecase I just pass a string ( see https://github.com/win7user10/Laraue.EfCoreTriggers/issues/19#issuecomment-968887728 ) so the current solution is perfekt :)

Sure it would be nice to pass 2 lists ( affected columns & values ) so I could use reflection to make sure all properties will be included in the trigger (even when I add new ones). However I'm very new to expressions an so on so I can't make a suggestion on how to implement this. But as a workaround I could also generate this string myself an then pass it to the raw function.

@win7user10 thank you for your implementation. I hope I can try it out within the next days