Visitor and evaluator edge cases can over-prune files or mishandle nulls

[kevinjqliu]

Several visitor/evaluator edge cases appear unsafe or inconsistent:

1. _StrictMetricsEvaluator.visit_not_equal / visit_not_in return ROWS_MUST_MATCH when a file can contain nulls or NaNs. Example stats with [null, 5] or [NaN, 5.0] and lower/upper bounds both 5 return true for NotEqualTo("x", 5) / NotIn("x", {5}), even though one row does not match. This can incorrectly mark whole files deleted.

2. _StrictMetricsEvaluator.eval returns ROWS_MUST_MATCH for record_count <= 0. record_count=0 is vacuously true, but record_count=-1 is unknown per the local comment; even AlwaysFalse() returns true.

3. ResidualVisitor comparison methods directly compare partition values to literals. A nullable identity partition value of None with LessThan("x", 1) raises TypeError, while row evaluation returns false.

4. ResidualVisitor.visit_not_nan(None) returns AlwaysFalse, while expression evaluation treats NotNaN(None) as true. Existing tests encode both behaviors, so the semantics are inconsistent.

Validated against the current tree; examples use stats/partition shapes already supported by the repo tests.

[tanmayrauth]

I went through visitors.py and the call sites, and three of the four hold up. The big one is 1: visit_not_equal and visit_not_in short-circuit on _can_contain_nulls/_can_contain_nans and return ROWS_MUST_MATCH without ever checking the bounds, where every other strict method (and Java) uses the contains*_only variants. Since the strict evaluator drives _DeleteFiles (snapshot.py:460), this isn't just over-pruning — a delete(NotEqualTo("x", 5)) against a file with stats [null, 5] would drop the whole file and take the legitimate 5 row with it, so it's effectively silent data loss. 3 and 4 are real too: the ResidualVisitor comparisons hit None < literal and raise TypeError where the row evaluator safely guards for None, and visit_not_nan(None) returns AlwaysFalse while the row evaluator treats it as true — both reachable on the scan path via residual_for. The only one I'd push back on is 2; the record_count <= 0 behavior matches upstream Java and looks intentional, though the comment could be clearer about the -1 case.

If nobody's already on this, I'd like to take it, starting with 1 since it's the data-loss path, with a regression test asserting the 5 row survives the delete, then a follow-up for the ResidualVisitor null handling.