fix(service-analytics): postgres 的「缺列」措辞不再被判为「缺源」(#6035) - #6346
Merged
Conversation
…d failure (#6035) `isMissingSourceError`'s docblock promises the dataset degradation path is scoped to missing SOURCE and that "column/syntax errors stay hard failures so real query bugs still surface". One postgres wording broke that by construction: column "label" of relation "acct" does not exist (SQLSTATE 42703) carries `relation "acct" does not exist` inside it verbatim. #5717 anchored the postgres limb to postgres's real missing-table wording and this string matched anyway -- it had to, because it literally contains that wording. No tightening of "does this say a relation is missing" can exclude it; only asking the more specific question first can. The fix is therefore an ORDERING, not a better regex: subtract the column phrase, then classify. Both consequences were wrong, and which one fired was an accident of whether the named relation happened to be the dataset's own object: a joined name produced a loud but FALSE cross-datasource topology refusal, while the dataset's own name degraded the widget to an empty grid with the mistyped column mentioned to nobody. Both halves are pinned. The subtraction is `rest-server.ts`'s `mapDataError` regex verbatim (its `unknownColumn` probe has extracted this same phrase ahead of the unknown-object branch since #5352, so the REST face answers 400 INVALID_FIELD rather than 404) -- the same pattern rather than a second dialect of it, so the two faces cannot disagree about what counts as postgres saying "column". `missingSourceRelation` subtracts it too: measured on origin/main it answered `sys_team` for this wording, so guarding only the sniffer would leave "is something missing" and "what is missing" contradicting each other -- the exact disagreement #5717 closed on this limb. This aligns a predicate with its own documentation rather than repairing an incident: analytics is a read face and postgres spells an unknown column in a SELECT as `column "bogus" does not exist`, with no `relation` in it. The value is that the disagreement no longer depends on that dormancy holding. #5717's 13 measured in-repo wordings are re-pinned as `queryDataset` OUTCOMES (empty grid / topology refusal / propagated) rather than as private-predicate booleans, so the pair is exercised jointly through the public API: exactly ONE verdict moves, the column phrase; the other 12 are unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015a5qkLzpGXhLL2F5gvJ7dD
|
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
github-actions
Bot
added
size/m
documentation
Contributor
📓 Docs Drift CheckThis PR changes 1 package(s): 8 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #6035
前提复核(结论:成立;但分诊评论的「事实修正」本身已过期,以下为实读)
9e3709a,而 #5717 的实现 PR #6045 在那之后才合并(705efebf2)。所以分诊那条事实修正(「:96 的宽判据msg.includes('relation') && msg.includes('does not exist')仍在服役」)在今天的 origin/main 上已不成立——#6045 确实把isMissingSourceError里的 postgres 支也收紧成了锚定正则。逐行实读(基于2bc187641,含 #6292):isMissingSourceErrordocblock 写明「not column/syntax errors, which stay hard failures」:96仍是includes('relation') && includes('does not exist')宽判据/relation\s+["']?[A-Za-z0-9_$.]+["']?\s+does not exist/imissingSourceRelation对同一措辞也给出缺源答案sys_teamcolumn "bogus" does not exist实测不命中)核心前提成立,
premise_still_valid: true。 只是「要动 :96 的宽判据」这个落点描述过期了——真正的落点不是「宽判据 vs 正则」之争,而是判定顺序:缺列措辞里逐字包含一整段合法的缺表措辞,所以无论那一支收紧到什么程度都排除不掉它,只有先问更具体的问题才可以。#6045 自己的 docblock 已经把这条残留指名留在原地(写着 filed as #6035),本 PR 接上。改了什么(一个文件,一处新常量 + 两处前置摘除)
新增
MISSING_COLUMN_OF_RELATION,在isMissingSourceError与missingSourceRelation两处判定之前摘除:正则逐字抄
rest-server.ts的mapDataError(它的unknownColumn探针自 #5352 起就先摘这条措辞、再进 unknown-object 分支,于是 REST 面回答400 INVALID_FIELD而不是404;用例钉在rest.test.ts)。刻意用同一条正则而非它的第二种方言——两个面不该对「postgres 什么时候在说 column」给出不同答案。仓内还有一条独立佐证:rest-unknown-object-heuristic.test.ts有一例就叫「the column-level branches still win over the relation limbs」,理由写的是「Postgres spells an unknown column withrelation … does not existinside it」。REST 面早就有这条纪律,本 PR 是把 analytics 面拉齐。要求两侧都带引号,是安全的犯错方向:postgres 的 errmsg 模板本身就是
column "%s" of relation "%s" does not exist;漏匹配只是维持今天的判定,而过度匹配会把真正缺表的错误变成硬失败,那是反向扩大、会回退 #5033 的刻意宽容。两半后果都修了(哪一半触发只是 fixture 的偶然)
sys_team)只修一半的话,从另一半看过去仍然是绿的,所以两半都钉。
missingSourceRelation是否同样前置摘除:取舍(立单要求写明)做了,理由是一致性而不是第二个 bug。 实测:改动前它对这条措辞回答
sys_team。若只修「是不是缺了什么」,这对函数就会互相矛盾——一个说什么都没缺,另一个指着一张表说它缺了。而那正是 #5717 刚在这一支上消除的分歧(它的 docblock 原话:so "is something missing" and "what is missing" can no longer disagree on this limb),再打开等于把同一颗雷重新接上电。诚实的代价说明:这处 guard 今天不可达,因为该函数只在
isMissingSourceError为真之后才被调用。但「不可达」是一个调用点的调用顺序的性质,不是这个函数的性质——它一行编辑之外就会变。所以我加了 guard,并且不为它单独写测试:没有公开路径能到达它,硬写一个只会得到一个「因为什么都没产生所以通过」的空用例(#5046 的教训)。这一点写在代码注释里,也写在这里,而不是用一个看起来验证过的断言糊过去。测试
新增
packages/services/service-analytics/src/__tests__/missing-column-phrase-hard-failure.test.ts(18 例)。关键设计:#5717 量的 13 条语料,这里按
queryDataset的可观测结果(空网格 / 拓扑拒收 / 原样上抛)重新钉,而不是按私有判据的布尔值——这样被守住的是调用方真能观察到的行为,而且isMissingSourceError+missingSourceRelation这一对是被联合行使的。每条措辞都裸抛(剥掉信封):其中好几条真实 producer 自带 ADR-0112 信封,会靠 #5717 的防线 B 上抛,不剥的话它们作为嗅探器的 pin 就是空的。13 条语料量化(实测,收紧前 / 后)
no such table: trelation "t" does not existTable '…' doesn't existDatasource … is not registered.Object 'x' is not registeredCUBE_NOT_FOUND(#3867)nested/relation value …13 条里只有 1 条改判,就是缺列那条;没有任何真实驱动措辞改判——这就是「收紧而非改变 #5033 行为」的证据形态。
反向验证(方向在跑之前就写死在测试文件头)
预测:本改动只减掉一条措辞,所以回退必须只让关于那条措辞的用例变红,别的一条都不许动。实测(把两处 guard 同时停用):
dataset-degradation-envelope.test.ts本文件 3 红 / 15 绿。我第一版在文件头把绿数写成了 12(18 例算错),更正留在原处而不是悄悄改掉——但要说清楚:算错的是绿的条数,预测的红集合完全命中,而红集合才是这个反向验证承载论证的部分。
两条回退失败的实际输出值得引用,因为它们就是缺陷本身而不只是红标记:
第一条是「对着一个拼写错误讲数据源拓扑的故事」,第二条是什么都没抛、控件拿到了空网格。
消费半径清扫
isMissingSourceError/missingSourceRelation都是analytics-service.ts的模块私有函数,全仓引用实测各只有 1 个调用点(queryDataset的 catch),无跨包 fixture 喂它们。仍按规矩全仓扫了of relation的所有拼写:命中的是packages/rest的既有先例与用例(不受影响,且已单独回归跑过)、packages/metadata的一处注释,以及本 PR 自己的文件——无需改写任何既有 fixture。验证记录(全部前台阻塞执行,持容器级
flock /tmp/os-heavy-verify.lock,--max-old-space-size=4096)pnpm --filter @objectstack/service-analytics exec vitest run --maxWorkers=2→ 64 files / 1238 tests passed(改动前后各跑一次,均全绿)queryDataset里还有第二个 message 嗅探器isMissingSourceError,命中即静默返回空结果 ——dataset-compiler的一条拒收措辞已经命中它,只因抛点在 try 之外才没出事 #5717 文件定向:2 files / 29 tests passedpackages/rest(analytics 相关 +rest-unknown-object-heuristic)→ 8 files / 105 tests passedpnpm --filter @objectstack/service-analytics build→ 成功;tsc --noEmit直测该包 → 10 个错误,与账本冻结数一致,新增文件贡献 0 个check:type-check-coverage、check:nul-bytes、check:doc-authoring、check:adr-anchors、check:route-envelope、check:error-code-casing、check:wildcard-fallthrough、check:durability-log-level、check:startup-registry-verdict、check:engine-double-contract、check:slot-lookup、check:query-options-erasure、check:release-notes、check:empty-changeset、pnpm lint(全仓 ESLint)→ 全 PASScheck:type-check-debt:首跑红,复核为未建依赖假阳性。该门在 lint.yml 里跑在turbo run build --filter='./packages/*' --filter='./packages/*/*'之后(脚本注释写明 tsc 要经各依赖已构建的dist/*.d.ts解析)。补跑同一条构建后复测 → PASS(34 个账本条目,none above its recorded number)。首跑报红的 12 个包里没有一个是 service-analytics,与本改动无关。grep -naP扫三个改动文件的控制字节区间 → 干净未触:
packages/spec、drivers、REST 路由本体、#5717 的信封防线 B。Changeset
.changeset/analytics-missing-column-hard-failure.md→@objectstack/service-analytics: patch(降级路径的判定行为对调用方可见)。出界发现
已立 #6347(
finding,未认领,未带pm:queue),不在本 PR 顺手改:packages/metadata/src/utils/schema-sync-errors.ts的MISSING_TABLE签名是同族子串洞——写路径缺列措辞实测命中它的 message 支(读路径措辞不命中),而matchesDriverError里 code / errno / message 是并列的或,所以「缺列 SQLSTATE42703不在 codes 集里」挡不住它。该判据自己的 docblock 恰好点名了 42703、说它「must stay loud」,并说明误判的代价是「start numbering at 1 would be the wrong answer against a table that may be full of rows」——而那正是消费点database-loader.ts:307-312在做的事。同样因只读面不产生该措辞而休眠,故按 observation 类记录,严重度留给分诊轮定级。