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
add_statsreplaces only the counts rows it reinserts, so when two statisticsfamilies share a
colskey and the data has changed between them, the rows ofthe earlier family that no longer correspond to anything survive — and
column_countsreports them.Split out of the review of #141, which fixed
stats_validenforcement. This oneis older than that PR and independent of it: it is not about the flag, it is
about
add_statsnot owning the rows it is replacing.Reproduction
60 rows,
flag = (n % 3 == 0),savingon:The second
add_statsrecomputes the 32(flag, n)pairs that now exist andreplaces 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_countsaddsthem 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 everyrecorded 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_statscalled directly, on data that changed without a restat, where another family
has already written under the same
colskey.Why the obvious fix does not work
add_statscannot delete its family's rows wholesale before inserting, becausecounts rows carry no record of which family wrote them. Two existing tests pin
the reasons:
test_range_count_survives_add_stats— a one-offcount({"n": {"$lt": 50}}, record=True)is stored undercols = ["n"], the same keyadd_stats(["n"])writes, and must survive it. Filtering on
extra IS NOT TRUErescues thiscase.
test_threshold_families_share_rows— counts rows record nothreshold, soadd_stats(["flag"], threshold=5)andadd_stats(["flag"])write into thesame key space with the same
extra = false, and legitimately share rows.Filtering on
extradoes 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 towork out, for a given
colskey, which counts rows some recorded family stillclaims, 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>_countsrelation. If it cannot, that is the part that getsmore expensive later — the
META_FORMATmigration machinery covers themeta_*tables, not the per-table
_counts/_statsrelations, so adding a columnthere 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_validis false,column_counts,numstatsandnull_countsreportwhat is recorded, because gating them would make them rebuild on every call with
nothing to converge on. That is described in
DataManagement.mdand pinned bythe
strict=Truexfailtest_column_counts_can_still_be_stale. Closing that oneneeds freshness per statistic; closing this one does not.
🤖 Generated with Claude Code