feat: resolve Oracle inline-view INSERT targets to their base table - #58
Merged
Merged
Conversation
Contributor
|
✅ PR title follows the Conventional Commits spec. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #58 +/- ##
=======================================
Coverage 94.77% 94.77%
=======================================
Files 27 27
Lines 5590 5686 +96
Branches 5590 5686 +96
=======================================
+ Hits 5298 5389 +91
- Misses 214 218 +4
- Partials 78 79 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
`INSERT INTO (SELECT a, b FROM emp WHERE dept = 10) VALUES (…)` writes the view's single base table — resolve through to it instead of dropping the whole statement as an unsupported target. The view projection names the target columns (an explicit list: plain columns only, `a AS x` writes base `a`; a wildcard / expression falls back to the column-less catalog-fill / diagnostic path), the WHERE predicate is filter reads against the target (a new `Insert::target_predicate`), and a SELECT source traces through the view (`s.x → emp.a`). Only the minimal insertable-view shape (projection + FROM + WHERE) resolves through — enforced by exhaustive `Query` / `Select` destructures, so a new clause forces a decision. Everything else keeps the previous drop + `UnsupportedStatement` flag: a join / set operation / nested WITH names no single base table SQL text can determine (key-preserved rules need a catalog), and any other clause (GROUP BY / DISTINCT / ORDER BY / FETCH / …) makes the view non-insertable and could carry silently-dropped column refs. An aliased base table (`FROM emp e`) resolves the predicate's `e.dept` through the alias. `TableReference`'s `TryFrom<&Insert>` sees through the same shapes. Follow-up from #55; only `OracleDialect` parses this syntax. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01AteCEF9XeviLCkfa3EQMfc
takaebato
force-pushed
the
feat/insert-subquery-target-resolution
branch
from
July 4, 2026 09:56
c6bd3c2 to
aef20c2
Compare
Merged
takaebato
added a commit
that referenced
this pull request
Jul 5, 2026
## Summary Organizes and fixes **write-target attribution for unqualified SET columns** (follow-up noted while reviewing #58; design agreed in discussion). Previously, `UPDATE t1 JOIN t2 ON … SET a = 1` attributed the write to the **root** (first) table unconditionally — even when a catalog said only `t2` owns `a` (the read side would resolve the same `a` to `t2`, so `SET a = a + 1` read `t2.a` but "wrote" `t1.a`). The root pin had no textual basis: MySQL's first table is just a join operand, and real MySQL rejects the ambiguous case outright (error 1052), so no write target ever exists to pin. Now the unqualified target resolves with the **same candidate/pick rules as a read**, over the statement's *writable* relations: | SET target | writable relations | written table | resolution | |---|---|---|---| | `t2.col` (qualified) | (any) | the qualifier's | Cataloged iff listed | | `col` | none besides root (MERGE / ON CONFLICT) | root | Cataloged iff listed | | `col` | one (single-table UPDATE) | root | Cataloged iff listed | | `col` | several — sole candidate | the owner | read-mirrored (witness downgrade included) | | `col` | several — several candidates | *none* | `Ambiguous` | | `col` | several — no candidate | *none* | `Unresolved` | The mirror applies only where there is a genuine choice (several writable relations). The sink is text-named — and keeps pinning unconditionally — for a single-table UPDATE, MERGE SET, ON CONFLICT SET, and PostgreSQL / T-SQL `UPDATE t SET … FROM u` (FROM relations join the read scope but are never writable; the writable set is snapshotted before FROM absorption). Implementation reuses the read side's resolution (`resolve_in` / `pick`) restricted to real-table relations, so witness rules and downgrades match reads exactly. The matrix lives as rustdoc on `resolve_assignment_column`. **Breaking** (`feat!`): `ColumnWrite`'s documented contract previously ruled out `Ambiguous`/`Unresolved` on writes. An unqualified multi-table SET can now surface them with `table: None` (exactly like an unattributed read), and such writes contribute no table-level write — `UPDATE t1 JOIN t2 ON … SET a = 1` catalog-free now yields `update_tables: []` (was `[t1]`), with the write still visible at column granularity. Tests: one per matrix row (catalog-aware, in `resolution.rs::catalog_strict`), read/write-consistency for `SET a = a + 1`, FROM- and MERGE-exclusion, the catalog-free `Ambiguous` pin, and a table-level "no update table" case. All gates green (fmt / clippy / `test --all` / doc); 758 tests. Edge cases covered on review: - **Lineage to an unattributed target**: `SET a = t2.c` still emits the value edge — its target carries `table: None` + `Ambiguous`, symmetric with an ambiguous *source* read appearing in lineage (pinned by test). - **Tuple SET** (`SET (a, b) = (1, 2)`) flows through the same attribution. - MySQL's comma form (`UPDATE t1, t2 SET …`) doesn't parse in sqlparser — unreachable. - `ARCHITECTURE.md`'s reads/sink section now covers the unqualified rule (the matrix stays on `resolve_assignment_column`). Deferred (tracked): extending the same catalog-owner machinery to the Oracle join-view INSERT projection (needs an Insert-model extension for the companion-table scans). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
takaebato
added a commit
that referenced
this pull request
Jul 5, 2026
) ## Summary Implements the remaining inline-view deferral from #58 (design agreed in discussion): an Oracle **join-view** INSERT now resolves to its base table by **column attribution**, instead of dropping the whole statement. ```sql INSERT INTO (SELECT e.id, e.name FROM emp e JOIN dept d ON e.dept_id = d.id WHERE d.active = 1) VALUES (100, 'x') -- before: everything empty + UnsupportedStatement -- after: create=[emp] read=[emp, dept] -- writes=[emp.id, emp.name] -- reads=[emp.dept_id, dept.id, dept.active] (ON + WHERE, filter) ``` ## Attribution rule (composes two existing mechanisms — no new rules) The row lands in the one relation **every** projected column agrees on: | projection column | attribution | precedent | |---|---|---| | qualified (`e.id`) | its qualifier, resolved against the view's relations — text-only, no catalog | multi-table UPDATE's `SET t2.col` | | unqualified (`name`) | the catalog-owner rule (`unqualified_write_binding`, #59) over the view's relations | #59's read-mirrored SET attribution | Any column ambiguous / unresolved / owned by a different relation than its siblings — or any factor naming a declared CTE (not a base table) — → drop + flag as before. Real Oracle rejects those shapes too (the INTO columns must all belong to one key-preserved table); key-preservedness itself isn't *verified* — that needs unique-key metadata the catalog doesn't carry — attribution assumes a valid statement. The classic comma-join spelling (`FROM emp e, dept d WHERE …`) resolves identically. ## Companion relations: `Insert::target_context` The joined tables other than the target (here `dept`) become scanned context: - **table reads** ✓ (`children()` now includes the context, so the scan surfaces; `emp` itself keeps the sink rule — it reads because its columns are referenced, not via a scan, consistent with the single-table view path) - **no data feed** ✓ — table lineage's `feeding_scans` walks `input` only, matching the semantics (the join gates which rows are visible; the value path is the source) - ON + WHERE land in the existing `target_predicate` (filter reads over the full view scope, aliases included) `TableReference`'s `TryFrom<&Insert>` stays **shape-determined**: the gate admits joins of plain tables, but a join view's base table needs binder resolution, so the plain identity parse resolves single-table views only (documented; join → `Err`). ## Edge cases covered on review - **CTE factors**: any view factor naming a declared CTE — target (used to fabricate a write to the CTE) or companion (used to surface the CTE name as a phantom base-table read) — now flags the whole statement, uniformly in the factor loop. A WITH *inside* the view stays gate-rejected; a top-level CTE consumed only by the source still traces through its body (pinned). - **USING / NATURAL join views** resolve, and the USING clause itself adds no reads — consistent with the SELECT path, where reads come from *reference* sites (pinned). - **Self-join views** resolve to the one table (identity is right; key-preservedness is per-instance and unverifiable without key metadata). - **Table lineage**: the companion never feeds (`s → emp` only, no `dept → emp`) — pinned at table level. - Gate rejections pinned: FROM-less view, derived factor (single and joined), wildcard / expression projections, ClickHouse `TABLE FUNCTION` targets (moved code) incl. an unrepresentable function name. - For full patch coverage, the gate hands its proof over **entirely as data** (`InsertTargetView { factors, projection, join_operators, selection }` — parse-don't-validate): the unreachable defensive arms are *deleted*, and no consumer re-reads the `Query`, so `factors` can't be paired with a clause it wasn't derived from. ## Tests Qualified projection (catalog-free), comma form, SELECT-source lineage through the view (`s.x → emp.id`), straddling columns → flag, unqualified without a catalog → flag, unqualified **with** a catalog attributing by owner (Oracle-cased catalog), table-level companion read + lineage, USING / self-join / gate-rejection pins, all CTE placements (factor → flag, inside the view → flag, top-level source-only → traces through the body), and the #58 flag test narrowed to an undetermined shape. All gates green (fmt / clippy / `test --all` / doc); 788 tests, **100% patch coverage** on source lines (llvm-cov ∩ diff). 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
takaebato
added a commit
that referenced
this pull request
Jul 5, 2026
## 🤖 New release * `sql-insight`: 0.3.0 -> 0.4.0 (✓ API compatible changes) * `sql-insight-cli`: 0.2.1 -> 0.2.2 <details><summary><i><b>Changelog</b></i></summary><p> ## `sql-insight` <blockquote> ## [0.4.0](sql-insight-v0.3.0...sql-insight-v0.4.0) - 2026-07-05 ###⚠️ Breaking Changes #### attribute unqualified SET targets with the read-side rules ([#59](#59)) by @takaebato #### upgrade sqlparser to 0.62 ([#55](#55)) by @takaebato ### Added - resolve Oracle join-view INSERT targets by column attribution ([#61](#61)) by @takaebato - fan out multi-column-alias lineage to every alias ([#60](#60)) by @takaebato - resolve Oracle inline-view INSERT targets to their base table ([#58](#58)) by @takaebato ### Fixed - don't surface a ClickHouse ARRAY JOIN operand as a table read ([#57](#57)) by @takaebato ### Other Changes - changelog breaking-change workflow, version-bump docs, and keywords ([#51](#51)) by @takaebato - tidy keywords, README versions, and add a version-sync check ([#46](#46)) by @takaebato </blockquote> ## `sql-insight-cli` <blockquote> ## [0.2.2](sql-insight-cli-v0.2.1...sql-insight-cli-v0.2.2) - 2026-07-05 ### Added - *(cli)* prebuilt binary distribution — cargo binstall, completions, man, and provenance ([#53](#53)) by @takaebato ### Fixed - *(deps)* update rust crate clap_mangen to 0.3 ([#54](#54)) by @renovate[bot] ### Other Changes - changelog breaking-change workflow, version-bump docs, and keywords ([#51](#51)) by @takaebato - tidy keywords, README versions, and add a version-sync check ([#46](#46)) by @takaebato </blockquote> </p></details> --- This PR was generated with [release-plz](https://github.com/release-plz/release-plz/). --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Takahiro Ebato <takahiro.ebato@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
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
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.
Summary
Implements a follow-up noted in #55: Oracle's inline-view INSERT
(
INSERT INTO (SELECT … FROM emp) …) now resolves through to the view's singlebase table as the write target, instead of dropping the whole statement as an
unsupported target. Semantically the row lands in the base table — the subquery
is an unnamed updatable view that restricts columns / enforces a predicate.
What resolves (single-table view only):
INSERT INTO (SELECT a FROM emp) …writes
emp(table level:create_tables=[emp]).columns only;
a AS xwrites basea. A wildcard / expression projectionmakes positions indeterminate → the usual column-less path (catalog fill, or
InsertColumnsUnresolved). The explicit list also drives the arity check(
… (SELECT a FROM emp) VALUES (1, 2)→InsertColumnsArityMismatch).WITH CHECK OPTIONwould enforce; sqlparser cannot parse the
WITH CHECK OPTIONkeywords insidethe target, so the plain predicate is what surfaces): reads
emp.dept, nevera lineage source. Carried on a new internal
Insert::target_predicate.INSERT INTO (SELECT a FROM emp) SELECT x FROM s→s.x → emp.a.What stays flagged (
UnsupportedStatement, as before): everything but theminimal insertable-view shape (projection + FROM + WHERE). A join (Oracle's
key-preserved rules need a catalog), a set operation, a nested
WITH/ pipechain, or a non-table factor names no single base table — and any other clause
(GROUP BY / HAVING / DISTINCT / ORDER BY / FETCH / CONNECT BY / …) makes the
view non-insertable (ORA-01732) and could carry column refs that would
otherwise drop silently. Enforced by exhaustive
Query/Selectdestructures, so a new sqlparser clause forces a keep-or-reject decision.
Edge cases covered on review:
INSERT INTO (SELECT e.a FROM emp e WHERE e.dept = 10) …): the predicate'se.deptresolves through the alias toemp.dept(and, as in SQL, the alias shadows the bare table name).GROUP BY/ORDER BY/DISTINCT/FETCH)previously slipped through the shape check with their column refs silently
dropped — now rejected to the flag+drop path.
INSERT OVERWRITE/REPLACE INTObucketing (peel_to_insert) only readsthe flags — unaffected.
TableReference'sTryFrom<&Insert>sees through the same shapes (and errorson the rest), so the public target-identity parse agrees with the analysis.
Tests: 7 column-level cases (values / WHERE-reads / lineage-through-view /
alias / wildcard / expression / arity), a table-level CRUD case, a
TryFrom<&Insert>unit test, and the existing subquery-target diagnostic testnarrowed to a join view. All gates green (fmt / clippy /
test --all/ doc).Only
OracleDialectparses this syntax (supports_insert_table_query).🤖 Generated with Claude Code