Skip to content

add_stats leaves rows of an overlapping family behind, and column_counts reports them #148

Description

@roed-math

add_stats replaces only the counts rows it reinserts, so when two statistics
families share a cols key and the data has changed between them, the rows of
the earlier family that no longer correspond to anything survive — and
column_counts reports them.

Split out of the review of #141, which fixed stats_valid enforcement. This one
is older than that PR and independent of it: it is not about the flag, it is
about add_stats not owning the rows it is replacing.

Reproduction

60 rows, flag = (n % 3 == 0), saving on:

table.stats.add_stats(["n"], {"flag": True})    # 20 rows, stored under cols = ["flag", "n"]
table.update({"n": {"$lt": 30}}, {"n": 999}, restat=False)
table.stats.add_stats(["flag", "n"])            # same cols key, different family, so it runs

table.stats.column_counts(["flag", "n"])        # 42 groups, summing to 70
# truth:                                          32 groups, and the table has 60 rows

The second add_stats recomputes the 32 (flag, n) pairs that now exist and
replaces exactly those 32 keys. The 10 pairs the constrained family had recorded
that no longer occur are not among them, so they stay, and column_counts adds
them to what it reports. It is not only a stale row for a value that vanished:
the group count and the total are both wrong, and a statistics page renders that
directly.

refresh_stats() repairs it — it empties the counts table before replaying every
recorded family against one snapshot of the data (42 groups → 32, sum 60). So
the reload and refresh pipelines do not produce this state; it needs add_stats
called directly, on data that changed without a restat, where another family
has already written under the same cols key.

Why the obvious fix does not work

add_stats cannot delete its family's rows wholesale before inserting, because
counts rows carry no record of which family wrote them. Two existing tests pin
the reasons:

  • test_range_count_survives_add_stats — a one-off count({"n": {"$lt": 50}}, record=True) is stored under cols = ["n"], the same key add_stats(["n"])
    writes, and must survive it. Filtering on extra IS NOT TRUE rescues this
    case.
  • test_threshold_families_share_rows — counts rows record no threshold, so
    add_stats(["flag"], threshold=5) and add_stats(["flag"]) write into the
    same key space with the same extra = false, and legitimately share rows.
    Filtering on extra does not rescue this one.

So "the rows of this family that the recomputation did not produce" is not
directly expressible over the counts table.

Suggested direction

The stats table already records each family as
(cols, stat, constraint_cols, constraint_values, threshold). That is enough to
work out, for a given cols key, which counts rows some recorded family still
claims, and to delete the rest — without adding a column anywhere. Fiddly, since
counts rows are keyed on allcols (statted columns plus constraint columns)
while the stats row records the statted columns and the constraint separately,
but it is bookkeeping rather than a format change.

Worth settling before 1.0.0: whether the accepted fix really can avoid a new
column on the <table>_counts relation. If it cannot, that is the part that gets
more expensive later — the META_FORMAT migration machinery covers the meta_*
tables, not the per-table _counts / _stats relations, so adding a column
there after 1.0 would need new migration plumbing. If the stats-table approach
above holds up, this is a pure-code fix and no more expensive after 1.0 than
before, which is why it is filed rather than blocking.

Related

A different and already-documented limitation, not this one: while
stats_valid is false, column_counts, numstats and null_counts report
what is recorded, because gating them would make them rebuild on every call with
nothing to converge on. That is described in DataManagement.md and pinned by
the strict=True xfail test_column_counts_can_still_be_stale. Closing that one
needs freshness per statistic; closing this one does not.

🤖 Generated with Claude Code

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions