Carry DCB tags through a bulk event import in hstore mode

[erdtsieck]

The problem

BulkInsertEventsAsync writes events with a single COPY ... mt_events(<fixed column list>) FROM STDIN BINARY, and the DCB tags are not in that list — nor does the bulk path write anything to the per-type mt_event_tag_* tables. So every event that arrives through a bulk import lands untagged.

That is documented today in docs/events/bulk-appending.md as a known trade-off ("No event tags"), so this is a limitation being lifted rather than a regression. The reason it seemed worth lifting: for a DCB user it fails in the quiet direction. An untagged event is not an error to QueryByTagsAsync or to a boundary aggregate — it is simply absent from the answer. A store whose history was loaded by a migration then gets a consistency boundary that silently excludes most of its events, and nothing points at the import.

The change

In DcbStorageMode.HStore the tags live in a column on mt_events itself, so they can travel in the same COPY:

• tags is appended to buildEventColumns() — last, after the optional metadata columns, so those keep the positions they had;
• writeEventRow writes the value from EventTagOperations.BuildHstore, the same rule the append path applies. That is why BuildHstore is internal now rather than private: a second implementation of the rule would be a second thing to keep in step;
• IEvent.Tags is null rather than empty when nothing was tagged, so the write is guarded on that and emits NULL.

No extra round trip and no second pass — one more column and one more write per row.

DcbStorageMode.TagTables is deliberately untouched. That would need a second COPY per registered tag type keyed on the seq_id, which interacts with the sequence blocks the events COPY draws from, and it felt like a separate change. The docs now state which mode carries tags through a bulk import and which does not.

Tests

hstore_dcb_tags_survive_a_bulk_import:

• a tagged event survives a bulk import and is found by QueryByTagsAsync on either registered tag type;
• an untagged event in the same import is not found by the tag query, and both events do land.

That second test earned itself immediately: it is what caught the null Tags, and it guards something worse than a lost tag — a COPY row that skips a column instead of writing null goes out of step with its column list, which shifts every later column.

Verified against the existing suites as well: 156 passed / 0 failed across BulkEventAppendTests, BulkEventStreamAppendTests and every Dcb test (net10.0).

Process note

CONTRIBUTING.md asks to discuss first on Discord or in an issue. I went straight to a PR because the change is small, additive and behind an existing mode flag — happy to move the discussion to an issue if you would rather triage it that way, or to close this if you would prefer the TagTables half solved in the same go.

[jeremydmiller]

Merged to master — your two commits cherry-picked with authorship intact (7d13c67, 2bceb30), plus 6591033 for the review follow-ups. Closing since GitHub will not do it for a cherry-picked merge.

Taking it as-is on the substance. Appending tags last in buildEventColumns() preserves the optional metadata column positions, and the null-vs-skip guard in writeEventRow is the right instinct — a COPY row that skips a column instead of writing null shifts every later one. Your second test earned its keep exactly as you said it would.

Verified red-then-green rather than taken on trust: with BulkEventAppender.cs and EventTagOperations.cs reverted to master, all three tests in the file fail; with them, all three pass. Full EventSourcingTests 1943/0, TenantPartitionedEventsTests 243/0.

What changed on top

The XML doc. Two stacked <summary> blocks on BuildHstore; the second is now <remarks>.

#5265 landed while this was open, and the two meet in BuildHstore. An event carrying two tags of one type is now refused in HStore mode rather than silently keeping one — an hstore maps one key to one value and the key is the tag type. Because you deliberately shared that method rather than reimplementing the rule, the bulk path inherited the refusal for free. That is the right outcome, and it now has a test instead of being an undocumented consequence of a merge. Your instinct there — "a second implementation of this rule would be a second thing to keep in step" — is what made it fall out correctly.

The DCB version bump. I raised this in review and settled on documenting rather than implementing. The bulk path does not bump mt_dcb_tag_version, which is the row a concurrent FetchForWritingByTags takes its consistency check against, so a bulk import running against a live store can commit tagged events without an in-flight boundary check noticing. That is consistent with an API that creates new streams, has no optimistic concurrency and is documented for loading a history — but given your whole argument is that a silently untagged event is the failure a consistency boundary must not have, it deserved to be stated rather than left implicit. Both limits are now in docs/events/bulk-appending.md.

DcbStorageMode.TagTables remains untouched, as you scoped it. The docs say which mode carries tags through an import and which does not.

Thanks for going straight to a PR on this one — it was small, additive, behind an existing mode flag, and the reasoning in the description made it quick to review.

[jeremydmiller]

Landed at 6591033.