Skip to content

Fix failing test_filter_plus_limit_dictionary_sizes #7

Description

@jayendra13

Fix Plan: Filter + LIMIT Interaction Bug

Summary

Test test_filter_plus_limit_dictionary_sizes fails: query returns 1 row instead of 5.

Query: SELECT time, lat, lon, temperature FROM data WHERE lat = 5.0 LIMIT 5

Expected: 5 rows
Actual: 1 row

Root Cause Analysis

Data Structure

The synthetic data has:

  • Coordinates in order: [time, lon, lat] (based on schema output)
  • Shapes: time(7), lon(10), lat(10)
  • Data variable temperature shape: [7, 10, 10]
  • Total rows: 7 × 10 × 10 = 700

The Bug

When LIMIT interacts with coordinate filters, the limit calculation doesn't account for the reduced effective shape after filtering.

Current (Broken) Flow

  1. Calculate base limits with calculate_coord_limits([7, 10, 10], 5):

    i=0 (time): inner_size = 10*10=100, needed = ceil(5/100) = 1
    i=1 (lon):  inner_size = 10,        needed = ceil(5/10) = 1
    i=2 (lat):  inner_size = 1,         needed = 5
    base_limits = [1, 1, 5]
    
  2. Override filtered coordinate: Since lat has a filter, load all values:

    limits = [1, 1, 10]  (lat uses full size for filtering)
    
  3. Load coordinate values:

    • time: 1 value
    • lon: 1 value
    • lat: 10 values (all, to find filter match)
  4. Apply filter: lat = 5.0 finds index 5 in loaded lat values:

    coord_ranges = [(0, 1), (0, 1), (5, 6)]  for [time, lon, lat]
    
  5. Calculate effective sizes:

    • time: range (0,1), start=0 → apply limit min(1, 1) = 1
    • lon: range (0,1), start=0 → apply limit min(1, 1) = 1
    • lat: range (5,6), start≠0 → filtered, range_size = 1
    effective_coord_sizes = [1, 1, 1] → 1 row total! ❌
    

Why This Is Wrong

The base_limits calculation assumes the full shape [7, 10, 10]. But with a filter on lat that matches exactly 1 value, the effective shape becomes [7, 10, 1].

With effective shape [7, 10, 1] and LIMIT 5:

i=0 (time): inner_size = 10*1=10, needed = ceil(5/10) = 1
i=1 (lon):  inner_size = 1,       needed = 5
i=2 (lat):  filtered → load all to find match

So we should load:

  • time: 1 value
  • lon: 5 values (not 1!)
  • lat: 10 values (all, for filtering)

Then with filter applied:

coord_ranges = [(0, 1), (0, 5), (5, 6)]
effective_coord_sizes = [1, 5, 1] = 5 rows ✓

Fix Approach

Option A: Two-Phase Limit Calculation (Recommended)

  1. Phase 1: Identify which coordinates have filters
  2. Phase 2: Calculate base_limits using an adjusted shape where filtered coordinates are assumed to have effective size 1 (or the filter's expected match count for range filters)
// In read_zarr(), before calculating coord_value_limits:
let adjusted_coord_sizes: Vec<usize> = if let Some(ref filters) = coord_filters {
    coord_sizes.iter().enumerate().map(|(i, &size)| {
        let coord_name = &coord_names[i];
        if filters.filters.contains_key(coord_name) {
            // Assume filter reduces to ~1 match (conservative estimate)
            // For range filters, could estimate better
            1
        } else {
            size
        }
    }).collect()
} else {
    coord_sizes.clone()
};

let base_limits = calculate_coord_limits(&adjusted_coord_sizes, lim);

Then in the existing code, filtered coordinates still load all values to find the actual match.

Option B: Post-Filter Limit Recalculation

After computing coord_ranges, recalculate limits based on actual filter match counts:

// After calculate_coord_ranges returns
let actual_filter_sizes: Vec<usize> = coord_ranges.iter()
    .map(|(start, end)| end - start)
    .collect();

// Recalculate limits with actual sizes
let refined_limits = calculate_coord_limits(&actual_filter_sizes, lim);

Then apply these refined limits to slice the coordinate values.

Recommendation

Option A is simpler and doesn't require recalculating limits after filtering. The estimate of "1 match per filter" is conservative—it may load slightly more data than necessary for range filters but will always produce correct results.

Files to Modify

  1. src/reader/zarr_reader.rs: Adjust coord_value_limits calculation in read_zarr() and read_zarr_async()
  2. tests/integration_pushdown.rs: Remove #[ignore] from test_filter_plus_limit_dictionary_sizes

Test Coverage

The existing test test_filter_plus_limit_dictionary_sizes validates this fix. Additional edge cases to consider:

  1. Filter on innermost coordinate (current failing case)
  2. Filter on outermost coordinate
  3. Filter on middle coordinate
  4. Multiple coordinate filters
  5. Range filters (BETWEEN) with LIMIT

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