bamaer commented on PR #8414: URL: https://github.com/apache/hop/pull/8414#issuecomment-5709430111
@mattcasters all 7 items addressed. Details below, including one finding that changes what bug 1 is. ### Bugs **1. Lazy String → Vector** — fixed. - `TYPE_STRING` source: now `parse(meta2.getString(data2))` (decode via the source's own storage metadata). - `TYPE_VECTOR` + `BINARY_STRING`: now `meta2.convertBinaryStringToNativeType(...)`, not `this.`. **Not reproducible from a pipeline.** `SelectValuesMeta:457` does not build a fresh Vector; it calls `ValueMetaFactory.cloneValueMeta(v, <Vector id>)`, and `cloneValueMeta` copies `storageMetadata` (`ValueMetaFactory:91-93`). The target therefore inherits the source's storage metadata and the old code worked. Same for a CSV field typed as Vector with lazy conversion: source and target are the same object. Conclusion: latent, not live. Fix kept because correctness should not depend on that inheritance, and the broad catch hid the NPE. **2. `cloneValueData`** — fixed with your patch. **Live**: `BaseTransform:1376` calls `rowMeta.cloneRow(row)` when copying rows to multiple targets, so a Copy hop on a lazy Vector field reaches it. ### Suggestions | # | Change | |---|---| | 3 | `writeData` converts first, null flag from `vector == null` | | 4 | `getValueFromResultSet` handles `float[]`, `double[]`, `String`, `java.sql.Array` (incl. `Number[]`) before `toString()` | | 5 | `catch (RuntimeException ignore)` → `catch (ClassCastException e)` with cause; NPE now propagates | | 6 | `typeCompare` uses `Arrays.compare`; javadoc now says lexicographic | | 7 | All four tests added | ### Verification Both fixes verified by reverting them and confirming the tests fail: - bug 1: `NullPointer Cannot invoke "IValueMeta.getType()" because "meta2" is null` - bug 2: `expected: <[91, 49, 44, 50, 44, 51, 93]> but was: <[1.0, 2.0, 3.0]>` 23 unit tests (was 19), 0 checkstyle violations, spotless + rat clean, three `0114` ITs passing. Also: fixed the value-types intro, which still listed a vector type as future work. New IT `0116-vector-lazy-conversion` covers a lazily converted Vector field from CSV Input; it does not cover the bug 1 path, per above. -- 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]
