Skip to content

feat: resolve Oracle inline-view INSERT targets to their base table - #58

Merged
takaebato merged 1 commit into
masterfrom
feat/insert-subquery-target-resolution
Jul 4, 2026
Merged

feat: resolve Oracle inline-view INSERT targets to their base table#58
takaebato merged 1 commit into
masterfrom
feat/insert-subquery-target-resolution

Conversation

@takaebato

@takaebato takaebato commented Jul 4, 2026

Copy link
Copy Markdown
Owner

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 single
base 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):

  • Write target = the view's base table: INSERT INTO (SELECT a FROM emp) …
    writes emp (table level: create_tables=[emp]).
  • Target columns = the view projection, as an explicit column list — plain
    columns only; a AS x writes base a. A wildcard / expression projection
    makes 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).
  • View WHERE = filter reads against the target (what a WITH CHECK OPTION
    would enforce; sqlparser cannot parse the WITH CHECK OPTION keywords inside
    the target, so the plain predicate is what surfaces): reads emp.dept, never
    a lineage source. Carried on a new internal Insert::target_predicate.
  • Column lineage traces through the view:
    INSERT INTO (SELECT a FROM emp) SELECT x FROM ss.x → emp.a.

What stays flagged (UnsupportedStatement, as before): everything but the
minimal insertable-view shape (projection + FROM + WHERE). A join (Oracle's
key-preserved rules need a catalog), a set operation, a nested WITH / pipe
chain, 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 / Select
destructures, so a new sqlparser clause forces a keep-or-reject decision.

Edge cases covered on review:

  • An aliased base table (INSERT INTO (SELECT e.a FROM emp e WHERE e.dept = 10) …): the predicate's e.dept resolves through the alias to
    emp.dept (and, as in SQL, the alias shadows the bare table name).
  • Clause-bearing views (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 INTO bucketing (peel_to_insert) only reads
    the flags — unaffected.

TableReference's TryFrom<&Insert> sees through the same shapes (and errors
on 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 test
narrowed to a join view. All gates green (fmt / clippy / test --all / doc).

Only OracleDialect parses this syntax (supports_insert_table_query).

🤖 Generated with Claude Code

@github-actions

github-actions Bot commented Jul 4, 2026

Copy link
Copy Markdown
Contributor

✅ PR title follows the Conventional Commits spec.

@codecov

codecov Bot commented Jul 4, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.74312% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 94.77%. Comparing base (9d76891) to head (aef20c2).

Files with missing lines Patch % Lines
sql-insight/src/reference.rs 88.52% 6 Missing and 1 partial ⚠️
sql-insight/src/resolver/binder/statement.rs 95.74% 2 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

`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
takaebato force-pushed the feat/insert-subquery-target-resolution branch from c6bd3c2 to aef20c2 Compare July 4, 2026 09:56
@takaebato
takaebato merged commit abf1d87 into master Jul 4, 2026
14 checks passed
@takaebato
takaebato deleted the feat/insert-subquery-target-resolution branch July 4, 2026 12:50
@github-actions github-actions Bot mentioned this pull request Jul 4, 2026
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant