vbhanuchander-lang opened a new pull request, #9027:
URL: https://github.com/apache/devlake/pull/9027
### Summary
The customize extractor cursor LEFT JOINs the raw table, so a domain row
whose `_raw_data_id` points at a raw record that has since been deleted comes
back with a NULL `data` column. That value fell through to the default branch
of the type switch:
```go
switch blob := row["data"].(type) {
case []byte: ...
case string: ...
default:
return nil // ends the whole scan
}
```
Nothing surfaced. The subtask reported success, and every row ordered after
the first orphan silently kept whatever custom field values it already had — so
a partially-populated table looks identical to a correctly-populated one. In
the reported case 1,382 orphaned rows out of 30,287 left roughly 28,000 rows
unpopulated.
This skips the row and carries on.
### Change
- `default` now increments a counter and `continue`s rather than returning.
- `extractCustomizedFields` returns how many rows it skipped for that
reason, and `ExtractCustomizedFields` logs a warning naming the table, the raw
table and the count. A summary rather than per-row logging, since the count can
run into the thousands.
- The scan now returns `rows.Err()`. An error raised while iterating the
cursor was previously discarded, which is the same class of silent partial
processing and would be invisible in exactly the same way.
Skipping is the right behaviour rather than failing: an orphaned
`_raw_data_id` means the raw record was cleaned up, which is a normal
consequence of raw-data retention, not a corrupt state. Failing the subtask
would make routine cleanup break every later sync.
### Does this close any open issues?
Closes #8945
### Screenshots
N/A — backend behaviour, no UI surface.
### Other Information
**Tests.** The plugin had no unit tests, so this adds four, using the
generated `dal` mocks to drive a cursor: orphans interleaved with valid rows
(the reported shape), all rows orphaned, no orphans, and a cancelled context.
I checked the main test actually catches the bug rather than just passing —
temporarily restoring the old `return nil` makes it fail with `"[]" should have
2 item(s), but has 0`, i.e. zero rows updated because the scan aborted at the
first orphan. With the fix, both valid rows are updated and the two orphans are
counted.
`go test ./plugins/customize/tasks/` passes, `gofmt` and `go vet` clean for
the changed files. The `plugins/customize/e2e` package fails locally for want
of `E2E_DB_URL`, which is unrelated and excluded from `scripts/unit-test-go.sh`.
**One thing I noticed but did not change.** The `[]byte` and `string`
branches are not symmetric: the `string` branch has the `issues` + `IsArray()`
handling that writes `IssueCustomArrayField` rows, and the `[]byte` branch does
not. Which branch a row takes depends on what the driver returns for the raw
`data` column, so that feature may behave differently across databases. That
felt like a separate question from this fix, and changing it could alter
behaviour for existing users, so I left it alone — happy to open a follow-up
issue if you would like it looked at.
--
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]
