Skip to content

Conversation

@universalmind303
Copy link
Member

Summary

  • Migrated ilike.rs from arrow2 to native Daft array APIs
  • Replace daft_arrow::array::BooleanArray collection with Vec<Option<bool>>
  • Use BooleanArray::from((name, slice)) instead of arrow2 BooleanArray
  • Added comprehensive test coverage for ilike function:
    • Broadcast pattern matching
    • SQL LIKE wildcards (% and _)
    • Element-wise matching
    • Null handling

Test plan

  • cargo test -p daft-functions-utf8 ilike passes (4 tests)

🤖 Generated with Claude Code

- Replace daft_arrow::array::BooleanArray collection with Vec<Option<bool>>
- Use BooleanArray::from((name, slice)) instead of arrow2 BooleanArray
- Add comprehensive test coverage for ilike function:
  - Broadcast pattern matching
  - SQL LIKE wildcards (% and _)
  - Element-wise matching
  - Null handling

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <[email protected]>
@universalmind303 universalmind303 changed the base branch from arrow-rs-migration to main January 6, 2026 17:52
@greptile-apps
Copy link
Contributor

greptile-apps bot commented Jan 6, 2026

Greptile Summary

This PR successfully migrates ilike.rs from arrow2 APIs to native Daft array APIs as part of the broader arrow-rs migration effort.

Key changes:

  • Replaced daft_arrow::array::BooleanArray collection with Vec<Option<bool>>
  • Changed from BooleanArray::from((name, Box::new(arrow_result))) to BooleanArray::from((name, result_vec.as_slice()))
  • Added 4 comprehensive tests covering broadcast patterns, wildcards, element-wise matching, and null handling

Technical correctness:
The migration correctly uses the existing BooleanArray::from((&str, &[Option<bool>])) implementation (defined in src/daft-core/src/array/from.rs:108-116) which internally converts the slice to daft_arrow::array::BooleanArray using from_trusted_len_iter. The logic for both broadcast and element-wise pattern matching remains unchanged and correct.

Confidence Score: 5/5

  • This PR is safe to merge with no identified issues
  • The migration is straightforward and mechanically correct, replacing arrow2 collection with Vec<Option> while preserving all existing logic. The added test coverage comprehensively validates the implementation's correctness across multiple scenarios. No custom instruction violations or logical issues found.
  • No files require special attention

Important Files Changed

Filename Overview
src/daft-functions-utf8/src/ilike.rs Successfully migrated from arrow2 daft_arrow::array::BooleanArray to native Daft APIs using Vec<Option<bool>> and comprehensive test coverage added

Sequence Diagram

sequenceDiagram
    participant Caller
    participant ilike_impl
    participant parse_inputs
    participant create_broadcasted_str_iter
    participant compile_like_regex
    participant BooleanArray

    Caller->>ilike_impl: ilike_impl(arr, pattern)
    ilike_impl->>parse_inputs: Validate inputs
    parse_inputs-->>ilike_impl: (is_full_null, expected_size)
    
    alt is_full_null
        ilike_impl->>BooleanArray: full_null()
        BooleanArray-->>Caller: Return null array
    else expected_size == 0
        ilike_impl->>BooleanArray: empty()
        BooleanArray-->>Caller: Return empty array
    else normal case
        ilike_impl->>create_broadcasted_str_iter: Get self_iter
        
        alt pattern.len() == 1 (broadcast)
            ilike_impl->>compile_like_regex: Compile single pattern
            compile_like_regex-->>ilike_impl: Regex
            loop for each value
                ilike_impl->>ilike_impl: Apply regex.is_match()
            end
            ilike_impl->>ilike_impl: collect() to Vec<Option<bool>>
        else element-wise matching
            loop for each (value, pattern) pair
                alt pattern in cache
                    ilike_impl->>ilike_impl: Reuse cached regex
                else pattern not in cache
                    ilike_impl->>compile_like_regex: Compile pattern
                    compile_like_regex-->>ilike_impl: Regex
                    ilike_impl->>ilike_impl: Cache regex
                end
                ilike_impl->>ilike_impl: Apply regex.is_match()
            end
            ilike_impl->>ilike_impl: collect() to Vec<Option<bool>>
        end
        
        ilike_impl->>BooleanArray: from((name, result_vec.as_slice()))
        BooleanArray-->>ilike_impl: BooleanArray instance
        ilike_impl->>ilike_impl: assert_eq!(result.len(), expected_size)
        ilike_impl-->>Caller: Return BooleanArray
    end
Loading

@codecov
Copy link

codecov bot commented Jan 6, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.48%. Comparing base (dc57405) to head (8bb568e).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #5936      +/-   ##
==========================================
- Coverage   72.48%   72.48%   -0.01%     
==========================================
  Files         966      966              
  Lines      125839   125870      +31     
==========================================
+ Hits        91213    91232      +19     
- Misses      34626    34638      +12     
Files with missing lines Coverage Δ
src/daft-functions-utf8/src/ilike.rs 95.37% <100.00%> (+2.72%) ⬆️

... and 10 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants