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

filterx/expr-function: simple functions should get unmarshalled values #528

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

bazsi
Copy link
Member

@bazsi bazsi commented Feb 25, 2025

Right now, arguments to simple functions are evaluated at the time of the call, but that means that FilterXMessageValue objects are encountered when evaluating them.

This is:

  1. inconvinient, as we have to consider both native objects and
    message values
  2. somewhat incorrect, as the unmarshalled version of the arguments are
    not cached in a filterx variable.
  3. if the function changes the copy, it will not be impacting the
    filterx variable, e.g. unset_empties($json) will be a noop, if $json
    is an LM_VT_JSON

I understand this may have a performance impact, but if it does, we can look into creating FilterXStrings with a borrowed string pointer.

Right now, arguments to simple functions are evaluated at the time of the
call, but that means that FilterXMessageValue objects are encountered when
evaluating them.

This is:
1) inconvinient, as we have to consider both native objects and
   message values
2) somewhat incorrect, as the unmarshalled version of the arguments are
   not cached in a filterx variable.
3) if the function changes the copy, it will not be impacting the
   filterx variable, e.g. unset_empties($json) will be a noop, if $json
   is an LM_VT_JSON

I understand this may have a performance impact, but if it does, we can
look into creating FilterXStrings with a borrowed string pointer.

Signed-off-by: Balazs Scheidler <[email protected]>
@bazsi bazsi force-pushed the filterx-simple-func-should-get-eval-typed-args branch from d0212e7 to 6972cf8 Compare February 25, 2025 16:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant