Skip to content

Split the high-water assertions in Bug_4785 and cover the per-tenant delete path - #5178

Merged
jeremydmiller merged 1 commit into
masterfrom
test/4785-high-water-grammar-cleanup
Aug 4, 2026
Merged

Split the high-water assertions in Bug_4785 and cover the per-tenant delete path#5178
jeremydmiller merged 1 commit into
masterfrom
test/4785-high-water-grammar-cleanup

Conversation

@jeremydmiller

Copy link
Copy Markdown
Member

Follow-up to #5176. That PR updated Bug_4785_delete_projection_progress_by_shard_name for #618ShardName no longer discards the tenant slot for HighWaterMark names — but left it in a state worth tidying. Test-only.

The name was the actual defect

The test was still called high_water_mark_identity_is_the_literal_constant_and_is_deletable_by_it, while the entire content of the change was that the identity is not always the literal constant. The body and comment were updated; the name was left asserting the premise the change disproved. Anyone grepping for the contract months from now would have been misled by exactly the part of a test that is meant to be self-describing.

It was doing two unrelated jobs

They fail for different reasons and should not share a name, so they are now separate:

  • high_water_identity_grammar_agrees_between_shard_name_and_marten (no DB) — pins that JasperFx's ShardName composition and Marten's HighWaterShardIdentity produce the same string, for both the store-global and tenant-scoped shapes. This agreement matters more than it looks: the progression SQL is keyed by string equality on name, so drift — a different separator, a dropped slot, an added version prefix — silently desyncs the writers from the readers instead of failing loudly. HighWaterShardIdentity's own doc warns about this, and nothing pinned it until Switching schemas ad-hoc via Advanced.Options.DatabaseSchemaName #618 moved one side.
  • store_global_high_water_row_is_deletable_by_its_literal_identity — the original delete assertion, unchanged in substance.

The coverage gap the bump exposed

Per-tenant high-water rows became a first-class shape and never went through the delete path at all. New test: deleting_one_tenants_high_water_row_leaves_a_lookalike_tenant_alone.

The tenant ids are chosen, not arbitrary. acme_corp and acmeXcorp differ in exactly one character, and that character is _ — a LIKE single-character wildcard in PostgreSQL. If this delete ever stopped being where name = ? and became a pattern match, deleting one tenant's row would silently take the other's. The store-global row is seeded too, since it is a prefix of both and a prefix match would eat it as well.

I checked the test discriminates rather than passing vacuously:

select 'HighWaterMark:acmeXcorp' like 'HighWaterMark:acme_corp',  -- t
       'HighWaterMark:acmeXcorp' =    'HighWaterMark:acme_corp';  -- f

This is not a hypothetical failure mode for this table. #5171 is exactly this bug in the per-tenant progression read filter, which builds an unescaped name like '%:' || tenant. The delete path is correct today — DeleteProjectionProgress emits parameterized where name = ? — and this pins it so the two paths cannot converge on the wrong answer while #5171 is still open.

Verification

26/26 in the file on net9.0 (was 24; one test became three).

The related design question — whether the identity-grammar assertion belongs in Marten at all, given Polecat composes the same grammar independently — is filed separately as #5177 rather than decided here.

🤖 Generated with Claude Code

https://claude.ai/code/session_01VpDCvJcBDZerieJB4JEHde

…delete

Follow-up to #5176, which updated this test for #618 (ShardName no
longer discards the tenant slot for HighWaterMark names) but left it in a state
worth tidying.

The name was the actual defect: the test was still called
high_water_mark_identity_is_the_literal_constant_and_is_deletable_by_it, while
the whole content of the change was that the identity is NOT always the literal
constant. Anyone grepping for the contract would have been misled.

It was also doing two unrelated jobs that fail for different reasons -- a
JasperFx/Marten contract check on the identity grammar, and the #4785 delete
behaviour. Now split:

- high_water_identity_grammar_agrees_between_shard_name_and_marten (no DB) pins
  that ShardName composition and HighWaterShardIdentity produce the same string.
  Both sides have to agree exactly, because the progression SQL is keyed by
  string equality on name, so drift desyncs writers from readers silently
  rather than failing.
- store_global_high_water_row_is_deletable_by_its_literal_identity keeps the
  original delete assertion.

Plus the coverage gap the bump exposed: per-tenant high-water rows became a
first-class shape and never went through the delete path. The new test uses
tenant ids acme_corp and acmeXcorp, which differ in exactly one character, and
that character is '_' -- a LIKE single-character wildcard in PostgreSQL. If this
delete ever stopped being `where name = ?` and became a pattern match, deleting
one tenant's row would silently take the other's. Verified against PostgreSQL
that the LIKE form does over-match and the equality form does not, so the test
discriminates rather than passing vacuously.

That is not a hypothetical failure mode for this table: #5171 is exactly this
bug in the per-tenant progression READ filter, which builds an unescaped
`name like '%:' || tenant`. The delete path is correct today; this pins it so
the two paths cannot converge on the wrong answer.

Test-only. 26/26 in the file (net9.0).

Refs #4785, #5171

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VpDCvJcBDZerieJB4JEHde
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant