bamaer commented on PR #8913:
URL: https://github.com/apache/hop/pull/8913#issuecomment-6164852424
@mattcasters, thanks for picking this up. Compiling once is the right call,
keeping `;` on import fixes real corruption (the old `;`→`,` rewrite broke
string literals), and it's about 2.5× faster than Formula with the fast path
off.
I built it, ran the tests, and ran a generated .ktr through PDI 9.4, Hop
with OpenFormula, and the current Formula transform. Five things before merging:
1. **Test provenance:** most of the tests in the openformula module are
ports of Pentaho libformula's tests. 102 of the 104 have libformula's class
names and test vectors (compare `LeftFunctionTest` or `Prd5174Test`), and 74
still carry its `@author` tags (Cedric Pronzato, Thomas Morgner, …). libformula
is LGPL 2.1 up to 9.4 and BSL 1.1 since, so we can't ship these under an ASF
header. I'm sure that wasn't intended, and I found no copied code in
`src/main`; it's only the tests. Could you replace them with fresh ones (the
OpenFormula spec examples are a good source) and drop the clean-room paragraph
from `NOTICE`? NOTICE is only for required attributions.
2. **Build:** checkstyle rejects the `Assertions.*` star imports in 94
tests, and spotless reorders the new entry in `assemblies/plugins/pom.xml`.
Happy to push that to your branch.
3. **Dates around DST** (`BinaryExpr.java:90-115`, `DateUtil.java:52`): in
Europe/Brussels `[d]+1` for 2026-10-25 gives 2026-10-25 23:00, and 29→30 March
gives 0 days in an Integer field. PDI and Formula return 2026-10-26 and 1.
4. **Timestamp fields** arrive as text (`RowFormulaContext.java:144`):
`[ts]+1` gives 1970-01-01 and `[ts]<DATE(2026;1;1)` is always false.
`getDate()` should do it.
5. **Result types** (`OpenFormula.java:165`): the type is fixed on the first
row, so `IF([a]>0;[a];0)` into an Integer field stores a BigDecimal on later
rows and the next transform fails.
Since `KettleImport` now sends every PDI Formula step here, I also compared
results with PDI. A handful differ (null dates in `YEAR()`, `VALUE("1,234.5")`,
`2^3^2`, `NULL()`). Maybe keep the import on Formula in this PR and switch it
once those are sorted? I can share the comparison setup and the list.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
