formula optimization

[rmannibucau]

Idea is to try to bypass Apache POI for formula evaluation.
Since it can be a crazy huge task the philosophy is "can i optimize this formula, if yes do it", there is also a system property to disable/enable this feature until we are sure we don't need it anymore.

Microbenchmark

• JMH 1.37, average-time mode (us/op, lower is better).
• Java 21.0.12 Temurin, single thread, -Xmx2g.
• Each case measured with 1 warmup iteration (30 s) and 2 measurement iterations (30 s).
• Baseline (POI) run with -Dorg.apache.hop.pipeline.transforms.formula.fast.FastFormulaCompiler.enabled=false.
• Each row evaluates a full Formula.processRow() (argument resolution + evaluation + output)

The scenarios below use formulas that the fast evaluator supports (so a fast vs POI difference is observable).

Scenario	Formula	Fast (us/op)	POI (us/op)	Speedup x	Faster %
CONCAT1	"CREATE TABLE " & [tableName] & "(FILENAME VARCHAR2(255), ROWNUM NUMBER, " & [sqlContent] & " )" & IF([compress]="Y"," COMPRESS","")	0.497	8.439	17.0	94.1
CONCAT2	[heures]*60 + [minutes]	0.207	3.927	19.0	94.7
CONCAT3	"merge into " & [outputTable] & " INPUT_TABLE using " & [inputTable] & " STAGING_TABLE ON (" & [joinClause] & " ) WHEN NOT MATCHED THEN INSERT ( " & [inputCols] & " ) VALUES ( STAGING_TABLE." & [outputCols] & "),"	0.990	6.618	6.7	85.0
IF/OR	IF(OR([mycolumn]="Œ",[mycolumn]="1",[mycolumn]="3",[mycolumn]="4"),"1",IF([mycolumn]="5","0",[mycolumn]))	0.868	5.162	5.9	83.2
CONCAT5	[abcd] & [version] & [fghij] & [hhmmss] & "." & [ext]	0.507	4.226	8.3	88.0
CONCAT+IF	IF([somecolumn] <> "N"," AND WHAT = '1111111100000000'","") & IF([othercolumn] <> "N"," AND EVER != '1'","") & IF([abcdef] <> "null"," AND DUMMY IN (" & [ghij] & ")","")	0.591	6.420	10.9	90.8

---

Thank you for your contribution! Follow this checklist to help us incorporate your contribution quickly and easily:

• Run mvn clean install apache-rat:check to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.
• If you have a group of commits related to the same change, please squash your commits into one and force push your branch using git rebase -i.
• [-] Mention the appropriate issue in your description (for example: addresses #123), if applicable.

To make clear that you license your contribution under the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.

• I hereby declare this contribution to be licensed under the Apache License Version 2.0, January 2004
• In any other case, please file an Apache Individual Contributor License Agreement.

[fpapon]

Nice improvement! I will take a look and make some tests with some pipelines.

[hansva]

I am not a big fan of this idea, we could make it more clear that this is an excel tranform and slow.
But I would be more in favor of an other formula transform or using a different library and another transform.

We will end up having to create/implement strange behavior POI/Excel has to make it compatible with it.

[bamaer]

I agree with @hansva. Making this fully POI compliant is not realistic, let alone maintaining it and remaining compatible. This could be great as a separate high-performance subset of the formulas, but it needs to be very clear that this is a very different transform behind the scenes.

[rmannibucau]

In the absolute I agree with you but mean we migrate on the fly formula between excel and this new component in pipelines too cause you can't ask people to rewrite hundreds of pipelines IMHO.
A miration tool is ok but has a huge drawback compared to this PR: you can't revert it if something is not well covered at runtime/in prod.

Would it be ok to:

1. reverse the default (the global toggle)
2. get it out like that
3. wait a few release and if ok promote it as a component

?

Indeed it makes a 2 step thing but it is always the case for huge changes like that so sounds the most profitable for end users to me.

wdyt?

[bamaer]

Imho, we should at the very minimum have a way to let users choose which engine they want to use to process their formulas, either POI or the fast engine. There currently are a couple of supported functions, let's say that grows to ~20, how would users currently know which engine processes their formula?

We should also make sure that we have integration tests that prove that the POI and "fast" formula engines provide the same results, or have the differences in formula behavior documented.

Don't get me wrong, I appreciate the initiative and the effort, but as you say, this will be a huge change if done well, so we need to be cautious.

[rmannibucau]

Imho, we should at the very minimum have a way to let users choose which engine they want to use to process their formulas, either POI or the fast engine. There currently are a couple of supported functions, let's say that grows to ~20, how would users currently know which engine processes their formula?

this PR is designed to ensure the user doesn't care, formula component is not designed around poi but more excel, the PR implement formula it can in a fast path else falls back on poi so it is transparent (else it is a bug and the system property a workaround) so the minimum spirit of this PR is to not have to get this question.

We should also make sure that we have integration tests that prove that the POI and "fast" formula engines provide the same results, or have the differences in formula behavior documented.

it is in the PR -> https://github.com/rmannibucau/hop/blob/38a644a4e34d084524084589d5670340fcce713e/plugins/transforms/formula/src/test/java/org/apache/hop/pipeline/transforms/formula/FormulaFastPathParityTest.java , not sure what an integration test would bring there but coverage is there

so we need to be cautious

100% aligned and this is why there is a system property backdoor, the question is more are we cautious at the cost of not enabling existing user to rely on it and only enable new users (or costly migration/test/review - note that there is it mainly human and not tech) or just make it work OOTB.

I prefer the upgrade and it works for free option and put effort in the parser harnessing on my side.

[mattcasters]

Code Review of PR #8304: Formula Optimization

Great initiative! Bypassing Apache POI's HSSF/XSSF workbook and sheet allocation for straightforward expressions is a huge win for one of Hop's most heavily used transforms. The 10x–20x microbenchmark improvements demonstrate how much overhead POI introduces for row-by-row formula evaluation.

Below are a few critical correctness and runtime failure issues, Excel/POI semantic discrepancies, and performance suggestions to address before this can safely merge.

---

1. Critical Bugs & Runtime Failure Risks

1.1 ArrayIndexOutOfBoundsException on missing fields and bracketed string literals

In FastFormulaCompiler:

// FastFormulaCompiler.key():
for (String fieldName : fieldNames) {
  key.append('|').append(fieldName);
  key.append(':').append(rowMeta.getValueMeta(rowMeta.indexOfValue(fieldName)).getType());
}

// FastFormulaCompiler.eligibleTypes():
for (String fieldName : fieldNames) {
  int type = rowMeta.getValueMeta(rowMeta.indexOfValue(fieldName)).getType();
  if (!isFastType(type)) { return false; }
}

• Issue: If a formula references a field not present in rowMeta (e.g. typos, missing input columns), rowMeta.indexOfValue(fieldName) returns -1. Calling rowMeta.getValueMeta(-1) immediately throws ArrayIndexOutOfBoundsException: Index -1 out of bounds.
• String literal brackets: In Formula.java, fieldNames is extracted via FormulaFieldsExtractor.getFormulaFieldList(), which extracts any text inside [...] without string literal awareness. A formula like:

  IF([status] = "[ACTIVE]", 1, 0)
  

  extracts "ACTIVE" as a field name. If "ACTIVE" is not a stream field, initialization fails with ArrayIndexOutOfBoundsException: -1 rather than falling back to POI.
• Recommendation: Check int idx = rowMeta.indexOfValue(fieldName); if (idx < 0) return CompiledFormula.NOT_ELIGIBLE; before attempting getValueMeta(idx).

---

1.2 null in numeric fields crashes the pipeline with unhandled UnsupportedFormulaException

In FastFormulaEvaluator.toNumber():

if (value == null || value == FastFormulaCompiler.NA) {
  throw new UnsupportedFormulaException("Cannot use " + value + " as a number");
}

• Issue: In Excel and Apache POI, blank/null cells in arithmetic expressions ([qty] * [price] or [amount] + 10) are treated as 0 (null + 10 = 10, null * 5 = 0).
• In FastFormulaEvaluator, if an incoming row has a null value in a numeric field:
  1. toNumber(null) throws UnsupportedFormulaException.
  2. Because the formula was marked eligible (fastPath = true) during first row initialization, this exception occurs during row processing (eval(args)).
  3. Formula.processRow() catches Exception and diverts to error handling or aborts the pipeline. There is no runtime fallback to POI.
• Real-world ETL data frequently contains nulls (e.g., from outer joins).
• Recommendation: In toNumber(), when value == null and setNa is false, treat null as 0.0d to match Excel/POI arithmetic behavior.

---

1.3 FastFormulaCompiler.NA sentinel leaks into String concatenation as "java.lang.Object@..."

In FastFormulaCompiler:

public static final Object NA = new Object();

And in FastFormulaEvaluator.TextValue:

private static String of(Object value) {
  if (value == null) return "";
  if (value instanceof Boolean ...) ...
  if (value instanceof Number ...) ...
  return String.valueOf(value);
}

• Issue: When isSetNa() is enabled and a null field is encountered, args[i] is set to FastFormulaCompiler.NA. If that field is used in text concatenation ([prefix] & "-" & [comment]), TextValue.of(NA) falls through to String.valueOf(NA), resulting in:
  "prefix-java.lang.Object@4f023fd2"
• In Excel/POI, concatenating an #N/A error propagates #N/A (or produces an error cell). It should not output the Java object hash.

---

1.4 Unchecked exceptions in compileUncached escape unhandled

In FastFormulaCompiler.compileUncached():

try {
  root = FastFormulaEvaluator.parse(resolvedFormula, fieldIndex);
} catch (UnsupportedFormulaException e) {
  return CompiledFormula.NOT_ELIGIBLE;
}

• Issue: If the parser throws an unchecked exception such as NumberFormatException (e.g., malformed exponential notation 1e-), StringIndexOutOfBoundsException, or NullPointerException, it bypasses this catch block and crashes initialization instead of returning NOT_ELIGIBLE.
• Recommendation: Catch Exception (or RuntimeException) and return CompiledFormula.NOT_ELIGIBLE.

---

2. Parity & Semantic Divergences with Excel / Apache POI

1. compareEqual between Boolean and Number / String:

   if (left instanceof Boolean || right instanceof Boolean) {
     return toBoolean(left) == toBoolean(right);
   }

   • In Excel and POI, booleans and numbers are strictly distinct types: TRUE = 1 is FALSE, and FALSE = 0 is FALSE. In FastFormulaEvaluator, toBoolean(1) returns true, so TRUE = 1 evaluates to TRUE.
   • Additionally, if left is Boolean and right is "Y", toBoolean("Y") throws UnsupportedFormulaException at runtime instead of returning FALSE.
2. compareEqual between Number and String:
   • In Excel and POI, numbers and strings are never equal (200 = "200" is FALSE). In FastFormulaEvaluator, it falls through to TextValue.of(left).equalsIgnoreCase(TextValue.of(right)), which returns TRUE.
3. Non-standard logical operators (&&, ||, !):
   • Excel formulas use AND(...), OR(...), NOT(...). Excel does not support &&, ||, or ! as logical operators (! is the sheet reference separator).
   • Introducing && and || creates a dialect mismatch: formulas like [a] > 1 && [b] > 1 work on the fast path, but fail with FormulaParseException in POI if the fast path is disabled or if an unsupported function causes fallback.
4. IF with 2 arguments:
   • In Excel and POI, the false branch is optional: =IF([score] >= 60, "Pass") evaluates to FALSE when the condition is false. FastFormulaEvaluator rejects this with UnsupportedFormulaException("IF requires 3 arguments").
5. TRIM whitespace inconsistency:

   if (text.indexOf(' ') < 0) {
     return text.trim();
   }

   • If there are no spaces, String.trim() strips all whitespace (tabs, newlines, control characters). If a space exists, the custom loop only collapses/strips ASCII spaces (' '), leaving tabs and newlines intact.

---

3. Performance & Memory Optimizations

3.1 Per-row RowMeta.indexOfValue lookups and per-row Object[] allocations

In Formula.java:

private Object[] buildFastArguments(List<String> fieldList, Object[] sourceRow, boolean setNa) {
  Object[] args = new Object[fieldList.size()];
  for (int i = 0; i < fieldList.size(); i++) {
    int fieldIndex = data.outputRowMeta.indexOfValue(fieldList.get(i));
    Object value = fieldIndex < 0 ? null : sourceRow[fieldIndex];
    args[i] = (value == null && setNa) ? FastFormulaCompiler.NA : value;
  }
  return args;
}

• RowMeta.indexOfValue() acquires a ReentrantReadWriteLock.readLock() on every call. Calling it for every field of every formula on every single row incurs repeated locking and string comparisons.
• In addition, allocating a new Object[] args array per formula per row creates avoidable GC churn on large pipelines.
• Optimization: Pre-compute the field indices during first into an int[][] fastFieldIndices array:

  int fieldIndex = fastFieldIndices[i][j];
  Object value = sourceRow[fieldIndex];

3.2 POI resources allocated even when all formulas use the fast path

In Formula.java:

poi = IntStream.range(0, meta.getFormulas().size())
    .mapToObj(it -> new FormulaPoi(this::logDebug))
    .toArray(FormulaPoi[]::new);
formulaFieldLists = ...;

If every formula in the transform is eligible and compiled for the fast path, pre-allocating poi and extracting formulaFieldLists can be skipped.

---

4. Hop Conventions & Configuration

1. Hop Variables vs System Properties:
   • Configuration is currently controlled via JVM property -Dorg.apache.hop.pipeline.transforms.formula.fast.FastFormulaCompiler.enabled=false. In Hop, users configure options via hop-config.json, project settings, or pipeline execution configurations. Exposing this via Hop environment variables or IVariables would make it manageable in Hop GUI and hop-run.
   • Also, FastFormulaCompiler.enabled is cached at class loading (private static volatile boolean enabled = enabledFromProperty();), meaning subsequent calls to System.setProperty(...) have no effect unless FastFormulaCompiler.setEnabled() is called directly.
2. Redundant setNa in Cache Key:
   • In FastFormulaCompiler.key():

     key.append(setNa ? "na" : "plain").append('=').append(resolvedFormula);

     setNa is not used during AST compilation in compileUncached; it is only used at runtime in buildFastArguments(). Including setNa in the cache key leads to duplicate cached ASTs.