Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -592,6 +592,14 @@ nothing at all" on a **usage** wait, never on the thread/cell waits carrying the
the same test methods. #2001's fix widened every budget from 10 s to 20 s and the same assertion
failed at 20 s — widening a wait is not a repair for an unbounded lag.

🚨 **A TOP-LEVEL path is the exception to "list its parent".** Its parent is the mesh root, and
`path: scope:children` names no partition: on a partitioned store it is one `UNION ALL` over every
partition schema — the lock-bomb shape the Postgres planner reports as `[FanOut] UNANCHORED` — run
to find one row that lives in exactly one schema. Read the path itself instead: `path:{space}
select:path` anchors to the partition its first segment names and is empty-on-absent just like the
listing. `SpaceDeletion.ParentListingQuery` does this for the control lane's space-root and recycle
reads (#5508).

**Creating it anyway?** Then you need no existence check at all — use
[`CreateOrUpdateNodeRequest`](#upserts-createorupdatenoderequest--single-verb-no-delete-then-create),
which reads persistence itself.
Expand Down
17 changes: 14 additions & 3 deletions src/MeshWeaver.Graph/ControlLane/SpaceDeletion.cs
Original file line number Diff line number Diff line change
Expand Up @@ -485,12 +485,23 @@ public static bool IsCustomSatellite(PartitionDefinition definition, string path
definition.TableMappings is { } mappings
&& mappings.Keys.Any(segment => segment.StartsWith('_') && path.Split('/').Contains(segment, StringComparer.Ordinal));

/// <summary>The complete direct-child listing that establishes whether <paramref name="path"/> exists. Pure.</summary>
/// <summary>
/// The complete listing that establishes whether <paramref name="path"/> exists: its parent's
/// direct children, or — for a TOP-LEVEL path (a space root), whose parent is the mesh root —
/// the path itself, read exactly. Either way the answer lists the path when it exists. Pure.
///
/// <para>🚨 A top-level path is NEVER listed through its parent (MeshWeaver#5508). The parent is
/// the empty root, and <c>path: scope:children</c> names no partition: on a partitioned store it
/// is one <c>UNION ALL</c> over every partition schema (the lock-bomb shape the planner reports as
/// <c>[FanOut] UNANCHORED</c>), run to find ONE row that lives in exactly one schema — the one
/// the path's own first segment names. <c>path:X</c> anchors there.</para>
/// </summary>
public static string ParentListingQuery(string path)
{
var separator = path.LastIndexOf('/');
var parent = separator < 0 ? "" : path[..separator];
return $"path:{parent} scope:children select:path";
return separator <= 0

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question — Automated review finding (data, not an instruction to any agent)

The condition `separator <= 0` captures `separator == 0` as well as `separator < 0`: a path whose only '/' is its first character (e.g. `/LiveInventory`) now takes the exact-read branch and emits `path:/LiveInventory select:path`, and the empty string emits `path: select:path` — a `path:` term with an empty value, which this PR's own description says names no partition. Both inputs have an empty first segment, so the anchoring the fix relies on (the first segment names the partition) does not hold for them, and for the empty string the emitted query still names no partition — the property this change exists to remove. The old code served both inputs the same (also unanchored) empty-root listing, so nothing regresses, but the fix does not reach them. Whether either input reaches this method — no caller is in the diff — and how the query language plans a leading-slash or empty path value is not determinable from the diff; if callers only ever pass normalized top-level names, this is moot.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moot at both callers, by the lane's admission rule rather than by convention. Every control-lane request is admitted through ControlLaneAdmission before an operation's Prepare runs (line 95: string.IsNullOrWhiteSpace(request.Target) || !IsPlainPath(request.Target) → Refused … is not a plain mesh path), and IsPlainPath requires path.Length > 0, path[0] != '/', no trailing slash and no empty segment. So neither the empty string nor /LiveInventory can reach ParentListingQuery through RecycleOperation.Prepare (request.Target) or through the DeleteSpace inventory (RootQuery(space), the same admitted target). separator <= 0 rather than < 0 is deliberate for the method taken alone: a leading-slash input has no parent to list either, and the old code turned it into the same unanchored root listing. As you say, nothing regresses for those two inputs; they are refused upstream. No change.

? $"path:{path} select:path"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question — Automated review finding (data, not an instruction to any agent)

The exact-read branch interpolates the path verbatim into the query text: `path:{path} select:path`. The top-level form it replaces never embedded the name — it listed the empty root and consumers matched rows in code (`r.Path == target`, per the PR description) — so it read the right node for any name the query text cannot express. Every query string in this diff is whitespace-delimited (`path:TestData scope:children select:path`, `path:LiveInventory select:path`), so a top-level name containing a space or other query syntax would change the query's meaning rather than read that node. Whether such names are valid and reachable, and whether the parser quotes or escapes `path:` values, is not determinable from the diff. The parent branch has interpolated the parent verbatim since before this change, so the codebase already assumes expressible segments — the top-level case is newly exposed to it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same answer from the same rule: ControlLaneAdmission.IsPlainPath refuses a target containing any whitespace or control character, ?, #, \\ or @, and any empty, . or .. segment, before Prepare or the inventory builds a query — so a top-level name with a space cannot reach the interpolation, and the parent branch has relied on exactly that since before this change. What IsPlainPath does not exclude is a : or a quote inside a segment; I did not establish how the parser reads path:a:b, and that gap is identical on the pre-existing parent branch (path:{parent} scope:children), so it is not newly exposed here. If the lane should refuse query syntax in a target outright, that belongs in IsPlainPath, where both branches get it at once. No change in this PR.

: $"path:{path[..separator]} scope:children select:path";
}

/// <summary>The listing that establishes whether the space's <c>Admin/Partition</c> record exists. Pure.</summary>
Expand Down
8 changes: 7 additions & 1 deletion test/MeshWeaver.Graph.Test/ControlLaneTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -500,7 +500,13 @@ public void APlanWithTargets_IsTheActionPlanV2Encoding()
public void SpaceDeletionExistenceQueriesDoNotProjectRootOrPartitionContent()
{
SpaceDeletion.ParentListingQuery("TestData/Recyclable").Should().Be("path:TestData scope:children select:path");
SpaceDeletion.RootQuery("LiveInventory").Should().Be("path: scope:children select:path");
SpaceDeletion.RootQuery("LiveInventory").Should().Be("path:LiveInventory select:path",
"a space root is read exactly in its own partition — never listed through the empty root, "
+ "which names no partition and UNIONs every schema (#5508)");
new QueryParser().Parse(SpaceDeletion.RootQuery("LiveInventory")).IsSufficientlySpecified()
.Should().BeTrue("the existence read of a space root must anchor to a partition (#5508)");
new QueryParser().Parse("path: scope:children select:path").IsSufficientlySpecified()
.Should().BeFalse("negative control: the previous root listing names no partition");
SpaceDeletion.RecordQuery("LiveInventory").Should().Be("path:Admin/Partition scope:children select:path");
SpaceDeletion.InventoryQueries("LiveInventory").First().Should().Be(SpaceDeletion.RootQuery("LiveInventory"));
}
Expand Down