mattcasters commented on PR #8428:
URL: https://github.com/apache/hop/pull/8428#issuecomment-5713370328
Follow-up review of `fef644f9` (not the conversation summary). All twelve
items from the first review are addressed in HEAD.
## Fixed
1. **Search dialog** — grouped `@GuiWidgetElement` /
`GuiWidgetGroupType.TABS`, `GuiCompositeWidgets.addScrolledComposite`, filters
via `registerExtraGroup`. Stream-field combo fills both `Combo` and `ComboVar`.
2. **Upsert dialog** — same recipe; mappings as an extra group; OK uses
`getWidgetsContents`. Commit size, dimensions, top-k and min-score stay
resolvable strings.
3. **Search `supportsErrorHandling()`** — returns `true`.
4. **Upsert `supportsErrorHandling()`** — returns `true`.
5. **Failed batch commit** — JDBC batch is off whenever an error hop is
attached (`isDoingErrorHandling()`, not `isUseSafePoints()`). Failed batches
roll back; error-hop rows use savepoints on insert and delete.
6. **Commit size 0** — treated like Table Output: one transaction for the
run (`Integer.MAX_VALUE`), not JDBC autocommit. Schema DDL still runs with
autocommit on, then the data commit size is restored.
7. **Synthetic keys** — empty document ID is rejected; the key is
`documentId + "_" + parseChunkIndex(...)`. No more `doc_null`. Covered by unit
tests and IT `0003`.
8. **HNSW / mapped columns** — run independently of create-table. `CREATE
EXTENSION` is the only statement still tied to creating the table (`IF NOT
EXISTS` on index and `ALTER`).
9. **Multiple copies** — `check()` errors on delete-with-copies, and on
concurrent DDL for index/mappings as well as create-table.
10. **Eat-row option** — documented in the search Options table.
11. **Injection keys** — `PgVectorSearchMeta.Injection.FILTER` and
`PgVectorUpsertMeta.Injection.MAPPING` are in the English bundles.
12. **Cosine range** — enum and docs say `[-1, 1]`. `VectorDistanceMetric`
has no custom `toString()`, so the metric combos still round-trip through
`Enum.valueOf`.
The extra self-fixes in this commit also check out: no eviction from the
per-document delete cache, `check()` skipping `${...}` numerics, and schema DDL
outside the data transaction.
## Leftovers (copy only, not blocking)
- **HNSW tooltip** still says “Create an HNSW vector index when the table is
created”. After item 8 that checkbox runs against an existing table; the user
manual already says so. `PgVectorUpsert.createHnswIndex.Tooltip` is the line
operators will actually read.
- **ID-field tooltip** still says “When empty, document ID and chunk index
are used.” Runtime now requires a document ID (`resolveId`); a chunk index
alone is rejected. The adoc Options table was updated;
`PgVectorUpsert.idField.Tooltip` was not.
Happy to approve from here.
--
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]
