[RFE / POC] Implement MiqExpression interpreter

[kbrock]

Overview

MiqExpression can run in both sql and ruby.

When it runs in ruby, it produces a string, regular expressions replaces the string a few times and then it is run through eval.

A. Seems like the regular expression gsub in _subst shouldn't be necessary
B. The regular expression swapping takes a lot of time.
C. We want to drop eval
D. We want to better understand the code / make it more maintainable.

@Fryguy goal: Lets walk the miq expression tree with a visitor instead of eval(generated_string).
@kbrock goal: Lets first generate a ruby string that doesn't require the subst{_find} to put values into it.

This proposal I consider my goal a stepping stone to get the interpreter.

Mid Goals

Removing code and and reducing processing:

• a single generated tree that does not need to be modified between each subst_match?
• only parse field/value once per node and determining action.
• fully understand/process intent of each operation once (at preprocess time?)

Steps

• ? convert preprocess_exp!(exp.dup) to preprocess_exp(exp)
• preprocess_exp! converts all operations to lowercase once (vs 15 operator.downcase)
• preprocess_exp! adds Field object for field and value into the tree (vs repeated Field.parse, col_details, is_field?)
  • drop col_details cache and get_cols_from_expression
  • may need to cache something for includes_for_sql
  • drop MiqExpression.get_col_info, MiqReport.get_col_info from the ui if possible.
• preprocess_exp! consume valid?.
• evaluate_atoms no longer modify expression. (This will become the interpreter)
• evaluate_atoms s/obj/rec/ in params for consistency

Aggressive changes

• Do we use find operator (can we drop)?
• pass rec to Condition.do_eval so value can be used directly instead of substituted
• _to_ruby "find" - generate correct code and drop Condition.subst_find(str, rec)
• _to_ruby generate correct code and drop Condition.subst(str, rec)
• preprocess_exp! convert hashes into s-expression structs

Making an Interpreter

• copy paste _to_ruby strings (without the "'s) into evaluate_atom_helper
• Profit

[miq-bot]

This issue has been automatically marked as stale because it has not been updated for at least 3 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation.

[miq-bot]

This issue has been automatically marked as stale because it has not been updated for at least 3 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation.

[miq-bot]

This issue has been automatically marked as stale because it has not been updated for at least 3 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

[kbrock]

If we can drop Condition#mode="tag_expr", then we can change Condition#subst to take in an MiqExpression#exp and we can drop all the gsub calls for temporary <find> / <value...> nodes. Possibly even transition some of this code over to MixExpression#to_ruby.

subst takes up a majority of our object creation and execution time (ignoring database lookups). Also a single generation of the ruby code will simplify the code enough to make an interpreter that much easier.

---

ok, "tag_expr" was transitioned to "tag_expr_v2" in 2008.
But at this time we had a bunch of policies in product/policy/ and they looked very different. Now, I'm pretty sure we no longer support script: and they all have condition:. So this is probably a 10 year old relic that can be put to rest.

commit 37762294131e0cc24e05adf13fa5bbc85fa19200
Author: Gregg Tanzillo
Date:   Wed Apr 2 17:08:04 2008 +0000

Changes were made

[Fryguy]

I also commented here (#22989 (comment)) with similar findings as you. I agree we can drop tag_expr.