Skip to content

Conversation

ethe
Copy link
Member

@ethe ethe commented Aug 12, 2025

  • Add query module with dynamic.rs, expression.rs, error.rs, and core mod.rs.
  • Dynamic AST: ExprOwned, PredOwned, Selector, BoundOwned with resolver to ResolvedExpr.
  • DSL predicates: Eq, Range, Prefix (upper-exclusive), In, IsNull, IsNotNull; logic: And, Or, Not.
  • Schema-generic APIs: Expression::resolve<S: Schema>(&S) and ColumnSelector::resolve_col<S: Schema>(&S).
  • Selectors: Col<'a> (name) and ColIndex<const I: usize> (index) with Arrow FieldRef in ResolvedSelector.
  • Helpers: add assert_value_type, derive_utf8_prefix_upper, derive_binary_prefix_upper.
  • Value ergonomics: implement From<T> for Value for common scalars, strings, and binary (enables Into<Value> usage).
  • Crate root: expose pub mod query in src/lib.rs.

ethe added 5 commits August 11, 2025 20:43
- Key API: replace `Key::to_arrow_datum` with `Key::to_arrow_datums` to supportmulti-component keys; update all builtin `Key` impls (ints/floats/string/date/time/timestamp/dynamic `Value`) to return a single-element Vec for single-PK types.
- Composite key showcase: add `Key2`/`Key2Ref` under `#[cfg(test)]` with `Encode`/`Decode`, `Ord`, `Hash`, `Key`/`KeyRef`; round-trip and ordering tests included.
- Parquet scan filtering: implement lexicographic composite range predicates in`get_range_filter` using `eq/gt/gt_eq/lt/lt_eq` with `and_kleene`/`or_kleene`; retain `_ts` upper-bound predicate.
- Scan plumbing: thread `pk_indices` through `SsTable::{get,scan}`, `Version::{query,streams,table_query}`, and `LevelStream`; update test callers. Build fixed projections as `[0, 1] ∪ pk_indices` in read paths.
- Docs: update composite PK RFC with “Current Status (2025-08-11)”; add draft RFC “Replace Schema Trait with Arrow Schema”.
- Clippy hygiene: remove needless range loops and redundant `.into_iter()`; add a focused `#[allow(clippy::too_many_arguments)]` on `Version::table_query`.

BREAKING CHANGE:
- `Key::to_arrow_datum` was replaced by `Key::to_arrow_datums(Vec<Arc<dyn Datum>>)`; external `Key` implementors must migrate to the new method (return one datum per PK component).
…ric resolution

- Add `query` module with `dynamic.rs`, `expression.rs`, `error.rs`, and core `mod.rs`.
- Dynamic AST: `ExprOwned`, `PredOwned`, `Selector`, `BoundOwned` with resolver to `ResolvedExpr`.
- DSL predicates: `Eq`, `Range`, `Prefix` (upper-exclusive), `In`, `IsNull`, `IsNotNull`; logic: `And`, `Or`, `Not`.
- Schema-generic APIs: `Expression::resolve<S: Schema>(&S)` and `ColumnSelector::resolve_col<S: Schema>(&S)`.
- Selectors: `Col<'a>` (name) and `ColIndex<const I: usize>` (index) with Arrow `FieldRef` in `ResolvedSelector`.
- Helpers: add `assert_value_type`, `derive_utf8_prefix_upper`, `derive_binary_prefix_upper`.
- Value ergonomics: implement `From<T> for Value` for common scalars, strings, and binary (enables `Into<Value>` usage).
- Crate root: expose `pub mod query` in `src/lib.rs`.
Copy link

codecov bot commented Aug 12, 2025

Codecov Report

❌ Patch coverage is 58.60215% with 231 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/query/dynamic.rs 49.24% 101 Missing ⚠️
src/query/expression.rs 48.64% 76 Missing ⚠️
src/record/dynamic/value/mod.rs 20.00% 36 Missing ⚠️
src/query/mod.rs 89.15% 18 Missing ⚠️

📢 Thoughts on this report? Let us know!

Copy link
Collaborator

@jonathanc-n jonathanc-n left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also note that we eventually want type coercion in the RFC

s
}

fn derive_binary_prefix_upper(prefix: &[u8]) -> Vec<u8> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theres a very minor edgecase where prefix + 0xFF + byte... will not be included.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we only support prefix matching on UTF8 encoding, it could be avoided.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants