feat: AVG skips SQL NULLs via per-column validity bitmap

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,

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.

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<double>::infinity();
     double dmax = -std::numeric_limits<double>::infinity();
     int64_t imin = std::numeric_limits<int64_t>::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;j<len;j++) if (m[j]) acc_update(a, s, t, start+j);
+                    continue;
+                }
                 if (s.kind==AggKind::Sum || s.kind==AggKind::Avg) {
                     if (s.coltype==Type::F64) { const double* d=col.f64()+start; double sm=0; for(size_t j=0;j<len;j++) sm+=d[j]*(double)m[j]; a.sum+=sm; }
                     else if (col.width==8)    { const int64_t* d=col.i64()+start; int64_t sm=0; for(size_t j=0;j<len;j++) sm+=d[j]*(int64_t)m[j]; a.isum+=sm; a.sum+=(double)sm; }
                     else                      { const int32_t* d=col.i32()+start; int64_t sm=0; for(size_t j=0;j<len;j++) sm+=(int64_t)d[j]*(int64_t)m[j]; a.isum+=sm; a.sum+=(double)sm; }
+                    for (size_t j=0;j<len;j++) a.n += m[j];
                 } else if (s.kind==AggKind::Min) {
                     if (s.coltype==Type::F64) { const double* d=col.f64()+start; for(size_t j=0;j<len;j++){ if(m[j]&&d[j]<a.dmin)a.dmin=d[j]; } }
                     else { for(size_t j=0;j<len;j++){ if(m[j]){ int64_t v=col.get_int(start+j); if(v<a.imin)a.imin=v; } } }
+                    for (size_t j=0;j<len;j++) a.n += m[j];
                 } else if (s.kind==AggKind::Max) {
                     if (s.coltype==Type::F64) { const double* d=col.f64()+start; for(size_t j=0;j<len;j++){ if(m[j]&&d[j]>a.dmax)a.dmax=d[j]; } }
                     else { for(size_t j=0;j<len;j++){ if(m[j]){ int64_t v=col.get_int(start+j); if(v>a.imax)a.imax=v; } } }
+                    for (size_t j=0;j<len;j++) a.n += m[j];
                 }
             }
         }
@@ -516,10 +528,14 @@ ResultSet exec_select(Table& t, const Stmt& st, int opt) {
                 if (s.kind==AggKind::CountStar || s.kind==AggKind::Count || s.col<0) continue;
                 Acc& a = accs[k];
                 const Column& c = t.columns[s.col];
-                if (full_scan) {
+                if (full_scan && c.nullmask.empty()) {
+                    // All-valid fast path: no per-row NULL checks.
                     if (s.coltype==Type::F64) { const double* d=c.f64(); for(size_t r=0;r<nmatch;r++){double v=d[r]; a.sum+=v; if(v<a.dmin)a.dmin=v; if(v>a.dmax)a.dmax=v;} }
                     else if (c.width==8)     { const int64_t* d=c.i64(); for(size_t r=0;r<nmatch;r++){int64_t v=d[r]; a.isum+=v; a.sum+=(double)v; if(v<a.imin)a.imin=v; if(v>a.imax)a.imax=v;} }
                     else                     { const int32_t* d=c.i32(); for(size_t r=0;r<nmatch;r++){int32_t v=d[r]; a.isum+=v; a.sum+=(double)v; if(v<a.imin)a.imin=v; if(v>a.imax)a.imax=v;} }
+                    a.n = (int64_t)nmatch;
+                } else if (full_scan) {
+                    for (size_t r=0;r<nmatch;r++) acc_update(a, s, t, r);
                 } else {
                     for (size_t x=0;x<nmatch;x++) acc_update(a, s, t, sel[x]);
                 }
@@ -652,9 +668,18 @@ Value make_agg_value(const Table& t, const Acc& a, const AggSpec& s, int64_t gro
         case AggKind::Sum:
             if (dec) return Value::make_logical(a.isum, Type::I64, Logical::DECIMAL, s.scale);   // exact scaled sum
             return s.coltype==Type::F64 ? Value::make_f64(a.sum) : Value::make_i64(a.isum);
-        case AggKind::Avg:
-            if (dec) return Value::make_f64(group_count? a.sum/(double)group_count/(double)pow10i(s.scale) : 0.0);
-            return Value::make_f64(group_count? a.sum/(double)group_count : 0.0);
+        case AggKind::Avg: {
+            // Divide by non-NULL count. All agg paths must keep Acc::n accurate.
+            const int64_t n = a.n;
+            if (n == 0) {
+                // SQLite: AVG over no non-NULL inputs is NULL. Historical
+                // all-valid tables (empty nullmask) keep 0.0 when no rows matched.
+                if (s.col >= 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);

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<Value>& row) {
     for (size_t c=0;c<columns.size();c++) {
         Column& col = columns[c]; const Value& v = row[c];
-        if (col.logical != Logical::NONE) { col.append_raw(coerce_literal_to_physical(v, col.logical, col.scale), 0); continue; }
-        switch (col.type) {
+        // Explicit SQL NULL: store a typed placeholder and mark the validity bit.
+        // Importers that still map NULL→0/"" leave nullmask empty (all-valid).
+        if (v.is_null) {
+            if (col.logical != Logical::NONE) col.append_raw(0, 0);
+            else switch (col.type) {
+                case Type::I64: case Type::I32: col.append_raw(0, 0); break;
+                case Type::F64: col.append_raw(0, 0.0); break;
+                case Type::STR: col.append_raw(col.dict->intern(""), 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];

src/storage.h

@@ -68,6 +68,19 @@ struct Column {
 #endif
     std::vector<ZoneMap> zones;
     std::unique_ptr<Dictionary> 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<uint8_t> 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)