[Bug](zonemap) Float and double zone maps written before has_nan existed are treated as NaN-free

[LuciferYang]

Search before asking

• I had searched in the issues and found no similar issues.

Description

ZoneMap::has_nan is optional bool = 8 in ZoneMapPB (gensrc/proto/segment_v2.proto), added by
#55633 together with the NaN handling in the zone map writer. ZoneMap::from_proto
(be/src/storage/index/zone_map/zone_map_index.cpp) reads it unconditionally, so a zone map
serialized before that field existed deserializes with has_nan == false. Nothing checks
has_has_nan(), and there is no segment-version gate.

Every consumer that treats has_nan == false as "no NaN in this zone" therefore trusts bounds that
may omit one. range_stats_usable_for_zonemap (be/src/exprs/expr_zonemap_filter.cpp) is one;
#55633 also taught the older olap predicate path to trust the same flag, so this is not limited to
expression zone maps.

Concretely: a FLOAT page written before #55633 holding {1.0, NaN, 2.0} stores bounds [1.0, 2.0]
with the field absent, because the generic min/max path never selects a NaN. After an upgrade,
WHERE d > 3.0 prunes the page, but Doris orders NaN above every other value, so that row satisfies
the predicate and should have been returned.

What I have not established is whether any deployment still reads pre-#55633 segments. That decides
the severity, and it is not something the code can answer.

Solution

Treat a FLOAT or DOUBLE ZoneMapPB with field 8 absent as unusable for range pruning, distinguishing
"no NaN" from "unknown", the same way the Parquet readers already distinguish those two with
floating_nan_count_unknown. The fix belongs at the from_proto or index-meta layer so that every
consumer inherits it.

Are you willing to submit PR?

• Yes I am willing to submit a PR!

[Doris-Breakwater]

Breakwater on-call analysis

Verdict: confirmed, actionable compatibility/correctness bug. The proposed failure mode is supported by the code, and the cross-version read path is reachable. I would prioritize this as a high-severity query-correctness issue because it can silently omit qualifying rows; the remaining unknown is prevalence, not validity.

Verified facts

• ZoneMapPB is proto2, and has_nan is field 8. ZoneMap::from_proto currently assigns zone_map.has_nan() without consulting has_has_nan(), so absent and explicitly-false metadata collapse to the same in-memory value.
• Before [improve](zonemap) support zonemap for float/double type #55633, the writer already instantiated zone maps for FLOAT and DOUBLE and used the generic std::minmax_element path. Reproducing that exact path with {1.0, NaN, 2.0} yields stored bounds [1.0, 2.0]; the old protobuf cannot record the hidden NaN.
• Doris comparison semantics place NaN above finite values. Both the legacy column-predicate path and the expression zone-map path consume the result of ZoneMap::from_proto, so those finite bounds can cause a false-negative page or segment prune for a predicate such as d > 3.0.
• The recent reversed-bounds protection from [fix](zonemap) Treat reversed zone map bounds as invalid #67431 only detects legacy pages whose bounds never moved (for example, NaN-only pages). Mixed finite/NaN pages have ordinary-looking ordered bounds, so that protection does not cover this case.
• This is not only hypothetical dead-format compatibility: the 3.1 branch has no has_nan field, while the 4.0/4.1 readers have the unguarded read. The official 4.x upgrade guide explicitly covers 3.x-to-4.0 upgrades and states that an upgrade replaces FE/BE binaries and libraries, leaving existing data files in place. Legacy rowsets can therefore be read by an affected 4.x BE immediately after upgrade.
• The issue currently has no labels or assignee. It should be triaged under the project's BE/storage/zone-map correctness conventions.

Recommended fix

Handle the compatibility distinction centrally in ZoneMap::from_proto: for FLOAT or DOUBLE, an absent field 8 should make range bounds unusable (for example, set the deserialized zone map's pass_all state, or introduce an explicit range-validity state). Preserve has_null/has_not_null so null-only pruning remains available. Do not fail the scan.

Using field presence is preferable to a segment-version gate: fields 6-8 were introduced together, and the Doris writer explicitly calls set_has_nan(false) for a known NaN-free zone, so has_has_nan() distinguishes new known-false metadata from legacy unknown metadata without inferring a deployment tag or version.

This needs to remain in the common deserialization/index layer so segment pruning, page pruning, predicate elimination, and expression zone-map evaluation inherit the conservative behavior. A consumer-only check would leave other pruning paths exposed.

Tests and next steps

1. Add FLOAT and DOUBLE unit cases that construct a ZoneMapPB with valid ordered finite bounds and deliberately do not set field 8; after from_proto, range pruning must be disabled.
2. Add the control case with set_has_nan(false); its range must remain usable. Also verify that an older non-floating zone map without field 8 remains usable.
3. Add a reader-level regression for both page and segment pruning using legacy metadata representing {1.0, NaN, 2.0} and d > 3.0. A segment artifact written by a pre-[improve](zonemap) support zonemap for float/double type #55633 build would be the strongest cross-version regression fixture.
4. Backport the fix to maintained 4.0 and 4.1 lines, since those are the readers that can encounter pre-4.0 rowsets. Coordinate with [fix](zonemap) Treat reversed zone map bounds as invalid #67431 coverage: the current 4.0 branch also lacks that separate reversed-bounds safeguard.

No logs or runtime profile are required to accept the bug because the metadata compatibility failure is deterministic. To quantify exposure and release urgency, it would still help to collect the source version(s) of upgraded clusters, whether FLOAT/DOUBLE columns can contain NaN, and whether all pre-4.0 rowsets have actually been rewritten by compaction. Until that inventory exists, affected deployments should not assume compaction has removed all legacy segments.

Breakwater-GitHub-Analysis-Slot: slot_2338064ac779