Skip to content

[metadata-core] sys_view_definition:owner/organization_id 为 NULL 的视图完全不受唯一索引约束(两条同名活跃共享视图可并存) #6417

Description

@baozhoutao

发现于 #5839(活跃行唯一)的实施。不在该 PR 范围内——#5839 的维护者裁决只针对「归档视图占名额」,本条是同一索引上的另一个缺口,修它是独立的行为变更。

Blocked-by: #5839 / PR #6415(修复点会落在该 PR 新增的 packages/metadata-protocol/src/migrations/view-definition-active-index.ts 里,需先落地)

事实

packages/metadata-core/src/objects/sys-view-definition.object.ts 的索引声明:

{ name: 'idx_sys_view_def_active', fields: ['name', 'organization_id', 'owner'], unique: true }

注释写的是:

A given view name is unique per (organization, owner) — a shared view (owner NULL) and each user's personal views don't collide.

「shared view (owner NULL) 与各用户的个人视图不冲突」成立,但它隐含的另一半——共享视图之间应当互相冲突——不成立。owner 对共享视图是 NULL,而 SQL 的 UNIQUE 把 NULL 视为互不相等,所以该索引对共享视图完全不生效。organization_id 同理(required: false,环境级视图为 NULL)。

实测(真实 SQLite,用驱动实际产出的 DDL)

DDL 取自 packages/drivers/driver-sql/src/declared-index-retired-keys.test.ts 已钉住的真实产出:

CREATE UNIQUE INDEX `idx_sys_view_def_active` on `sys_view_definition` (`name`, `organization_id`, `owner`)
CASE 2: 两条 ACTIVE 个人视图,同 (name, org, owner)
  insert active personal view : OK
  second ACTIVE duplicate     : REJECTED: UNIQUE constraint failed  ← 约束生效

CASE 3: 两条 ACTIVE 共享视图(owner NULL)
  insert active shared view   : OK
  second ACTIVE shared dup    : OK                                  ← 约束不生效

CASE 4: 两条 ACTIVE 环境级视图(organization_id NULL)
  insert active env view      : OK
  second ACTIVE env dup       : OK                                  ← 约束不生效

即:个人视图受约束,共享视图与环境级视图不受任何约束。

用户可达的后果

同一租户内可以存在两个同名的共享视图。视图切换器按 name 聚合与去重的地方会拿到重名条目;name 本身是「全局唯一的限定视图 id object.viewKey」(字段注释原文),所以任何按 name 定位视图的读路径都存在取到哪一条不确定的问题。

已有的对照解法

sys_metadata 侧的同类问题已经解决:metadata-protocol 的 ensureOverlayIndex() 用 COALESCE(package_id, '') 把 NULL 折成空串,注释里写明理由是「a plain unique index would treat NULLs as distinct and allow duplicate globals」。ADR-0120 D3 则对租户列用 COALESCE(<tenantField>, '__global__'),并在 SqlDriver.createNullSafeUniqueIndex 里带了 MySQL 函数式键值不支持时的降级与 D4 的冲突行口径。两条既有先例都可直接照抄。

为什么 #5839 没有顺手修

#5839 的迁移只改索引的行范围(WHERE state = 'active'),刻意不动键的拼写。这不是遗漏,是让该迁移严格弱于它替换掉的全量索引——活跃行是全部行的子集,所以任何满足旧约束的库必然满足新约束,迁移不可能在存量数据上建失败。

把键改成 NULL-safe 则相反:那是收紧,在已经存在重名活跃共享视图的库上会直接建不出索引(上面 CASE 3 就是这样一条现成的样本),需要 ADR-0120 D4 的冲突行处置口径配套。这是独立的取舍,需要单独拍板。

PR #6415 已加了一条测试把该缺口如实钉住(does NOT close the pre-existing NULL-distinct hole for shared views),修复本条时应把它翻转为正向断言。

需要拍板的点

共享视图(scope='shared', owner NULL)之间的同名,应当禁止还是允许?

  • 禁止 ⇒ 按 ensureOverlayIndex / ADR-0120 D3 的 COALESCE 形态收紧,并按 D4 处置存量冲突行;
  • 允许 ⇒ 该索引对共享视图形同虚设这一点应写进声明注释,且 name 字段「全局唯一」的描述需要改。

Activity

  1. os-zhuang commented on Aug 7, 2026

    @os-zhuang
    Contributor

    Triage: needs-user-decision + domain:metadata.

    Why the decision inbox and not the queue: the card ends on an explicit either/or — should two same-name shared views (owner NULL) be prohibited or allowed — and the card is right that the two branches are not variants of one fix. Prohibiting is a tightening that can fail to build on existing data and needs the ADR-0120 D4 conflict-row disposition; allowing means rewriting the name field's "globally unique" self-description. That is a product-semantics call plus a stored-data-shape call, both on the maintainer-confirm list, not something routing can resolve.

    Landing site: packages/metadata-core/src/objects/sys-view-definition.object.ts — verified on origin/main, the idx_sys_view_def_active declaration is still fields: ['name','organization_id','owner'], unique: true (:137). packages/metadata* ⇒ domain:metadata, same lane as its sibling #5839.

    On the Blocked-by: line: the body's dependency on #5839 / PR #6415 stands and stays recorded there. No pm:blocked label was added, because a card awaiting a ruling is undispatchable regardless and two pm-states on one card is the half-state the label discipline bans. Whoever clears the ruling should add pm:blocked at that moment if #6415 has not merged yet.

    For the maintainer, the one-line version: SQL UNIQUE treats NULLs as distinct, so today the index constrains personal views only — shared and environment-level views are unconstrained, and two same-name active shared views can coexist (CASE 3/4 in the card are real SQLite runs). ensureOverlayIndex's COALESCE(package_id,'') and ADR-0120 D3's COALESCE(…, '__global__') are both in-tree precedents for the "prohibit" branch.

    本评论来自分诊座位 Routine(#5474 试点),不构成认领。


    Generated by Claude Code

  2. os-project-manager commented on Aug 8, 2026

    @os-project-manager
    Collaborator

    Maintainer ruling — 2026-08-08. The maintainer reviewed the PM's three-axis analysis of the decision inbox and accepted the recommendations (「按照你的建议继续」). Recorded by the PM session; needs-user-decision comes off with this comment.

    Decision: forbid same-name active shared views — tighten the index to a NULL-safe key. Two same-named active shared views (or environment-level views) may not coexist; name keeps its declared meaning as the globally unique qualified view id.

    Rationale: the defect is user-reachable today (the view switcher de-duplicates by name, and any read path locating a view by name has no defined answer when two rows match); the declaration says unique and must be enforced, not documented away; and a loud rejection at insert is learnable for an authoring agent while a silent duplicate is not.

    Implementation — copy the two in-repo precedents rather than inventing a form:

    Also unblocked: the body's Blocked-by: #5839 / PR #6415 is cleared — PR #6415 merged 2026-08-07T17:56:44Z, and the migration file this fix lands in (migrations/view-definition-active-index.ts) now exists on main.


    Generated by Claude Code

  3. self-assigned this
    on Aug 8, 2026
  4. os-zhuang commented on Aug 8, 2026

    @os-zhuang
    Contributor

    Claim: PM loop round 1 (domain:metadata seat, sticker #6367)
    Session: session_01W6bLax4KMrSfnE1ydFU8Dw
    Branch: claude/issue-6417-view-def-nullsafe-index
    Worktree: objectstack-issue-6417
    Domain: domain:metadata
    File surface: packages/metadata-core/src/objects/sys-view-definition.object.ts + packages/metadata-protocol/src/migrations/view-definition-active-index.{ts,test.ts} + (only if the declared-index DDL pin moves) packages/drivers/driver-sql/src/declared-index-retired-keys.test.ts + one .changeset/*.md
    Serial constraints cleared: predecessor #5839 / PR #6415 MERGED 2026-08-07T17:56:44Z (the migration file this fix extends exists on origin/main; verified by ls-tree); no in-flight claim touches these files (#6418 targets ensureOverlayIndex in protocol.ts, a different index and file, and is not yet dispatched). Maintainer ruling of 2026-08-08 (forbid duplicates; NULL-safe tightening per ensureOverlayIndex / ADR-0120 D3, conflict-row disposition per D4) governs — not re-opened here.


    Generated by Claude Code

  5. os-zhuang commented on Aug 8, 2026

    @os-zhuang
    Contributor

    Review verdict: ACCEPT — PM domain:metadata seat, session session_01W6bLax4KMrSfnE1ydFU8Dw.

    PR #6666 implements the 2026-08-08 maintainer ruling in full:

    • The runtime migration's key is now NULL-safe — COALESCE(organization_id, '__global__') (ADR-0120 D3's tenant sentinel) + COALESCE(owner, '') (ensureOverlayIndex's non-tenant discriminator form) — still WHERE state = 'active', still under the declared name. Both spellings copied from the named in-repo precedents, not invented.
    • ADR-0120 D4's conflict disposition is implemented and proven on a real SQLite database: previous index kept byte-for-byte, unenforced key named, the exact duplicate-listing query shipped in the report (and the test runs that query and asserts it names the seeded offenders), os migrate plan pointed at, boot never blocked.
    • PR fix(metadata): sys_view_definition 的「活跃行唯一」补运行时 partial UNIQUE 迁移 (#5839) #6415's honest pin (does NOT close the pre-existing NULL-distinct hole) flips to a positive rejection assertion, with the constraint's identity asserted (index name), not merely "it threw".
    • The declaration in sys-view-definition.object.ts is untouched beyond its comment, so the driver-sql declared-index DDL pin did not move — verified against the PR's file list.
    • Reverse verification ran with the direction predicted before execution (11 red under the restored limb), with two non-evidence results reported honestly.
    • One change beyond the ruling's letter, accepted on review: the dialect-degradation report (and the catch-all arm) is raised from info/warn to error. Rationale holds — the same missing DDL now costs a stated integrity guarantee rather than slot recycling, which is the durability arm's own description and the level SqlDriver.createNullSafeUniqueIndex (which the ruling itself points at) uses. Pinned by tests and declared in the PR body rather than smuggled.

    Process notes: PR title translated to English by the PM (language policy 2026-08-06); the dev's successor-pricing answer for #6418 (unaffected; one caution) is recorded on #6418. Next: CI convergence check, then ready + merge queue — landings on this lane stay serial.


    Generated by Claude Code

  6. os-zhuang commented on Aug 8, 2026

    @os-zhuang
    Contributor

    Landing confirmed — PR #6666 merged through the queue as bb7cb417c (verified by both readings: queue branch drained + the migration file's history on origin/main). Issue auto-closed by the Fixes line at 2026-08-08T10:07:44Z. sys_view_definition's active-row index is NULL-safe from this commit on; deployments holding pre-existing duplicate active shared views get the ADR-0120 D4 conflict disposition at boot instead of a silent hole. — domain:metadata PM, session session_01W6bLax4KMrSfnE1ydFU8Dw


    Generated by Claude Code

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions