Skip to content

Commit ca0a98c

Browse files
kvlongekvlonge
committed
Reviews: unary-sign evidence 08fc5bb blocked, PR #152 at 0eaab3d and rejection messages at 316d770 approved
1 parent 419da73 commit ca0a98c

3 files changed

Lines changed: 102 additions & 0 deletions

File tree

‎coverage/syntax/audits/parser-keyword-table-alias-span-pr152-0eaab3d3.toml‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
[review]
2+
schema = "sqlquack.recorder-change-review.v1"
3+
decision = "D118"
4+
pull_request = 152
5+
branch = "fix/keyword-table-alias-span"
6+
commit = "0eaab3d34415efa7a6f8432f3eda23cf9cb8f1eb"
7+
base = "eaf17f696fcd823ecd544ce653f836bbbc8b0cba"
8+
merge_base = "eaf17f696fcd823ecd544ce653f836bbbc8b0cba"
9+
reviewed_at = "2026-11-07T12:58:25Z"
10+
round = 1
11+
verdict = "approved"
12+
checked = [
13+
"0eaab3d3 is one commit on main eaf17f69. It changes three lines in parse_join_table_suffix (joins.rs) and adds one test_located row. The keyword alias (OFFSET or LIMIT before MATCH_CONDITION on an ASOF JOIN table) now takes its token's span via Identifier::with_span(peek().span), the pattern primary.rs uses for identifiers.",
14+
"Identifier.span is skipped when None and ignored by equality. The polyglot located comparison strips span fields, so only located output changes. The test row points at /0/select/joins/0/this/alias/alias and expects OFFSET.",
15+
"Built 0eaab3d3. check accepts ASOF JOIN y OFFSET MATCH_CONDITION (…), the same with LIMIT, a plain alias z, no alias, and a trailing LIMIT 5. convert refuses the OFFSET and LIMIT alias shapes with SQ0002 node:table_function, as it does without the fix, so no conversion outcome changes.",
16+
"syntax-coverage at 0eaab3d3: parser tree 69c7c6be (main is 7fa8e1ec, as the lane found). stale is empty. diff coverage/syntax/baseline.json shows 0 problems, no form or obligation gained or lost, and 5128 contradicted.",
17+
"Did not run the sqlquack-syntax tests, clippy, polyglot test_exact or the full gate. Those are the lane's, at the slot.",
18+
]
19+
20+
[[review.finding]]
21+
kind = "sequencing"
22+
blocking = false
23+
detail = "#152 moves the Snowflake parser_tree id, like #136 and #138. Whichever lands later must re-record coverage/syntax/baseline.json on its merged head."
24+
25+
[review.files_sha256]
26+
"crates/sqlquack-syntax/src/parser/_helpers/queries/joins.rs" = "073259d669431bce549bd77077ae60b3814a26a6528b0c622fdcce45468825c8"
27+
"crates/sqlquack-syntax/tests/parser/test_located.rs" = "cdfd439b105a8703112f42efb1ed0866eb1049d2f31552e1a006914d5fdf8984"

‎coverage/syntax/audits/parser-snowflake-rejection-messages-316d770f.toml‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
[review]
2+
schema = "sqlquack.recorder-change-review.v1"
3+
decision = "D118"
4+
related = ["CHI-1067"]
5+
branch = "fix/snowflake-rejection-messages"
6+
commit = "316d770ff115dd20c5646eb8a0e72d4f7837bfd5"
7+
base = "9aaf4a319d976ae01b420a22515b029176bc7fdd"
8+
merge_base = "a9d9407988ab39ecd53c076a15ca40dc48688696"
9+
previous_review = "fc2802703e966937d47ec48effa0181c3ff8a4e5"
10+
reviewed_at = "2026-11-07T12:58:25Z"
11+
round = 2
12+
verdict = "approved"
13+
checked = [
14+
"316d770f is 9aaf4a31 plus 808fe0e8 and 316d770f. Only rejections.rs and test_refusals.rs change. double_equals now returns no locations when node_location gives none, in place of bounding by the whole input. The new test row SET x = 1; SELECT 1 == 2 AS y; SHOW TABLES; USE DATABASE d expects exactly one double_equals. 316d770f shortens the doc comment.",
15+
"Built 316d770f and probed check and convert. Exactly one SQ0210 for each of: SHOW TABLES; SELECT 1 == 2, USE DATABASE d; SELECT 1 == 2; USE SCHEMA s, SET x = 1; SELECT $x == 1, SET x = 1; SELECT 1 == 2, SELECT 1 == 2; SET x = 1, a WHERE 'x' == status, and BEGIN SELECT 1 == 2; END. In convert, SET, SHOW and USE get their own SQ0002 again. SELECT 1 == 2; SELECT 3 == 4 gives two. SELECT 1 = 1 gives none.",
16+
"SET x = (1 == 1) is still refused with SQ0210, so the lane's known limit (== inside a statement without a location) is narrower than stated.",
17+
"fc280270's other checks (hints, polyglot gating, coverage diff and corpus moves) are unaffected, since only core refusal code changes.",
18+
"Did not run check-merge, verify-recorded or translation-results. The gate on 316d770f is the lane's.",
19+
]
20+
21+
[[review.finding]]
22+
kind = "resolved"
23+
blocking = false
24+
detail = "fc280270's blocking finding, duplicate and misattributed SQ0210 for statements without a node location, is fixed."
25+
26+
[review.files_sha256]
27+
"crates/sqlquack-core/src/refusals/_helpers/rejections.rs" = "b67268a060fa43526096e4771a2c2e7d345b3fd18a35bd4839b2fc78857fd006"
28+
"crates/sqlquack-core/src/refusals/tests/test_refusals.rs" = "c482971e16765622aa95073abf6bc8cadda9f93b873460927364f2ea0155cb85"

‎coverage/syntax/audits/product-d315-unary-signs-evidence-08fc5bb4.toml‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
[review]
2+
schema = "sqlquack.recorder-change-review.v1"
3+
decision = "D315"
4+
related = ["D322"]
5+
branch = "wip/d315-unary-signs"
6+
commit = "08fc5bb426c5aabdbd64eafc57aea898882ad869"
7+
base = "eaf17f696fcd823ecd544ce653f836bbbc8b0cba"
8+
merge_base = "eaf17f696fcd823ecd544ce653f836bbbc8b0cba"
9+
receipts = ["f40e3d25 (2 results)", "08fc5bb4 (1 result)"]
10+
reviewed_at = "2026-11-07T12:58:25Z"
11+
round = 1
12+
verdict = "blocked"
13+
checked = [
14+
"08fc5bb4 is four commits on main eaf17f69 (a00323e9, f40e3d25, 27122574, 08fc5bb4), all authored with the configured no-reply identity. It only adds cases/relational/unaliased_unary_signs.toml (3 cases) and three evidence results. Nothing is modified or deleted.",
15+
"Recorded results: unaliased_unary_signs names its columns +X, -X, + 1, +(1), with rows (2, -2, 1, 1) twice and (NULL, NULL, 1, 1). unaliased_unary_signs_over_no_rows has the same names and no rows. +X and -X are FIXED(10,0), and + 1 and +(1) are FIXED(2,0). unaliased_double_unary_plus (SELECT ++1) is error 1005/42P13 and is declared compile_error.",
16+
"The D315 literal receipts agree: -1 and - 1 are NUMBER(1,0), +1 is NUMBER(2,0), -0.25 is NUMBER(3,2) and -1.5 is NUMBER(2,1). So Snowflake widens a unary plus over a literal by one digit of precision, and does not widen a column (+x stays NUMBER(10,0)) or a unary minus.",
17+
"Replayed the three cases with verify on a build of D315's head 9b337966, with the full cases tree and only this branch's results, so every other case is pending. All three fail. unaliased_unary_signs and unaliased_unary_signs_over_no_rows: column + 1, no declared type equality admits Snowflake NUMBER(2,0) and DuckDB DECIMAL(1,0). unaliased_double_unary_plus: Snowflake raised error 1005 (SQLSTATE 42P13), but DuckDB returned rows.",
18+
"The conversion is SELECT x AS \"+X\", (-x) AS \"-X\", CAST(1 AS DECIMAL(1,0)) AS \"+ 1\", (CAST(1 AS DECIMAL(1,0))) AS \"+(1)\". On DuckDB its names and values equal the recorded ones, so D315's naming is right. Only the type of a unary plus over a literal, and the acceptance of ++1, are wrong.",
19+
]
20+
21+
[[review.finding]]
22+
kind = "wrong_type"
23+
blocking = true
24+
detail = "rule.unary_plus (group 1, on main since D322) keeps a literal's type. Snowflake types +1, + 1 and +(1) as NUMBER(2,0) where the conversion gives DECIMAL(1,0). This is on main already: SELECT +1 AS v converts to CAST(1 AS DECIMAL(1,0)). Values are equal, so it shows only in the column type. Smallest fix: type a unary plus over a number literal, parenthesised or not, as NUMBER(p+1,s), or refuse it until receipts show the rule. Receipts to ask for: +1.5, +123, +0.25, +(1.5) and +(+1)."
25+
26+
[[review.finding]]
27+
kind = "accepts_rejected"
28+
blocking = true
29+
detail = "SELECT ++1 converts (named ++1 under D315, and SELECT ++1 AS v on main) but Snowflake rejects it with 1005/42P13, invalid function '+'. Smallest fix: refuse a unary plus whose operand is itself a unary plus, with a Snowflake-rejects message. Receipts would settle +(+1), +-1, -+1 and - -1."
30+
31+
[[review.finding]]
32+
kind = "auditor_miss"
33+
blocking = false
34+
detail = "0647755c (group 1 delta at aa6abaa9) probed SELECT +1 AS v converting to CAST(1 AS DECIMAL(1,0)) and did not compare that type with Snowflake's. No recorded case had a literal operand under node:pos, so verify could not catch it either."
35+
36+
[[review.finding]]
37+
kind = "sequencing"
38+
blocking = false
39+
detail = "D315's own naming is right, and its approval (70ba8450) stands. This branch can only land after the converter fixes above are on main. Until then its three cases fail verify on any head with D315."
40+
41+
[review.files_sha256]
42+
"cases/relational/unaliased_unary_signs.toml" = "bd08f9f4d1b6a5c6f208256283fa5798ac0c35191a772f40c01fb1571cfa6876"
43+
"evidence/results/502f6551ffb87fe10dab7b9f98810f3e8ca419b698d60fe709b54c55dfe70cb2.arrow" = "1643051822e962f2968e8c6b240ba29fd9f5b6e4c033296dad5f672492d01254"
44+
"evidence/results/502f6551ffb87fe10dab7b9f98810f3e8ca419b698d60fe709b54c55dfe70cb2.json" = "dcb6c657fba7183bc58d2501637ee54f78c52ef3725e51d31a9b94b78324277c"
45+
"evidence/results/8dce34c644ff7b85a840d50014c23ed106d7384e013d3717c3a3a3e6a544539d.arrow" = "c70638feffa886c6ac3b39d5d1ef5879e737c385486d06f8c8fa2d0fbf82ffd1"
46+
"evidence/results/8dce34c644ff7b85a840d50014c23ed106d7384e013d3717c3a3a3e6a544539d.json" = "a8e1dc9c71596b6236df09fa6b0415558a31961d22dd33b2ef5487e293246390"
47+
"evidence/results/e79042ef8918f6e48c6e8a06463ce4eba4ce28b3190d48be15a9ffa896c60d23.json" = "3df5d8be2f10cc05f07e447f3f8844f585114af9f65b5558566e4b66f4ca344f"

0 commit comments

Comments
 (0)