diff --git a/docs/architecture.md b/docs/architecture.md index 8db9198..0a76fe4 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -102,8 +102,9 @@ All reach [`execute`](../src/execution/mod.rs), which runs output step. Encoding stays in `output/`. Before scanning for output, `Execution::prepare` resolves the catalog entry of every planned table, checks required tables and renderable columns, and performs the optional -`parentBlockHash` check. Relation targets are required only when there -are source rows to follow; an empty source does not open its target. +`parentBlockHash` check. A relation target is required like every other +planned table, also when no source row reaches it; an empty source only spares +its scan. ### Where payload deferral is chosen @@ -152,7 +153,7 @@ only their own source items without evaluating those predicates again. when possible. Otherwise it applies the equality or hierarchical masks from [`join/`](../src/join/mod.rs) after scanning. A pushed-down relation needs no second join over its result. A relation whose followed rows all have a null in -the key or address relates to nothing and never opens its target; when a key +the key or address relates to nothing and never scans its target; when a key column cannot be read at all, the join after the scan is what reports it. [`compute_block_weights`](../src/output/weight.rs) accounts for selected fields diff --git a/spec/07-invariants.md b/spec/07-invariants.md index 7b8ed54..23cfa25 100644 --- a/spec/07-invariants.md +++ b/spec/07-invariants.md @@ -569,29 +569,32 @@ data is read. No output precedes any error. ### INV-E3 **A selected column absent from the chunk is an error** (`ColumnNotFound`), not a -`null`. +`null`. It is an error whether or not a row of its table is read: a relation +with no row to follow reads nothing of its target, and still needs every column +the query selects from it. *Why:* `null` tells the client "this transaction had no `sighash`". The truth is "this chunk predates `sighash`". Those call for different actions. ### INV-E4 -**A missing table is an error** (`TableNotFound`) — at the moment the query -needs it, and not before. +**A missing table is an error** (`TableNotFound`), raised before any row is +read. -The block table and every table with an item request are needed by every -query that names them, whether or not any row matches. A relation's target is -needed only once the relation has a row to follow: a relation with no inputs -never opens its target, so its absence is not observed, and the query answers -as it would on a complete chunk. +Every table the query requests is needed: the block table, every table with an +item request and every relation's target. This holds whether or not any row +matches, and whether or not the relation has a row to follow. The engine MUST NOT open a table the query does not name. What else the chunk directory holds — a stray file, a table of another kind too damaged to read — is not the query's business. A table the query does open and cannot read is `MalformedChunkData`. -*Why:* a chunk that predates a table is a chunk with nothing in it, and a query -that would not have touched the table has no reason to fail. Opening every file -in the directory up front turns each of them into a way to fail a query that -asked for none of it, and charges a header-only query for every footer parse. +*Why:* a dataset has a table or it does not. Checked only where a relation has +a row to follow, the answer for a chunk depends on its data: a stream over a +dataset without the table returns pages until one has a row to follow, and then +fails. [INV-E3](#inv-e3) holds a selected column to the same rule. Opening every +file in the directory up front turns each of them into a way to fail a query +that asked for none of it, and charges a header-only query for every footer +parse. ### INV-E5 **Fork detection.** When `parentBlockHash` is supplied and the chunk knows the diff --git a/spec/08-conformance.md b/spec/08-conformance.md index d495104..401fa4a 100644 --- a/spec/08-conformance.md +++ b/spec/08-conformance.md @@ -241,9 +241,10 @@ panic ([INV-E1](07-invariants.md#inv-e1)): |---|---| | Drop a column that a query filters on | `ColumnNotFound` ([INV-X3](07-invariants.md#inv-x3)) | | Drop a column that a query selects | `ColumnNotFound` ([INV-E3](07-invariants.md#inv-e3)) | +| Drop a column a query selects from a relation's target, when no row points at it | `ColumnNotFound` ([INV-E3](07-invariants.md#inv-e3)) | | Drop a column nothing mentions | Unchanged output ([INV-X2](07-invariants.md#inv-x2)) | | Drop a whole table a query needs | `TableNotFound` ([INV-E4](07-invariants.md#inv-e4)) | -| Drop a relation's target when no row points at it | Unchanged output ([INV-E4](07-invariants.md#inv-e4)) | +| Drop a relation's target when no row points at it | `TableNotFound` ([INV-E4](07-invariants.md#inv-e4)) | | Add a stray or unreadable file nothing names | Unchanged output ([INV-E4](07-invariants.md#inv-e4)) | | Widen `block_number` from 32 to 64 bits | Unchanged output ([INV-D7](07-invariants.md#inv-d7)) | | Store an item index as signed | Unchanged output ([INV-D7](07-invariants.md#inv-d7)) | @@ -393,8 +394,8 @@ renamed out from under a comment inside it. | [INV-O14](07-invariants.md#inv-o14) | CT-6 | **C** | six Arrow-parity cases; `a_rolled_header_field_is_shipped_in_arrow`; `binary_arrow_carries_every_hex_value_json_does` renders every hex field of every fixture chunk both ways and compares the values row for row, `binary_arrow_keeps_an_odd_length_hex_value` and `binary_arrow_refuses_a_hex_column_holding_something_else` the two cases a decoder can get wrong | | [INV-E1](07-invariants.md#inv-e1) | CT-9 | **P** | the request half is fuzzed under CT-9, and `a_chunk_that_disagrees_with_the_catalog_does_not_panic` pins the encoders against a chunk written to disagree. `a_block_number_nothing_can_place_is_refused_on_every_path` adds the corrupt-value half for the one column every layer places a row by, and asserts the refusal is the same from an unbounded scan, a bounded range, a relation pull, a hierarchical pull, a range that excludes the offending row — a chunk that errors on one shape and under-answers on another is worse than either. It runs both writings of that chunk, one stating a null count and one stating none, since a check reading the count off the metadata says nothing about the file that omits it. `a_reader_that_leaves_out_item_matches_fails_the_query` covers a chunk reader outside the engine that answers a relation's scan without the item matches it follows. The chunk-type *sweep* the invariant asks for needs HC-3, and two existing tests still assert a panic rather than forbid one | | [INV-E2](07-invariants.md#inv-e2) | CT-2 | **P** | validation precedes scanning by construction; nothing asserts that no output precedes an error | -| [INV-E3](07-invariants.md#inv-e3) | CT-8 | **C** | `selecting_an_absent_column_is_an_error`; `a_roll_field_with_a_missing_source_is_an_error` for a source of a roll field, which is positional and cannot be tolerated; `a_selected_column_nothing_renders_is_an_error` for the other way a chunk can fail to carry a selected value — stored at a type no encoder reads, refused off the schema so a range that reaches no row refuses too; `a_variant_column_nothing_reads_is_an_error` for the column that picks a row's groups; `a_sort_key_nothing_orders_is_an_error` for a key the output orders by but does not render; `the_arrow_output_ships_what_json_cannot_render` that the refusal is the JSON encoder's, not the chunk's | -| [INV-E4](07-invariants.md#inv-e4) | CT-8 | **C** | `a_missing_table_is_an_error`, `a_missing_relation_table_is_an_error`, and `a_missing_block_table_is_an_error` cover every way a plan names a table; `a_missing_relation_table_is_not_observed_without_inputs`, `only_the_tables_a_query_names_are_opened` and `a_relation_query_asks_only_for_the_tables_it_names` cover when it is not asked for | +| [INV-E3](07-invariants.md#inv-e3) | CT-8 | **C** | `selecting_an_absent_column_is_an_error`; `a_relation_target_missing_a_selected_column_is_an_error_without_inputs` for a relation with no row to follow, which reads nothing of its target; `a_roll_field_with_a_missing_source_is_an_error` for a source of a roll field, which is positional and cannot be tolerated; `a_selected_column_nothing_renders_is_an_error` for the other way a chunk can fail to carry a selected value — stored at a type no encoder reads, refused off the schema so a range that reaches no row refuses too; `a_variant_column_nothing_reads_is_an_error` for the column that picks a row's groups; `a_sort_key_nothing_orders_is_an_error` for a key the output orders by but does not render; `the_arrow_output_ships_what_json_cannot_render` that the refusal is the JSON encoder's, not the chunk's | +| [INV-E4](07-invariants.md#inv-e4) | CT-8 | **C** | `a_missing_table_is_an_error`, `a_missing_relation_table_is_an_error`, and `a_missing_block_table_is_an_error` cover every way a plan names a table, and `a_missing_relation_table_is_an_error_without_inputs` every way a relation can have no row to follow; `only_the_tables_a_query_names_are_opened` and `a_relation_query_asks_only_for_the_tables_it_names` cover when it is not asked for | | [INV-E5](07-invariants.md#inv-e5) | CT-2, CT-7 | **C** | the cases in `ct2_request/fork.rs`, including a chain that skips block numbers, a gap wider than `P-FORK-WINDOW`, and a `fromBlock` on a skipped number: every slot of the Solana fixture, and synthetic gaps at four parent-number widths, across row groups and across either end of the chunk. `the_fork_check_answers_the_same_as_the_reference` compares verdict and message with the reference on seven fixture chunks | | [INV-E6](07-invariants.md#inv-e6) | CT-2 | **C** | `every_validation_error_carries_its_kind` is the §6.2 table, one row per kind; `every_request_bound_carries_its_kind` and `an_unanswerable_reserved_key_carries_its_kind` cover the four that need a request too large to write inline or a catalog of their own; `the_fork_error_message_is_pinned` covers the one fixed message | | [INV-E7](07-invariants.md#inv-e7) | CT-4, CT-8 | **C** | `test_semi_join_unsupported_key_type` for a join key. For a filter column, `a_filter_on_an_uncomparable_column_is_an_error` retypes a discriminator, a flag, an index and a program id under every filter kind that reaches a column — a program id stored as bytes among them, which read as UTF-8 would answer "no rows" — and asserts the kind; `an_uncomparable_column_is_refused_by_every_predicate_kind` and `test_list_contains_any_on_a_non_list_is_refused` the predicates underneath, and `the_stored_type_check_agrees_with_evaluation` that the pre-read probe and the evaluation are one answer. The other direction — a type that is comparable and used to panic — is `a_bloom_and_a_dictionary_of_any_key_width_are_read`, with `a_bloom_reads_every_bytes_type` and `a_dictionary_keeps_its_null_keys_out_of_the_answer` underneath | diff --git a/src/execution/prepare.rs b/src/execution/prepare.rs index 7d74aea..a5b0141 100644 --- a/src/execution/prepare.rs +++ b/src/execution/prepare.rs @@ -108,9 +108,10 @@ impl<'a> Execution<'a> { } } - /// The tables every query scans: the block table and each table with an item - /// request. A relation's target is not among them — it is opened only when the - /// relation has rows to follow, and its absence is reported there (INV-E4). + /// Every table the query names: the block table, each table with an item + /// request and each relation's target. A relation with no row to follow still + /// needs its target, or a chunk's answer would depend on whether a row + /// matched (INV-E4). fn ensure_required_tables_present(&self) -> Result<()> { let ensure_present = |table: &str| { crate::engine_ensure!( @@ -125,6 +126,9 @@ impl<'a> Execution<'a> { ensure_present(&self.plan.block_table)?; for table in &self.tables { ensure_present(&table.plan.table)?; + for relation in &table.plan.relations { + ensure_present(&relation.target_table)?; + } } Ok(()) @@ -147,13 +151,13 @@ impl<'a> Execution<'a> { sized.then_some(table) } - /// Refuse a chunk that stores a selected column at a type the output cannot - /// use, before any row is read (INV-E3). + /// Refuse a chunk that lacks a selected column or stores one at a type the + /// output cannot use, before any row is read (INV-E3). /// /// Off the schema rather than off the batches, so that the same chunk refuses - /// whatever the query: the writers resolve against batches, and a range that - /// reaches no row of a table resolves nothing. A column the chunk lacks is not - /// this check's business — the scan raises `ColumnNotFound` for it. + /// whatever the query: the writers resolve against batches, a range that + /// reaches no row of a table resolves nothing, and a relation with no row to + /// follow scans nothing. /// /// What "use" means depends on the format. JSON renders through an encoder, /// orders rows by the sort keys and groups them by the variant tag; Arrow ships @@ -183,7 +187,12 @@ impl<'a> Execution<'a> { for &column in projection.required() { let Ok(field) = schema.field_with_name(column) else { - continue; + crate::engine_bail!( + crate::error::ErrorKind::ColumnNotFound, + "column '{}' is not found in '{}'", + column, + table + ); }; let declared = desc.columns.get(column); diff --git a/tests/conformance/ct8_adversarial.rs b/tests/conformance/ct8_adversarial.rs index ea290c4..40cf873 100644 --- a/tests/conformance/ct8_adversarial.rs +++ b/tests/conformance/ct8_adversarial.rs @@ -12,11 +12,14 @@ use arrow::datatypes::{DataType, Field, SchemaRef, TimeUnit}; use sqd_query_engine::error::{error_kind, ErrorKind}; +use sqd_query_engine::metadata::DatasetDescription; use sqd_query_engine::output::{execute_chunk, execute_chunk_arrow, execute_plan}; use sqd_query_engine::query::{compile, parse_query, Plan}; use sqd_query_engine::scan::{ChunkReader, ParquetChunkReader, ScanRequest, Scanned}; use std::collections::BTreeSet; +use std::path::Path; use std::sync::{Arc, Mutex}; +use tempfile::TempDir; use crate::harness::chunk::{ chunk_with_column_retyped, chunk_with_nullable_column, chunk_without_column, @@ -94,39 +97,131 @@ fn a_missing_relation_table_is_an_error() { ); } -/// A relation with nothing to follow never opens its target, so a chunk missing -/// that table answers a query whose primary scan is empty the way a complete -/// chunk does — and refuses the same query the moment the relation has inputs. -/// -/// Covers CT-8 · INV-E4 -#[test] -fn a_missing_relation_table_is_not_observed_without_inputs() { - let metadata = catalog(); - let source = weighted_chunk(BLOCKS, &uniform(&[10, 11], 0), &uniform(BLOCKS, 0)); - let chunk = chunk_without_table(source.path(), "transactions"); - let query = |from: u64, to: u64| { +/// A query whose relation has no row to follow, on a chunk that holds every +/// table and column the query names. +struct NothingToFollow { + case: &'static str, + metadata: DatasetDescription, + chunk: TempDir, + target: &'static str, + /// A column of `target` the query selects. + selected: &'static str, + query: String, +} + +/// Each way a relation can be left with no row to follow. +fn nothing_to_follow() -> Vec { + let past_the_logs = |include_all_blocks: bool| { serde_json::json!({ "type": "test", - "fromBlock": from, - "toBlock": to, - "includeAllBlocks": true, + "fromBlock": 12, + "toBlock": 14, + "includeAllBlocks": include_all_blocks, "logs": [{"transaction": true}], "fields": {"block": {"number": true}, "transaction": {"input": true}} }) .to_string() }; + let synthetic = |case, query| NothingToFollow { + case, + metadata: catalog(), + chunk: weighted_chunk(BLOCKS, &uniform(&[10, 11], 0), &uniform(BLOCKS, 0)), + target: "transactions", + selected: "input", + query, + }; - answers_the_same( - &metadata, - &query(12, 14), - source.path(), - chunk.path(), - "dropping a relation target no row points at", - ); + let transactions = |items: serde_json::Value| { + serde_json::json!({ + "type": "test", + "fromBlock": 100, + "toBlock": 115, + "transactions": items, + "fields": {"transaction": {"transactionIndex": true}, "trace": {"kind": true}} + }) + .to_string() + }; + let no_such_index = serde_json::json!({"transactionIndex": [9], "transactionTraces": true}); + let evm = |case, query| NothingToFollow { + case, + metadata: evm_like::catalog(), + chunk: evm_like::chunk(), + target: "traces", + selected: "kind", + query, + }; - let err = run_against(&metadata, chunk.path(), &query(10, 11)) - .expect_err("a relation with inputs must observe its missing target"); - assert_eq!(error_kind(&err), Some(ErrorKind::TableNotFound)); + vec![ + synthetic( + "a range with no source rows, every block", + past_the_logs(true), + ), + synthetic("a range with no source rows", past_the_logs(false)), + evm( + "a filter no source row matches", + transactions(serde_json::json!([no_such_index])), + ), + evm( + "the only item asking for the relation matches nothing", + transactions(serde_json::json!([{"transactionIndex": [0]}, no_such_index])), + ), + ] +} + +impl NothingToFollow { + /// Assert that both formats refuse the query on `chunk` with `kind`, naming + /// `missing`, where the complete chunk answers it. + fn assert_refused(&self, chunk: &Path, kind: ErrorKind, missing: &str) { + let case = self.case; + let complete = run_against(&self.metadata, self.chunk.path(), &self.query).unwrap(); + assert_eq!( + count_items(&complete, self.target), + 0, + "{case}: the relation must have nothing to follow for the check to mean anything" + ); + + let parsed = parse_query(self.query.as_bytes(), &self.metadata).unwrap(); + let plan = compile(&parsed, &self.metadata).unwrap(); + let reader = ParquetChunkReader::open(chunk).unwrap(); + let json = execute_chunk(&plan, &self.metadata, &reader, false).err(); + let arrow = execute_chunk_arrow(&plan, &self.metadata, &reader, false, false).err(); + + for (format, err) in [("json", json), ("arrow", arrow)] { + let err = + err.unwrap_or_else(|| panic!("{case}, {format}: answered without '{missing}'")); + assert_eq!(error_kind(&err), Some(kind), "{case}, {format}: {err:#}"); + assert!( + err.root_cause().to_string().contains(missing), + "{case}, {format}: the error must name '{missing}', got: {}", + err.root_cause() + ); + } + } +} + +/// A dataset has a table or it does not. A relation target the chunk lacks is +/// refused also on a page where no row points at it: refused only where one +/// does, a stream over such a dataset returns its first pages and then fails. +/// +/// Covers CT-8 · INV-E4 +#[test] +fn a_missing_relation_table_is_an_error_without_inputs() { + for case in nothing_to_follow() { + let chunk = chunk_without_table(case.chunk.path(), case.target); + case.assert_refused(chunk.path(), ErrorKind::TableNotFound, case.target); + } +} + +/// A column the relation target lacks is refused, for the same reason, also +/// where no row points at the target. +/// +/// Covers CT-8 · INV-E3 +#[test] +fn a_relation_target_missing_a_selected_column_is_an_error_without_inputs() { + for case in nothing_to_follow() { + let chunk = chunk_without_column_at(case.chunk.path(), case.target, case.selected); + case.assert_refused(chunk.path(), ErrorKind::ColumnNotFound, case.selected); + } } /// Only the tables a query names are opened. A file the query does not touch —