fix(rule_runtime): avoid rewriting select clause sources (by @keynslug)

Thanks to @keynslug for the insight and the patch.

With this, we avoid rewriting the parsed select clause sources, and
attempt to reuse the result context when reading values during
evaluation.

apps/emqx_rule_engine/src/emqx_rule_maps.erl

@@ -129,15 +129,6 @@ general_find({index, _}, List, _OrgData, Handler) when not is_list(List) ->
 
 do_put({key, Key}, Val, Map, _OrgData) when is_map(Map) ->
     maps:put(Key, Val, Map);
-do_put({key, Key}, Val, Data, _OrgData) when is_binary(Data) ->
-    case emqx_utils_json:safe_decode(Data, [return_maps]) of
-        {ok, Map = #{}} ->
-            %% Avoid losing other keys when the data is an encoded map...
-            Map#{Key => Val};
-        _ ->
-            %% Fallback to the general case otherwise.
-            #{Key => Val}
-    end;
 do_put({key, Key}, Val, Data, _OrgData) when not is_map(Data) ->
     #{Key => Val};
 do_put({index, {const, Index}}, Val, List, _OrgData) ->

apps/emqx_rule_engine/src/emqx_rule_runtime.erl

@@ -23,7 +23,6 @@
 -export([
     apply_rule/3,
     apply_rules/3,
-    clear_rule_payload/0,
     inc_action_metrics/2
 ]).
 
@@ -196,18 +195,18 @@ select_and_transform([], _Columns, Action) ->
 select_and_transform(['*' | More], Columns, Action) ->
     select_and_transform(More, Columns, maps:merge(Action, Columns));
 select_and_transform([{as, Field, Alias} | More], Columns, Action) ->
-    Val = eval(Field, Columns),
+    Val = eval(Field, [Action, Columns]),
     select_and_transform(
         More,
-        nested_put(Alias, Val, Columns),
+        Columns,
         nested_put(Alias, Val, Action)
     );
 select_and_transform([Field | More], Columns, Action) ->
-    Val = eval(Field, Columns),
+    Val = eval(Field, [Action, Columns]),
     Key = alias(Field, Columns),
     select_and_transform(
         More,
-        nested_put(Key, Val, Columns),
+        Columns,
         nested_put(Key, Val, Action)
     ).
 
@@ -217,25 +216,25 @@ select_and_collect(Fields, Columns) ->
     select_and_collect(Fields, Columns, {#{}, {'item', []}}).
 
 select_and_collect([{as, Field, {_, A} = Alias}], Columns, {Action, _}) ->
-    Val = eval(Field, Columns),
+    Val = eval(Field, [Action, Columns]),
     {nested_put(Alias, Val, Action), {A, ensure_list(Val)}};
 select_and_collect([{as, Field, Alias} | More], Columns, {Action, LastKV}) ->
-    Val = eval(Field, Columns),
+    Val = eval(Field, [Action, Columns]),
     select_and_collect(
         More,
         nested_put(Alias, Val, Columns),
         {nested_put(Alias, Val, Action), LastKV}
     );
 select_and_collect([Field], Columns, {Action, _}) ->
-    Val = eval(Field, Columns),
+    Val = eval(Field, [Action, Columns]),
     Key = alias(Field, Columns),
     {nested_put(Key, Val, Action), {'item', ensure_list(Val)}};
 select_and_collect([Field | More], Columns, {Action, LastKV}) ->
-    Val = eval(Field, Columns),
+    Val = eval(Field, [Action, Columns]),
     Key = alias(Field, Columns),
     select_and_collect(
         More,
-        nested_put(Key, Val, Columns),
+        Columns,
         {nested_put(Key, Val, Action), LastKV}
     ).
 
@@ -368,6 +367,16 @@ do_handle_action(RuleId, #{mod := Mod, func := Func, args := Args}, Selected, En
     inc_action_metrics(RuleId, Result),
     Result.
 
+eval({Op, _} = Exp, Context) when is_list(Context) andalso (Op == path orelse Op == var) ->
+    case Context of
+        [Columns] ->
+            eval(Exp, Columns);
+        [Columns | Rest] ->
+            case eval(Exp, Columns) of
+                undefined -> eval(Exp, Rest);
+                Val -> Val
+            end
+    end;
 eval({path, [{key, <<"payload">>} | Path]}, #{payload := Payload}) ->
     nested_get({path, Path}, may_decode_payload(Payload));
 eval({path, [{key, <<"payload">>} | Path]}, #{<<"payload">> := Payload}) ->

apps/emqx_rule_engine/test/emqx_rule_maps_SUITE.erl

@@ -73,8 +73,16 @@ t_nested_put_map(_) ->
         nested_put(?path([k, t, <<"a">>]), v1, #{k => #{<<"t">> => v0}})
     ),
     %% since we currently support passing a binary-encoded json as input...
+    %% will always decode as json, even if non-intentional...
+
+    %% note: since we handle json-encoded binaries when evaluating the
+    %% rule rather than baking the decoding in `nested_put`, we test
+    %% this corner case that _would_ otherwise lose data to
+    %% demonstrate this behavior.
+
+    %% loses `b' !
     ?assertEqual(
-        #{payload => #{<<"a">> => v1, <<"b">> => <<"v2">>}},
+        #{payload => #{<<"a">> => v1}},
         nested_put(
             ?path([payload, <<"a">>]),
             v1,
@@ -89,6 +97,23 @@ t_nested_put_map(_) ->
             #{payload => emqx_utils_json:encode(#{a => #{old => <<"v2">>}})}
         )
     ),
+    %% loses `b' !
+    ?assertEqual(
+        #{payload => #{<<"a">> => #{<<"old">> => <<"{}">>, <<"new">> => v1}}},
+        nested_put(
+            ?path([payload, <<"a">>, <<"new">>]),
+            v1,
+            #{payload => emqx_utils_json:encode(#{a => #{old => <<"{}">>}, b => <<"{}">>})}
+        )
+    ),
+    ?assertEqual(
+        #{payload => #{<<"a">> => #{<<"new">> => v1}}},
+        nested_put(
+            ?path([payload, <<"a">>, <<"new">>]),
+            v1,
+            #{payload => <<"{}">>}
+        )
+    ),
     ok.
 
 t_nested_put_index(_) ->