Skip to content

adapter: report structured SQLSTATEs for dataflow-execution evaluation errors - #36967

Closed
antiguru wants to merge 5 commits into
MaterializeInc:mainfrom
antiguru:claude/magical-dijkstra-ZemyU
Closed

adapter: report structured SQLSTATEs for dataflow-execution evaluation errors#36967
antiguru wants to merge 5 commits into
MaterializeInc:mainfrom
antiguru:claude/magical-dijkstra-ZemyU

Conversation

@antiguru

@antiguru antiguru commented Jun 10, 2026

Copy link
Copy Markdown
Member

Motivation

Closes SQL-347.
Evaluation errors reported XX000 when produced during dataflow execution (reading a collection or index), while constant folding already mapped the same errors to precise class-22 codes.
This completes the dataflow half of the SQLSTATE fix; the constant-eval half (SQL-326) landed in #36814.

Description

PeekResponse::Error now carries a structured PeekError across the compute → adapter boundary instead of a bare string: Dataflow(DataflowError) for evaluation errors and Internal(String) for peek-machinery failures with no structured form.
The wire protocol is bincode, so no proto changes are needed.
The adapter maps PeekError::Dataflow to a new AdapterError::Dataflow, whose code routes the inner error through dataflow_error_code/eval_error_code (the same mapping constant folding uses), while its Display keeps the existing message.
The structure is preserved on every peek path: the index error-trace, the MFP iterator, the persist fast-path do_peek, and the peek-stash channel.
pgwire send-rows/FETCH and the HTTP/WS adapters render the error via into_response/SqlResult::err instead of hardcoding INTERNAL_ERROR.

The issue suggested dropping the Evaluation error: prefix to match the constant path.
This PR deliberately keeps the prefix, so dataflow evaluation errors gain the correct SQLSTATE without changing their message text, avoiding churn on the ~15 .slt/.td assertions that match Evaluation error: ....

@antiguru
antiguru requested review from a team as code owners June 10, 2026 17:05
@antiguru
antiguru requested review from aljoscha and def- June 10, 2026 19:20
@antiguru
antiguru force-pushed the claude/magical-dijkstra-ZemyU branch from 01bf360 to e96f908 Compare June 11, 2026 15:53
claude and others added 5 commits June 12, 2026 16:40
…STATEs

Evaluation errors report different SQLSTATEs depending on where they
arise. Constant folding maps the `EvalError` through `eval_error_code`
and gets a precise class-22 code (e.g. `SELECT 1/0` -> 22012). The same
error produced while reading a collection (e.g. `SELECT 1/0 FROM t`) was
stringified early in the compute layer into `PeekResponse::Error(String)`,
became `AdapterError::Unstructured`, and reported XX000 with an
`Evaluation error:` prefix.

Carry the structured error across the compute -> adapter boundary instead
of a bare string:

* `PeekResponse::Error` (and the peek-stash channel, the persist fast-path
  `do_peek`, and the index error-trace / MFP iterator) now carry a new
  `PeekError` sum type: `Dataflow(DataflowError)` for structured evaluation
  errors, `Internal(String)` for peek-machinery failures with no structured
  form. The wire protocol is bincode, so no proto changes are needed.
* `PeekResponseUnary::Error` carries an `AdapterError`. At the boundary a
  `DataflowError::EvalError` becomes `AdapterError::Eval`, so it receives
  the same SQLSTATE constant folding produces and sheds the
  `Evaluation error:` prefix; everything else stays `Unstructured` (XX000).
* The pgwire send-rows and FETCH paths, and the HTTP/WS adapters, now render
  the error via `into_response`/`SqlResult::err` rather than hardcoding
  `INTERNAL_ERROR`.

Adds `test_dataflow_error_codes` covering the fast-path peek and the
materialized-view error-trace path.

https://claude.ai/code/session_013pDom9j3DoUvAVztRk4pAe
…zero

`SELECT 1/0 FROM mz_sources` is constant-folded into a literal eval
error evaluated in the fast-path peek, so it now reports the structured
22012 (with the `Evaluation error:` prefix from DataflowError) instead
of XX000.

https://claude.ai/code/session_013pDom9j3DoUvAVztRk4pAe
Introduce `AdapterError::Dataflow`, which keeps the existing dataflow
error message (including the `Evaluation error:` prefix that
`DataflowError`'s Display adds) while `code` derives a precise SQLSTATE
from the inner error via `dataflow_error_code`/`eval_error_code`. This
avoids collapsing into `AdapterError::Eval`, which would have changed the
message of every dataflow evaluation error.

https://claude.ai/code/session_013pDom9j3DoUvAVztRk4pAe
`test_dataflow_error_codes` issued `CREATE TABLE` and `INSERT` in a
single `batch_execute`, which forms an implicit transaction block; DDL
cannot run inside one, so the test failed at setup with E25001. Run the
statements separately.

Also apply `cargo fmt` to the `PeekError` call sites introduced earlier
on this branch.
The FETCH path rendered `PeekResponseUnary::DependencyDropped` via
`to_concurrent_dependency_drop()`, changing the message from "query could
not complete because relation ... was dropped" to "relation '...' was
dropped". Restore `query_terminated_error()` so the message and SQLSTATE
are unchanged; this rendering is out of scope for the evaluation-error
SQLSTATE work and broke `test_subscribe_on_dropped_source`.
@antiguru
antiguru force-pushed the claude/magical-dijkstra-ZemyU branch from e96f908 to cf1da4d Compare June 12, 2026 14:45
@antiguru antiguru closed this Aug 24, 2026
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.

2 participants