From 5d00bfbd1a65b514fb2ad771dc5c2207e49c45f6 Mon Sep 17 00:00:00 2001 From: TimelordUK Date: Mon, 31 Aug 2026 14:22:04 +0100 Subject: [PATCH] fix(engine): resolve ORDER BY to the Nth output column (P16) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ORDER BY 2` returned rows in natural insertion order, with no error. The integer was not being ignored — `OrderByAliasTransformer` promotes any ORDER BY item that is not a visible column into a hidden SELECT item so it survives projection, and `NumberLiteral("2")` qualified. So the query sorted, correctly, on a hidden column whose value is the constant 2, where every row compares equal. Row count gives no hint of this: `ORDER BY 2 DESC LIMIT 3` returns three rows that look plausible but are the first three in file order. The ordinal is positional against the *output* columns, so it is resolved in `query_engine::apply_multi_order_by_with_context` rather than in the transformer. The transformer knows the select list but not the output: `SELECT *` is still unexpanded there and GROUP BY has not run, and both are shapes where `ORDER BY 2` has to keep meaning the same thing. The transformer's only job is to stop promoting numeric literals. Rules pinned against DuckDB before implementing, not assumed: - `ORDER BY 2` — 2nd output column, under an explicit select list, `SELECT *`, or after GROUP BY. - `ORDER BY 1+1` — NOT an ordinal. It is an ordinary constant expression and sorts nothing, in both engines. - `ORDER BY 0` / `-1` / out of range — error. - `ORDER BY 1.5` — error. This is why the transformer skips every numeric literal rather than only integer-valued ones: promoting `1.5` would leave it a silent no-op, and only the engine knows the valid range to report. Same principle as P13 stage 1 — a refusal beats a different query that succeeds. Columns promoted for ORDER BY visibility (and HAVING's `__hidden_agg_` columns) are appended after the real output, so they are excluded from the ordinal range: `SELECT a, b FROM t ORDER BY c, 3` errors rather than landing on `__hidden_orderby_1`. Parity 134 -> 139 AGREE (+2 fixed, +3 new coverage, +1 new BOTH_ERR). `order_by_ordinal_star` and `order_by_ordinal_group_by` were added because the defect is about output columns, so each construct that changes what those are is a separate risk — and "top N by total" is where silently returning group order was most likely to be believed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01TmQRCdZUn3RYqyVFoeKRsY --- docs/SQL_PARITY.md | 58 ++++++++- src/data/query_engine.rs | 58 +++++++++ src/query_plan/order_by_alias_transformer.rs | 116 +++++++++++++++++- tests/comparison/corpus/08_ordering.toml | 55 +++++++-- tests/test_multi_column_order_by.rs | 119 +++++++++++++++++++ 5 files changed, 394 insertions(+), 12 deletions(-) diff --git a/docs/SQL_PARITY.md b/docs/SQL_PARITY.md index 8e4113e2..3ee5c481 100644 --- a/docs/SQL_PARITY.md +++ b/docs/SQL_PARITY.md @@ -82,7 +82,8 @@ Suggested fix order, by silent blast radius: | ~~4~~ | ~~[P28](#p28) `INTO #tmp` stages unfiltered rows~~ | ✅ **Fixed 2026-08-16** — turned out to stage the *whole source table*, and the sweep it prescribed found [P31](#p31) | | ~~7~~ | ~~[P18](#p18)/[P19](#p19) three-valued logic~~ | ✅ **Fixed 2026-08-22** — 125 → 129 AGREE. Delivered as three slices ([R10](ENGINE_REFACTORING.md#r10)); the two no-op ones landed first, so the semantics change reviewed on its own | | ~~8~~ | ~~[P24](#p24) `RANGE` treated as `ROWS`~~ | ✅ **Fixed 2026-08-30** — 129 → 133 AGREE (+2 fixed, +2 new coverage). One defect, not two: the parser already emitted the right default frame, so fixing peer groups closed both cases. Spun off [P33](#p33) | -| 9 | [P14](#p14), [P16](#p16), [P17](#p17), [P20](#p20), [P23](#p23), P13 stage 2 | Smaller, self-contained, decisions already taken | +| ~~9a~~ | ~~[P16](#p16) `ORDER BY ` ignored~~ | ✅ **Fixed 2026-08-31** — 134 → 139 AGREE. The literal was being promoted into a hidden *constant* column, so the sort ran on a column where every row tied | +| 9b | [P14](#p14), [P17](#p17), [P20](#p20), [P23](#p23), P13 stage 2 | Smaller, self-contained, decisions already taken | | 10 | [P22](#p22), [P25](#p25), [P26](#p26), [P15](#p15), [P32](#p32) | Hard errors — visible, so less urgent than any of the above | | — | [P27](#p27) `OR` in `JOIN ... ON` | **Reclassified 2026-08-08 — not a quick win.** `JoinCondition` is a `Vec` implicitly AND-ed, so there is nowhere in the AST to put an `OR`; it needs join conditions to become an expression, which reaches the join execution code. Sequence it with the R-log, not here | @@ -629,9 +630,13 @@ annotation be removed. silently or loudly depending only on luck. ### P16 — `ORDER BY ` is silently ignored -- **Status:** 🔴 OPEN +- **Status:** 🟢 FIXED 2026-08-31 — 134 → 139 AGREE (+2 fixed, +3 new coverage, + +1 new BOTH_ERR) - **Corpus:** `08_ordering.toml :: order_by_ordinal`, `order_by_ordinal_desc` - (both DIFFER). + (were DIFFER, now AGREE). Added with the fix: `order_by_ordinal_star`, + `order_by_ordinal_group_by`, + `order_by_ordinal_expression_is_not_positional` (AGREE) and + `order_by_ordinal_out_of_range` (BOTH_ERR). - **Observed:** `ORDER BY 2` and `ORDER BY 2 DESC` return rows in **natural insertion order** — no sorting is applied at all, and no error is raised. The integer is evaluated as a constant expression, so every row compares equal. @@ -649,6 +654,53 @@ annotation be removed. were the first three in file order rather than the top three. Row count is not evidence of correct ordering. +#### As built (2026-08-31) + +The root cause was not in the sort at all, and not where the entry above guessed +("the integer is evaluated as a constant expression"). It is close, but the +mechanism matters for where the fix goes: +`OrderByAliasTransformer::promote_hidden_order_by_columns` exists to keep an +ORDER BY key alive through projection, and it promotes **anything that is not a +visible column** into a hidden SELECT item. `NumberLiteral("2")` is not a +column, so it was promoted as a hidden column *whose value is the constant 2* — +after which the engine sorted, correctly, on a column where every row compares +equal. Nothing was ignored; the wrong thing was sorted on. + +**The ordinal is resolved during execution, not in the transformer.** The +transformer knows the select list but not the *output* columns, and the two +differ in exactly the cases that matter: `SELECT *` is still unexpanded there, +and GROUP BY has not run. `apply_multi_order_by_with_context` in +`query_engine.rs` sees the projected view and resolves all three shapes with one +rule, so the transformer's only job is to stop promoting numeric literals. + +Rules pinned against DuckDB before implementing, rather than assumed: + +| Query | Behaviour | +|---|---| +| `ORDER BY 2` | 2nd output column — under an explicit select list, `SELECT *`, or after GROUP BY | +| `ORDER BY 1+1` | **not** an ordinal: an ordinary constant expression, sorts nothing | +| `ORDER BY 0`, `ORDER BY -1`, `ORDER BY 3` (of 2) | error, `should be between 1 and N` | +| `ORDER BY 1.5` | error — DuckDB: *"ORDER BY non-integer literal has no effect"* | + +The last row is why the transformer skips **every** numeric literal and not just +integer-valued ones: leaving `1.5` to be promoted would have kept it a silent +no-op, and only the engine knows the valid range to report. That is the P13 +principle — a refusal beats a different query that succeeds. + +**Hidden columns are excluded from the ordinal range.** Columns promoted for +ORDER BY visibility (and HAVING's `__hidden_agg_` columns) are appended *after* +the real output, so `SELECT a, b FROM t ORDER BY c, 3` must error rather than +resolve to `__hidden_orderby_1`. Pinned by +`test_order_by_ordinal_ignores_promoted_hidden_columns`. + +**A note on picking the corpus cases.** Two cases would have been enough to turn +the entry green, and would have left the two most valuable shapes untested: the +defect is about *output* columns, so every construct that changes what the +output columns are is a separate risk. `SELECT *` and GROUP BY were added for +that reason, and `order_by_ordinal_group_by` is the one that matters most in +practice — "top N by total" is where silently returning group order is most +likely to be believed. + ### P17 — Default NULL placement differs on `ASC` - **Status:** 🔴 OPEN — **decision made 2026-08-02: follow the reference engine (NULLS LAST in both directions), plus explicit `NULLS FIRST`/`LAST` from P13 diff --git a/src/data/query_engine.rs b/src/data/query_engine.rs index e5c94038..82ed732e 100644 --- a/src/data/query_engine.rs +++ b/src/data/query_engine.rs @@ -3135,6 +3135,55 @@ impl QueryEngine { Ok(view.with_rows(unique_row_indices)) } + /// Resolve `ORDER BY ` - a 1-based positional reference to a select-list + /// item - to a source column index (P16). + /// + /// The reference is against the *output* columns, so it is resolved here + /// rather than in `OrderByAliasTransformer`: by this point projection, + /// `SELECT *` expansion and GROUP BY have all run, and `ORDER BY 2` means + /// the same thing in every one of those shapes. + /// + /// Columns promoted for ORDER BY visibility (and HAVING's hidden + /// aggregates) are appended *after* the real output, so they are excluded + /// from both the mapping and the range check - `ORDER BY 3` must not + /// silently land on `__hidden_orderby_1`. + /// + /// Following DuckDB: out-of-range is an error, and a non-integer literal is + /// an error too rather than a sort that quietly does nothing. + fn resolve_order_by_ordinal(view: &DataView, literal: &str) -> Result { + use crate::query_plan::having_alias_transformer::HIDDEN_AGG_PREFIX; + use crate::query_plan::order_by_alias_transformer::HIDDEN_ORDERBY_PREFIX; + + let output_columns: Vec = view + .get_display_columns() + .into_iter() + .filter(|&idx| { + view.source().columns.get(idx).is_none_or(|c| { + !c.name.starts_with(HIDDEN_ORDERBY_PREFIX) + && !c.name.starts_with(HIDDEN_AGG_PREFIX) + }) + }) + .collect(); + + let ordinal: i64 = literal.trim().parse().map_err(|_| { + anyhow!( + "ORDER BY {} has no effect: a non-integer literal is not a positional reference. Use a column name, or a position between 1 and {}", + literal, + output_columns.len() + ) + })?; + + if ordinal < 1 || ordinal as usize > output_columns.len() { + return Err(anyhow!( + "ORDER BY position {} is out of range - should be between 1 and {}", + ordinal, + output_columns.len() + )); + } + + Ok(output_columns[ordinal as usize - 1]) + } + /// Apply multi-column ORDER BY sorting to the view fn apply_multi_order_by( &self, @@ -3155,6 +3204,15 @@ impl QueryEngine { let mut sort_columns = Vec::new(); for order_col in order_by_columns { + // ORDER BY is positional, so it resolves against the + // projected output rather than by name (P16). + if let SqlExpression::NumberLiteral(literal) = &order_col.expr { + let col_index = Self::resolve_order_by_ordinal(&view, literal)?; + let ascending = matches!(order_col.direction, SortDirection::Asc); + sort_columns.push((col_index, ascending)); + continue; + } + // Extract column name from expression (currently only supports simple columns) let (column_name, is_quoted) = match &order_col.expr { SqlExpression::Column(col_ref) => ( diff --git a/src/query_plan/order_by_alias_transformer.rs b/src/query_plan/order_by_alias_transformer.rs index 4ac81078..44b2cc9f 100644 --- a/src/query_plan/order_by_alias_transformer.rs +++ b/src/query_plan/order_by_alias_transformer.rs @@ -161,6 +161,17 @@ impl OrderByAliasTransformer { } } + /// Is this ORDER BY item a bare numeric literal? + /// + /// Only a bare literal counts. `ORDER BY 1+1` is an ordinary constant + /// expression that sorts nothing, which is what DuckDB does too, so it is + /// deliberately not matched here. Non-integer literals are matched so that + /// `ORDER BY 1.5` reaches the engine and is rejected there, rather than + /// being promoted into a constant column and silently sorting nothing. + fn is_numeric_literal(expr: &SqlExpression) -> bool { + matches!(expr, SqlExpression::NumberLiteral(_)) + } + /// Check if an ORDER BY column matches an aggregate pattern /// Returns the normalized aggregate string if it matches fn extract_aggregate_from_order_column(column_name: &str) -> Option { @@ -333,6 +344,16 @@ impl OrderByAliasTransformer { } } + // ORDER BY (P16) is a positional reference to an output + // column, not an expression to compute. Promoting it would append a + // hidden *constant* column and sort on that - every row compares + // equal, so the sort silently becomes a no-op. Resolution needs the + // projected column list, which only exists once the query runs, so + // leave the literal in place for query_engine to resolve. + if Self::is_numeric_literal(&order_col.expr) { + continue; + } + // Determine the dedup key and clone the expression we'll promote. // For Column refs we use the column name (case-insensitive); for // arbitrary expressions we use the debug-formatted string. Two @@ -381,7 +402,7 @@ impl OrderByAliasTransformer { #[cfg(test)] mod tests { use super::*; - use crate::sql::parser::ast::{ColumnRef, QuoteStyle, SortDirection}; + use crate::sql::parser::ast::{ColumnRef, OrderByItem, QuoteStyle, SortDirection}; #[test] fn test_extract_aggregate_from_order_column() { @@ -424,6 +445,99 @@ mod tests { ); } + #[test] + fn ordinal_literal_is_not_promoted_to_a_hidden_column() { + // P16: promoting `ORDER BY 2` would append a hidden CONSTANT column and + // sort on it, so every row compares equal and the sort silently does + // nothing. The literal must survive to query_engine, which resolves it + // positionally against the projected output. + let mut stmt = SelectStatement::default(); + stmt.select_items = vec![ + SelectItem::Column { + column: ColumnRef::unquoted("id".to_string()), + leading_comments: Vec::new(), + trailing_comment: None, + }, + SelectItem::Column { + column: ColumnRef::unquoted("score".to_string()), + leading_comments: Vec::new(), + trailing_comment: None, + }, + ]; + stmt.order_by = Some(vec![OrderByItem { + expr: SqlExpression::NumberLiteral("2".to_string()), + direction: SortDirection::Desc, + }]); + + let stmt = OrderByAliasTransformer::new() + .transform_statement(stmt) + .expect("transform"); + + assert_eq!(stmt.select_items.len(), 2, "no hidden column appended"); + assert!(matches!( + stmt.order_by.as_ref().unwrap()[0].expr, + SqlExpression::NumberLiteral(ref n) if n == "2" + )); + } + + #[test] + fn non_integer_literal_also_reaches_the_engine() { + // `ORDER BY 1.5` is not positional, but it must still not be promoted: + // the engine rejects it, whereas a hidden constant column would make it + // a silent no-op. Only the engine knows the valid range to report. + let mut stmt = SelectStatement::default(); + stmt.select_items = vec![SelectItem::Column { + column: ColumnRef::unquoted("id".to_string()), + leading_comments: Vec::new(), + trailing_comment: None, + }]; + stmt.order_by = Some(vec![OrderByItem { + expr: SqlExpression::NumberLiteral("1.5".to_string()), + direction: SortDirection::Asc, + }]); + + let stmt = OrderByAliasTransformer::new() + .transform_statement(stmt) + .expect("transform"); + + assert_eq!(stmt.select_items.len(), 1); + assert!(matches!( + stmt.order_by.as_ref().unwrap()[0].expr, + SqlExpression::NumberLiteral(_) + )); + } + + #[test] + fn arithmetic_in_order_by_is_still_promoted() { + // The other half of the rule: `1+1` is an ordinary constant expression, + // not an ordinal, so it keeps the existing promotion path. Guards + // against the ordinal check widening to any numeric-valued expression. + let mut stmt = SelectStatement::default(); + stmt.select_items = vec![SelectItem::Column { + column: ColumnRef::unquoted("id".to_string()), + leading_comments: Vec::new(), + trailing_comment: None, + }]; + stmt.order_by = Some(vec![OrderByItem { + expr: SqlExpression::BinaryOp { + left: Box::new(SqlExpression::NumberLiteral("1".to_string())), + op: "+".to_string(), + right: Box::new(SqlExpression::NumberLiteral("1".to_string())), + }, + direction: SortDirection::Asc, + }]); + + let stmt = OrderByAliasTransformer::new() + .transform_statement(stmt) + .expect("transform"); + + assert_eq!(stmt.select_items.len(), 2, "expression promoted as hidden"); + assert!(matches!( + stmt.order_by.as_ref().unwrap()[0].expr, + SqlExpression::Column(ref c) if c.name.starts_with(HIDDEN_ORDERBY_PREFIX) + )); + } + #[test] fn test_is_aggregate_function() { let sum_expr = SqlExpression::FunctionCall { diff --git a/tests/comparison/corpus/08_ordering.toml b/tests/comparison/corpus/08_ordering.toml index eeed5caf..7e39a570 100644 --- a/tests/comparison/corpus/08_ordering.toml +++ b/tests/comparison/corpus/08_ordering.toml @@ -125,24 +125,63 @@ expect = "BOTH_ERR" # is a table alias, which is valid SQL and accepted by the reference engine too. # The defect was only ever about tokens with nowhere to belong. -# --- P16: ORDER BY is silently ignored --- +# --- P16: ORDER BY (fixed 2026-08-31) --- +# +# The ordinal is positional — it names the Nth *output* column — so every shape +# that changes what the output columns are needs its own case: an explicit +# select list, SELECT *, and GROUP BY. Before the fix the literal was promoted +# into a hidden CONSTANT column and sorted on, so all of them silently returned +# insertion order with no error. +# +# All six cases filter NULLs out of the sort key, so a P17 regression cannot be +# mistaken for a P16 one. [[case]] id = "order_by_ordinal" data = "null_edges.csv" sql = "SELECT id, score FROM null_edges WHERE score IS NOT NULL ORDER BY 2, 1" -expect = "DIFFER" -# P16. Rows come back in natural (insertion) order — the ordinal is evaluated as -# a constant, so every row compares equal and nothing sorts. No error is raised. -# NULLs are filtered out so this cannot be confused with P17. +# P16. Resolves to the 2nd select-list item. [[case]] id = "order_by_ordinal_desc" data = "null_edges.csv" sql = "SELECT id, score FROM null_edges WHERE score IS NOT NULL ORDER BY 2 DESC, 1" -expect = "DIFFER" -# P16 with a direction, which is also ignored. Worth pinning separately: a fix -# that resolves the ordinal but drops ASC/DESC would still pass the case above. +# P16 with a direction. Worth pinning separately: a fix that resolved the +# ordinal but dropped ASC/DESC would still pass the case above. + +[[case]] +id = "order_by_ordinal_star" +data = "null_edges.csv" +sql = "SELECT * FROM null_edges WHERE score IS NOT NULL ORDER BY 3, 1" +# P16 under SELECT *, where the ordinal counts source columns rather than +# select-list items. This is why the ordinal is resolved during execution and +# not in OrderByAliasTransformer — the star is still unexpanded there. + +[[case]] +id = "order_by_ordinal_group_by" +data = "null_edges.csv" +sql = "SELECT team, SUM(score) AS total FROM null_edges WHERE team IS NOT NULL AND score IS NOT NULL GROUP BY team ORDER BY 2 DESC, 1" +# P16 after GROUP BY, where the ordinal counts the aggregated output. The +# "top N by total" shape, and the one where silently returning group order was +# most likely to be believed. + +[[case]] +id = "order_by_ordinal_expression_is_not_positional" +data = "null_edges.csv" +sql = "SELECT id, score FROM null_edges WHERE score IS NOT NULL ORDER BY 1+1, 1" +# The other half of the rule: only a BARE integer is positional. `1+1` is an +# ordinary constant expression that sorts nothing, in both engines. Pinned so a +# future "evaluate ORDER BY expressions" change cannot quietly turn arithmetic +# into an ordinal. + +[[case]] +id = "order_by_ordinal_out_of_range" +data = "null_edges.csv" +sql = "SELECT id, score FROM null_edges ORDER BY 3" +expect = "BOTH_ERR" +# Out of range is an error, not a no-op — same reasoning as P13 stage 1. The +# range excludes columns promoted for ORDER BY visibility, which are appended +# after the real output: `ORDER BY label, 3` must not land on the hidden one. # --- P17: default NULL placement differs on ASC --- diff --git a/tests/test_multi_column_order_by.rs b/tests/test_multi_column_order_by.rs index 3d1d22ac..0865d1b7 100644 --- a/tests/test_multi_column_order_by.rs +++ b/tests/test_multi_column_order_by.rs @@ -422,3 +422,122 @@ fn test_order_by_table_qualified_column_still_resolves() { .collect(); assert_eq!(regions, vec!["Africa", "Americas", "Asia"]); } + +/// Build the small fixture the P16 ordinal tests share. +fn ordinal_fixture() -> Arc { + let mut table = DataTable::new("scores"); + table.add_column(DataColumn::new("id")); + table.add_column(DataColumn::new("team")); + table.add_column(DataColumn::new("score")); + + for (id, team, score) in [ + (1, "alpha", 50), + (2, "beta", 10), + (3, "alpha", 90), + (4, "beta", 30), + ] { + table + .add_row(DataRow::new(vec![ + DataValue::Integer(id), + DataValue::String(team.to_string()), + DataValue::Integer(score), + ])) + .unwrap(); + } + + Arc::new(table) +} + +fn column_as_strings(view: &DataView, col: usize) -> Vec { + (0..view.row_count()) + .map(|i| view.get_row(i).unwrap().values[col].to_string()) + .collect() +} + +/// P16: `ORDER BY 2` is a positional reference to the 2nd select-list item. It +/// used to be promoted into a hidden constant column, so every row compared +/// equal and the rows came back in insertion order with no error at all. +#[test] +fn test_order_by_ordinal_resolves_to_select_list_position() { + let view = QueryEngine::new() + .execute(ordinal_fixture(), "SELECT id, score FROM scores ORDER BY 2") + .unwrap(); + + assert_eq!(column_as_strings(&view, 0), vec!["2", "4", "1", "3"]); +} + +/// The direction has to survive resolution — a fix that found the column but +/// dropped DESC would still pass the test above. +#[test] +fn test_order_by_ordinal_honours_desc() { + let view = QueryEngine::new() + .execute( + ordinal_fixture(), + "SELECT id, score FROM scores ORDER BY 2 DESC", + ) + .unwrap(); + + assert_eq!(column_as_strings(&view, 0), vec!["3", "1", "4", "2"]); +} + +/// Under `SELECT *` the ordinal counts source columns. This is why the ordinal +/// is resolved at execution time and not in `OrderByAliasTransformer`, where +/// the star has not been expanded yet. +#[test] +fn test_order_by_ordinal_under_select_star() { + let view = QueryEngine::new() + .execute(ordinal_fixture(), "SELECT * FROM scores ORDER BY 3") + .unwrap(); + + assert_eq!(column_as_strings(&view, 0), vec!["2", "4", "1", "3"]); +} + +/// Out of range is an error rather than a sort that quietly does nothing. +#[test] +fn test_order_by_ordinal_out_of_range_errors() { + for sql in [ + "SELECT id, score FROM scores ORDER BY 3", + "SELECT id, score FROM scores ORDER BY 0", + ] { + let err = QueryEngine::new() + .execute(ordinal_fixture(), sql) + .expect_err("out-of-range ordinal must be rejected"); + assert!( + err.to_string().contains("out of range"), + "unexpected error for `{sql}`: {err}" + ); + } +} + +/// A column promoted purely so ORDER BY can see it is appended after the real +/// output, and must not be reachable by position: `ORDER BY team, 3` selects +/// from two output columns, not three. +#[test] +fn test_order_by_ordinal_ignores_promoted_hidden_columns() { + let err = QueryEngine::new() + .execute( + ordinal_fixture(), + "SELECT id, score FROM scores ORDER BY team, 3", + ) + .expect_err("hidden ORDER BY column must not be addressable by position"); + assert!( + err.to_string().contains("between 1 and 2"), + "hidden column leaked into the ordinal range: {err}" + ); +} + +/// A non-integer literal is rejected too — it is not positional, and promoting +/// it would make it a silent no-op. +#[test] +fn test_order_by_non_integer_literal_errors() { + let err = QueryEngine::new() + .execute( + ordinal_fixture(), + "SELECT id, score FROM scores ORDER BY 1.5", + ) + .expect_err("non-integer literal must be rejected"); + assert!( + err.to_string().contains("has no effect"), + "unexpected error: {err}" + ); +}