From 2bdcb84d3d50f9b247650d66cc355339b25210c9 Mon Sep 17 00:00:00 2001 From: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> Date: Wed, 26 Aug 2026 10:44:08 +0530 Subject: [PATCH] feat: AVG skips SQL NULLs via per-column validity bitmap Scoped NULL spike (#10): mark explicit INSERT NULLs on Column::nullmask and make AVG ignore them (divide by non-NULL count), keeping the empty-bitmap fast path for importers/benches. Document coverage and remaining NULL work. Closes #10 Signed-off-by: Vedant Madane <6527493+VedantMadane@users.noreply.github.com> --- GOOD_FIRST_ISSUES.md | 2 ++ docs/NULL_AVG_SPIKE.md | 36 ++++++++++++++++++++++++++++++++++++ src/exec.cpp | 33 +++++++++++++++++++++++++++++---- src/storage.cpp | 19 ++++++++++++++++--- src/storage.h | 13 +++++++++++++ 5 files changed, 96 insertions(+), 7 deletions(-) create mode 100644 docs/NULL_AVG_SPIKE.md diff --git a/GOOD_FIRST_ISSUES.md b/GOOD_FIRST_ISSUES.md index 880c8ed..f8dc38b 100644 --- a/GOOD_FIRST_ISSUES.md +++ b/GOOD_FIRST_ISSUES.md @@ -158,6 +158,8 @@ fast path. document what's covered and what's still TODO (this intentionally does *not* finish the whole NULL item). +**Status (partial):** AVG skip-NULL spike — see [docs/NULL_AVG_SPIKE.md](docs/NULL_AVG_SPIKE.md). + --- Bigger pieces (vectorized joins, cost-based optimizer, MVCC, compression, diff --git a/docs/NULL_AVG_SPIKE.md b/docs/NULL_AVG_SPIKE.md new file mode 100644 index 0000000..363f7ad --- /dev/null +++ b/docs/NULL_AVG_SPIKE.md @@ -0,0 +1,36 @@ +# AVG NULL-skip spike (#10) + +## Covered + +- Per-column in-memory validity bitmap (`Column::nullmask`). Empty = all valid + (fast path used by importers and historical data). +- SQL `INSERT ... VALUES (..., NULL, ...)` marks the row NULL instead of storing + a typed zero / empty string only. +- `AVG(col)` skips NULL inputs and divides by the non-NULL count (SQLite). +- All-NULL / no non-NULL inputs → `AVG` returns SQL `NULL`. +- Non-NULL fast path unchanged when `nullmask` is empty (bench loads). + +## Still TODO (full NULL roadmap item) + +- Persist validity bitmaps on disk / catalog (restart loses NULL marks today). +- Importers (`src/import.cpp`) still map CSV/IMDb `\N` → 0 / `""` without setting + the bitmap. +- `SUM` / `MIN` / `MAX` / `COUNT(col)` NULL semantics (partially share + `acc_update`, but SUM of all-NULL should be NULL; COUNT(col) should skip NULLs). +- Three-valued predicate logic (`WHERE col = 1` with NULL). +- Projection of NULL cells (`SELECT col` should print NULL when marked). +- Zone maps / indexes ignoring NULL keys. + +## How to verify + +```bash +make clean && make +./basalt :memory: <<'SQL' +CREATE TABLE t(id INT, v DOUBLE); +INSERT INTO t VALUES (1, 10.0), (2, NULL), (3, 30.0); +SELECT AVG(v) FROM t; +SQL +# expect 20 +``` + +Cross-check with SQLite: `SELECT AVG(v) FROM (VALUES (10.0),(NULL),(30.0));` → 20. diff --git a/src/exec.cpp b/src/exec.cpp index 7ea168a..0c885eb 100644 --- a/src/exec.cpp +++ b/src/exec.cpp @@ -158,6 +158,7 @@ struct AggSpec { AggKind kind; int col; Type coltype; Logical logical=Logical::N struct Acc { double sum = 0; int64_t isum = 0; // exact integer running value for MIN/MAX/SUM on ints + int64_t n = 0; // non-NULL inputs seen (AVG/SUM skip NULLs; #10) double dmin = std::numeric_limits::infinity(); double dmax = -std::numeric_limits::infinity(); int64_t imin = std::numeric_limits::max(); @@ -166,6 +167,9 @@ struct Acc { static inline void acc_update(Acc& a, const AggSpec& s, const Table& t, size_t row) { const Column& c = t.columns[s.col]; + // SQLite-style: AVG/SUM/MIN/MAX ignore NULL inputs. + if (c.is_null_at(row)) return; + a.n++; if (s.coltype == Type::F64) { double v = c.f64()[row]; a.sum += v; if (v < a.dmin) a.dmin = v; if (v > a.dmax) a.dmax = v; @@ -405,16 +409,24 @@ ResultSet exec_select(Table& t, const Stmt& st, int opt) { for (int k : scan_items) { Acc& a = accs[k]; const AggSpec& s = aggspecs[k]; const Column& col = t.columns[s.col]; + // When a NULL bitmap is present, fold validity into the match mask. + if (!col.nullmask.empty()) { + for (size_t j=0;ja.dmax)a.dmax=d[j]; } } else { for(size_t j=0;ja.imax)a.imax=v; } } } + for (size_t j=0;ja.dmax)a.dmax=v;} } else if (c.width==8) { const int64_t* d=c.i64(); for(size_t r=0;ra.imax)a.imax=v;} } else { const int32_t* d=c.i32(); for(size_t r=0;ra.imax)a.imax=v;} } + a.n = (int64_t)nmatch; + } else if (full_scan) { + for (size_t r=0;r= 0 && !t.columns[s.col].nullmask.empty()) return Value::null(Type::F64); + return Value::make_f64(0.0); + } + if (dec) return Value::make_f64(a.sum / (double)n / (double)pow10i(s.scale)); + return Value::make_f64(a.sum / (double)n); + } case AggKind::Min: if (s.logical!=Logical::NONE) return Value::make_logical(a.imin, s.coltype, s.logical, s.scale); if (s.coltype==Type::F64) return Value::make_f64(a.dmin); diff --git a/src/storage.cpp b/src/storage.cpp index 3acaaba..2960b6b 100644 --- a/src/storage.cpp +++ b/src/storage.cpp @@ -62,7 +62,7 @@ Column& Column::operator=(Column&& o) noexcept { #endif name=std::move(o.name); type=o.type; logical=o.logical; scale=o.scale; width=o.width; path=std::move(o.path); fd=o.fd; base=o.base; wbase=o.wbase; mapped_bytes=o.mapped_bytes; cap_bytes=o.cap_bytes; count=o.count; dirty=o.dirty; - zones=std::move(o.zones); dict=std::move(o.dict); + zones=std::move(o.zones); dict=std::move(o.dict); nullmask=std::move(o.nullmask); #ifdef __EMSCRIPTEN__ membuf=std::move(o.membuf); base=wbase=membuf.data(); // pointers follow the moved buffer #endif @@ -155,12 +155,25 @@ void Column::write_bulk(const void* data, size_t nbytes, size_t nelem) { void Table::insert_row(const std::vector& row) { for (size_t c=0;cintern(""), 0); break; + } + col.set_null_at(col.count - 1, true); + continue; + } + if (col.logical != Logical::NONE) { col.append_raw(coerce_literal_to_physical(v, col.logical, col.scale), 0); } + else switch (col.type) { case Type::I64: case Type::I32: col.append_raw(v.i64, 0); break; case Type::F64: col.append_raw(0, v.type==Type::F64? v.f64 : (double)v.i64); break; case Type::STR: col.append_raw(col.dict->intern(v.s), 0); break; } + if (!col.nullmask.empty()) col.set_null_at(col.count - 1, false); } if (pk_col>=0) { const Column& pc = columns[pk_col]; const Value& kv = row[pk_col]; diff --git a/src/storage.h b/src/storage.h index b231f50..7413bad 100644 --- a/src/storage.h +++ b/src/storage.h @@ -68,6 +68,19 @@ struct Column { #endif std::vector zones; std::unique_ptr dict; + // Per-row NULL bitmap (1 = NULL). Empty means "no NULLs known" — the + // historical fast path (importers still map SQL NULL to 0/""). Populated + // when SQL INSERT/VALUES supplies an explicit NULL (see #10 AVG spike). + std::vector nullmask; + + bool is_null_at(size_t r) const { + return !nullmask.empty() && r < nullmask.size() && nullmask[r] != 0; + } + void set_null_at(size_t r, bool is_null) { + if (!is_null && nullmask.empty()) return; // all-valid fast path + if (nullmask.size() <= r) nullmask.resize(r + 1, 0); + nullmask[r] = is_null ? 1 : 0; + } Column() = default; Column(std::string n, Type t, std::string p, Logical lg=Logical::NONE, int sc=0)