Name the ten ClickBench queries that do not have one answer

README.md

@@ -98,6 +98,8 @@ cargo run --release -p iris-bench-cli -- clickbench check duckdb.json datafusion
 
 Agreement is per query, across every system that answered it. A query only one system answered is reported separately rather than counted as confirmed, and a system that gave two different answers across its own three runs is named even when the systems agreed with each other.
 
+Ten of the forty three do not have one answer, and they are named in code with the reason attached rather than tolerated when they come up. Nine put a LIMIT on top of an ordering that does not tell the rows inside the window apart from the ones just outside, so two correct systems return a different arbitrary ten. The tenth asks for every column, and the two setups ClickBench publishes do not build the same table out of the same corpus, so the same rows cannot render the same way. Each of the ten was run against the real corpus and read before it went on the list, a test pins the list so it cannot grow quietly, and everything not on it still has to agree.
+
 A digest tells you two systems differ and tells you nothing about what they differ on, so there is a fourth command for the moment after a comparison fails. It takes the table in the same way a run does, runs only the queries you name, and prints the rows both digests were taken over.
 
 ```

crates/bench-cli/src/clickbench.rs

@@ -292,8 +292,19 @@ pub(crate) fn check(paths: &[PathBuf]) -> anyhow::Result<()> {
     let comparison = compare(&reports);
 
     println!("{} agreed", comparison.agreed.len());
-    for one in &comparison.disagreed {
-        println!("{} disagreed", one.query);
+
+    // Ten of the forty three do not pick out a single result, so two correct systems return
+    // different rows for them and both are right. Which ten and why is written down in
+    // bench_workload::clickbench, with the evidence, and it is a list somebody edits on purpose
+    // rather than a rule that lets today's disagreement excuse itself.
+    let (expected, real): (Vec<_>, Vec<_>) = comparison
+        .disagreed
+        .iter()
+        .partition(|one| clickbench::undetermined(&one.query).is_some());
+    for one in real.iter().chain(expected.iter()) {
+        let note = clickbench::undetermined(&one.query)
+            .map_or_else(String::new, |why| format!(", and {}", why.why()));
+        println!("{} disagreed{note}", one.query);
         for reading in &one.readings {
             println!(
                 "    {:<16} {} rows {}",
@@ -303,8 +314,17 @@ pub(crate) fn check(paths: &[PathBuf]) -> anyhow::Result<()> {
             );
         }
     }
-    for one in &comparison.unstable {
-        println!("{} disagreed with itself on {}", one.driver, one.query);
+    let (wobbly, churning): (Vec<_>, Vec<_>) = comparison
+        .unstable
+        .iter()
+        .partition(|one| clickbench::undetermined(&one.query).is_some());
+    for one in churning.iter().chain(wobbly.iter()) {
+        let note = if clickbench::undetermined(&one.query).is_some() {
+            ", which is the same tie seen from one system"
+        } else {
+            ""
+        };
+        println!("{} disagreed with itself on {}{note}", one.driver, one.query);
     }
     if !comparison.alone.is_empty() {
         println!(
@@ -321,8 +341,20 @@ pub(crate) fn check(paths: &[PathBuf]) -> anyhow::Result<()> {
         );
     }
 
-    anyhow::ensure!(comparison.clean(), "the systems did not agree");
-    println!("every query that more than one system answered got the same answer");
+    anyhow::ensure!(
+        real.is_empty() && churning.is_empty(),
+        "the systems did not agree about {} queries that have one answer",
+        real.len() + churning.len()
+    );
+    if expected.is_empty() && wobbly.is_empty() {
+        println!("every query that more than one system answered got the same answer");
+    } else {
+        println!(
+            "every query with one answer got the same answer, and {} of the ones without a single \
+             answer came back differently, which is what they do",
+            expected.len()
+        );
+    }
     Ok(())
 }
 

crates/bench-workload/src/agree.rs

@@ -66,7 +66,14 @@ pub struct Comparison {
 }
 
 impl Comparison {
-    /// Whether anything here should stop a set of timings being published.
+    /// Whether every query that two systems both answered came back the same, both times.
+    ///
+    /// The strict reading, and the right one to start from. A workload can still have queries whose
+    /// published text does not pick out a single result, and then two correct systems differ and
+    /// this returns false about nothing. Which queries those are is the workload's business rather
+    /// than this module's, so a caller that has such a workload sorts its own disagreements out
+    /// against a list it can be held to. `bench_workload::clickbench::undetermined` is the one that
+    /// exists.
     #[must_use]
     pub fn clean(&self) -> bool {
         self.disagreed.is_empty() && self.unstable.is_empty()

crates/bench-workload/src/clickbench.rs

@@ -47,6 +47,49 @@
 //! `CONFIG.md` so that a corpus which one day arrives without those logical types is a thing
 //! somebody looks at again instead of a thing that quietly answers a different question.
 //!
+//! # Ten of the forty three do not have one answer
+//!
+//! Two systems that return different rows for the same query are normally one correct system and
+//! one that is fast because it did less. Ten of these forty three are not that. They are queries
+//! whose published text does not pick out a single result, so two correct systems return different
+//! rows and both are right, and a comparison that called that a failure would be reporting a
+//! property of the benchmark as a fault in an engine.
+//!
+//! This is not a category anything gets put in for disagreeing. Every entry below was run and
+//! looked at, the reason is a property of the published SQL or of the published setup rather than
+//! of the results, and `iris-bench clickbench answer` reproduces the evidence for any of them.
+//!
+//! Nine of the ten are ties under a `LIMIT`. The window the `LIMIT` returns is only determined when
+//! the ordering key tells the rows inside it apart from the rows just outside, and in these nine it
+//! does not. q17 has no `ORDER BY` at all and takes ten of many million groups. q23 and q24 order
+//! by `EventTime`, which is a second and not a row identity. q21, q31 and q32 order by a count over
+//! a grouping close to one row per group. q38, q39, q40 and q41 order by a count and then take ten
+//! rows at an offset of a hundred, a thousand or ten thousand, which lands the window deep in a
+//! tail where every remaining group has the same tiny count.
+//!
+//! What was checked, on the real corpus, is that both engines computed the same aggregate. For all
+//! six of the grouped ones the sequence of ordering key values is identical between the two, and
+//! the values are tied across the whole window: q31 and q41 return ten counts of one, q38 ten of
+//! two, q39 ten of fifteen, q32 four twos then six ones, and q40 has every value in it repeated at
+//! least twice so even its boundary sits inside a tie. Two engines returned the same counts and a
+//! different arbitrary ten of the rows holding them.
+//!
+//! The tenth is q23, and it is a different thing wearing the same clothes. It is `SELECT *`, and
+//! the two published setups do not produce the same table. `DuckDB`'s load uses `SELECT * REPLACE`,
+//! which converts `EventDate` where it stands, and `DataFusion`'s view uses `SELECT * EXCEPT` with
+//! the converted column appended, which moves `EventDate` from the fifth column to the hundred and
+//! fifth. `DuckDB` also converts the three timestamps and `DataFusion` leaves them as raw seconds.
+//! On the real corpus the two returned the same ten rows, and a hundred and two of the hundred and
+//! five columns matched exactly: the three that did not are `EventTime`, `ClientEventTime` and
+//! `LocalEventTime`, holding the same instants written two ways. So the rows agree and the
+//! rendering cannot, and that follows from carrying each entry's own setup rather than from
+//! anything either engine did.
+//!
+//! Note what this does not excuse. The other thirty three have to agree, and they do. An unlisted
+//! query that disagrees is still a hard failure, adding to the list is a change somebody has to
+//! make on purpose and defend, and a test in this module asserts the list is exactly these ten so
+//! that it cannot grow by accident.
+//!
 //! # What this module does not do
 //!
 //! It does not rewrite anything itself. If a system needs a rewrite that upstream has not
@@ -244,6 +287,57 @@ pub fn projection(driver: &str) -> Option<&'static str> {
     }
 }
 
+/// Why a published query does not pick out a single result.
+///
+/// Both of these are properties of what `ClickBench` publishes rather than of what any engine did,
+/// which is why they live here next to the queries and the setup scripts. The module doc has the
+/// evidence for every query that carries one.
+#[derive(Clone, Copy, PartialEq, Eq, Hash, Debug, serde::Serialize, serde::Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub enum Undetermined {
+    /// A `LIMIT` cuts a window whose ordering key does not tell the rows inside it apart from the
+    /// rows just outside, so which ten come back is the plan's choice and not the query's.
+    Tie,
+    /// The query returns every column, and the two published setups do not put the columns in the
+    /// same order or hold them in the same types, so the same rows cannot render the same way.
+    Setup,
+}
+
+impl Undetermined {
+    /// One line saying what this is, for a reader of a comparison.
+    #[must_use]
+    pub fn why(self) -> &'static str {
+        match self {
+            Self::Tie => "the LIMIT cuts a tie, so which rows come back is not decided by the query",
+            Self::Setup => {
+                "SELECT * over two setups that do not build the same table out of the same corpus"
+            }
+        }
+    }
+}
+
+/// Which of the forty three do not have one answer, and why.
+///
+/// Ten of them. This is a list rather than a rule because the property it stands for is not
+/// decidable from the SQL alone: whether an ordering key separates the rows a `LIMIT` cuts between
+/// depends on the data, and the corpus is pinned but the reasoning still has to be done by looking.
+/// So every entry was run on the real corpus and read, the module doc says what was seen, and
+/// `iris-bench clickbench answer` puts the same rows back on a terminal for anybody checking.
+///
+/// A list is also the point. A query that disagreed today cannot talk its way in here, because
+/// getting in is an edit somebody makes deliberately and defends, and the test below pins the whole
+/// set so that it does not grow while nobody is reading.
+#[must_use]
+pub fn undetermined(query: &str) -> Option<Undetermined> {
+    match query {
+        "q23" => Some(Undetermined::Setup),
+        "q17" | "q21" | "q24" | "q31" | "q32" | "q38" | "q39" | "q40" | "q41" => {
+            Some(Undetermined::Tie)
+        }
+        _ => None,
+    }
+}
+
 /// The workload, for one system's rewrite of the queries.
 ///
 /// # Panics
@@ -371,6 +465,55 @@ mod tests {
         assert_eq!(projection("nothing-by-that-name"), None);
     }
 
+    #[test]
+    fn the_queries_without_one_answer_are_exactly_these_ten() {
+        // Pinned so the list cannot grow while nobody is reading. Every one of these was run on the
+        // real corpus and looked at, and a query that starts disagreeing tomorrow has to be added
+        // here on purpose with the same evidence rather than by loosening a rule.
+        let listed: Vec<String> = (0..QUERIES)
+            .map(|at| format!("q{at}"))
+            .filter(|id| undetermined(id).is_some())
+            .collect();
+        assert_eq!(
+            listed,
+            [
+                "q17", "q21", "q23", "q24", "q31", "q32", "q38", "q39", "q40", "q41"
+            ]
+        );
+        assert_eq!(undetermined("q0"), None);
+    }
+
+    #[test]
+    fn only_the_query_that_selects_every_column_blames_the_setup() {
+        // The two setups differ in where they put EventDate and in whether they convert the three
+        // timestamps, and the only way a query can see either of those is by asking for all of the
+        // columns. Any other query naming a converted column names it the same way on both sides.
+        for at in 0..QUERIES {
+            let id = format!("q{at}");
+            let sql = &workload(Dialect::DuckDb).queries[at].sql;
+            let selects_everything = sql.contains("SELECT * FROM");
+            assert_eq!(
+                undetermined(&id) == Some(Undetermined::Setup),
+                selects_everything,
+                "{id} is {sql}"
+            );
+        }
+    }
+
+    #[test]
+    fn every_query_without_one_answer_cuts_a_window_short() {
+        // A tie only matters where something is thrown away. A query that returns its whole result
+        // hands back the same rows either way and the canonical form sorts them, so LIMIT is what
+        // every entry on the list has in common and a query without one could not qualify.
+        for at in 0..QUERIES {
+            let id = format!("q{at}");
+            if undetermined(&id).is_some() {
+                let sql = &workload(Dialect::DuckDb).queries[at].sql;
+                assert!(sql.contains("LIMIT"), "{id} is {sql}");
+            }
+        }
+    }
+
     #[test]
     fn every_dialect_has_all_forty_three() {
         for dialect in ALL {

crates/bench-workload/src/lib.rs

@@ -41,7 +41,7 @@ pub mod protocol;
 pub mod workload;
 
 pub use agree::{Comparison, Disagreement, Reading, Unstable, compare};
-pub use clickbench::Dialect;
+pub use clickbench::{Dialect, Undetermined};
 pub use digest::{Digest, ParseDigestError};
 pub use leaderboard::{BAND, Calibration, Column, Published, Reference, Standing, geomean};
 pub use protocol::{Cold, Measured, Outcome, PageCache, RUNS, Report, Run, Warm, measure};

docs/METHODOLOGY.md

@@ -66,6 +66,18 @@ Canonicalised is the load bearing word, because three systems asked the same que
 
 Two of the rules cost something and are worth stating. Floats are rendered to six significant digits, because two engines summing a hundred million values will not agree past that when one adds them in parallel partial sums and the other in order. Six digits survives that and still catches a hardcoded group count or an overflowed accumulator, neither of which is a seventh digit difference. What it gives up is a genuine small error, and that is the price of a digest, which cannot express a tolerance. Rows are sorted unless the query specified an order, because two systems that returned the same rows in a different order both answered the question that was asked, and a query that does specify an order gets that order checked.
 
+## Queries that do not have one answer
+
+Sorting an unordered result handles two systems returning the same rows in a different order. It does not handle a query that does not have one set of rows to return, and a published benchmark can contain those. Ten of ClickBench's forty three do.
+
+Nine of them put a LIMIT on top of an ordering that does not tell the rows inside the window apart from the rows just outside it, so two correct systems return a different arbitrary ten and neither is wrong. One of them asks for every column, and the two setups ClickBench publishes for the two systems here do not build the same table out of the same corpus, so the same rows cannot render the same way. What was seen on the real corpus, query by query, is in the module documentation for the workload.
+
+The rule for these is that they are named in advance, in code, one at a time, with the reason attached. A query is not excused because it disagreed. It is excused because somebody ran it, looked at what came back, wrote down why the benchmark does not determine it, and put it on a list that a test pins so it cannot grow while nobody is reading. Everything not on that list has to agree, and a disagreement there is still a hard failure that stops the timings being published.
+
+The same applies to a system that answers one query two different ways across its own runs. On a query with one answer that is a fault and it fails. On one of the ten it is the same tie seen from one system instead of two, and it is reported as that rather than counted twice.
+
+`iris-bench clickbench answer` puts the rows behind any of this back on a terminal, so none of it has to be taken on trust.
+
 ## Pre-registration
 
 Every claim this repository intends to test is registered before it is run, with the threshold that decides it. Rows carry a purpose field, either confirmatory or exploratory, and an exploratory row cannot be cited as a claim. A claim that was registered and not run shows in the ledger as pending, which makes quietly dropping an inconvenient result visible as a gap in a committed file.