Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ Set `OMNIGRAPH_UPDATE_OPENAPI=1` only when the drift is intentional.
process environment, concurrency).
- Every issue fix lands a regression test at the cheapest tier that catches
the defect: a `.gqt` logic test when the defect is visible in rows, counts,
or errors, a `_issue_NNN` Rust test when it needs mechanism or scale
result column types, or errors, a `_issue_NNN` Rust test when it needs mechanism or scale
assertions; when the reported symptom additionally needs scale to
manifest, a second `#[ignore]`d test in a `tests/repro_issue_*.rs` target
guards it, and the two cross-reference each other in comments.
Expand Down
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/omnigraph-compiler/src/query/typecheck.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1691,7 +1691,7 @@ fn infer_projection_field(
/// spelling and names an unaliased property by the property alone; the two
/// spellings drift for unaliased projections today, and this function
/// follows the executor because T25 guards the batch the executor builds.
fn executed_column_name(expr: &Expr, alias: Option<&str>) -> String {
pub fn executed_column_name(expr: &Expr, alias: Option<&str>) -> String {
if let Some(alias) = alias {
return alias.to_string();
}
Expand Down
112 changes: 112 additions & 0 deletions crates/omnigraph-compiler/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,32 @@ impl ScalarType {
}
}

/// The inverse of [`Self::to_arrow`] over its image; `None` for an Arrow
/// type no scalar maps to.
pub fn from_arrow(data_type: &DataType) -> Option<Self> {
Some(match data_type {
DataType::Utf8 => Self::String,
DataType::Boolean => Self::Bool,
DataType::Int32 => Self::I32,
DataType::Int64 => Self::I64,
DataType::UInt32 => Self::U32,
DataType::UInt64 => Self::U64,
DataType::Float32 => Self::F32,
DataType::Float64 => Self::F64,
DataType::Date32 => Self::Date,
DataType::Date64 => Self::DateTime,
DataType::LargeBinary => Self::Blob,
DataType::FixedSizeList(item, dim)
if item.name() == "item"
&& item.is_nullable()
&& *item.data_type() == DataType::Float32 =>
{
Self::Vector(u32::try_from(*dim).ok()?)
}
_ => return None,
})
}

pub fn is_numeric(&self) -> bool {
matches!(
self,
Expand Down Expand Up @@ -163,6 +189,21 @@ impl PropType {
}
}

/// The inverse of [`Self::to_arrow`] over its image, non-nullable and
/// enum-free (both are erased by `to_arrow`); `None` outside the image.
pub fn from_arrow(data_type: &DataType) -> Option<Self> {
if let DataType::List(item) = data_type {
if item.name() != "item" || !item.is_nullable() {
return None;
}
return Some(Self::list_of(
ScalarType::from_arrow(item.data_type())?,
false,
));
}
Some(Self::scalar(ScalarType::from_arrow(data_type)?, false))
}

pub fn display_name(&self) -> String {
let base = if let Some(values) = &self.enum_values {
format!("enum({})", values.join(", "))
Expand Down Expand Up @@ -216,6 +257,77 @@ mod tests {
);
}

const EVERY_SCALAR: [ScalarType; 12] = [
ScalarType::String,
ScalarType::Bool,
ScalarType::I32,
ScalarType::I64,
ScalarType::U32,
ScalarType::U64,
ScalarType::F32,
ScalarType::F64,
ScalarType::Date,
ScalarType::DateTime,
ScalarType::Vector(3),
ScalarType::Blob,
];

#[test]
fn from_arrow_inverts_to_arrow_over_every_scalar_list_and_nullability() {
for scalar in EVERY_SCALAR {
assert_eq!(ScalarType::from_arrow(&scalar.to_arrow()), Some(scalar));
for list in [false, true] {
for nullable in [false, true] {
let prop = if list {
PropType::list_of(scalar, nullable)
} else {
PropType::scalar(scalar, nullable)
};
let mut expected = prop.clone();
expected.nullable = false;
assert_eq!(
PropType::from_arrow(&prop.to_arrow()),
Some(expected),
"{prop:?}"
);
}
}
}
assert_eq!(
PropType::from_arrow(&PropType::enum_type(vec!["a".into()], true).to_arrow()),
Some(PropType::scalar(ScalarType::String, false))
);
assert_eq!(
ScalarType::from_arrow(&DataType::Struct(Default::default())),
None
);
assert_eq!(ScalarType::from_arrow(&DataType::Int8), None);
}

#[test]
fn to_arrow_image_table_is_pinned() {
let table: [(ScalarType, DataType); 11] = [
(ScalarType::String, DataType::Utf8),
(ScalarType::Bool, DataType::Boolean),
(ScalarType::I32, DataType::Int32),
(ScalarType::I64, DataType::Int64),
(ScalarType::U32, DataType::UInt32),
(ScalarType::U64, DataType::UInt64),
(ScalarType::F32, DataType::Float32),
(ScalarType::F64, DataType::Float64),
(ScalarType::Date, DataType::Date32),
(ScalarType::DateTime, DataType::Date64),
(ScalarType::Blob, DataType::LargeBinary),
];
for (scalar, data_type) in table {
assert_eq!(scalar.to_arrow(), data_type, "{scalar:?}");
}
assert_eq!(
PropType::list_of(ScalarType::I32, true).to_arrow(),
DataType::List(Arc::new(Field::new("item", DataType::Int32, true)))
);
}

#[test]
fn prop_type_from_param_type_name_supports_lists_and_nullable_scalars() {
assert_eq!(
Expand Down
2 changes: 2 additions & 0 deletions crates/omnigraph-gqt/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ publish = false
doctest = false

[dependencies]
arrow-schema = { workspace = true }
futures = { workspace = true }
omnigraph = { package = "omnigraph-engine", path = "../omnigraph", version = "0.10.0" }
omnigraph-compiler = { path = "../omnigraph-compiler", version = "0.10.0" }
Expand All @@ -21,6 +22,7 @@ tempfile = { workspace = true }
tokio = { workspace = true }

[dev-dependencies]
arrow-array = { workspace = true }
datatest-stable = "0.3"

# One libtest test per `cases/*.gqt`; mechanism and flags: tests/gq_logic_tests.rs.
Expand Down
7 changes: 5 additions & 2 deletions crates/omnigraph-gqt/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,11 @@ or `--workspace` reaches it.
`issue_NNN_<short_name>.gqt`; `scripts/check-fix-regression.py` looks for it
here.
- `src/lib.rs`: the runner (case parsing, execution against a fresh
temporary store, row comparison, bless). Format self-tests and the corpus
layout check are its unit tests (`src/tests.rs`).
temporary store, the `--- expect shape` check of each rows step's result
columns, the result-schema check against the compiler's inferred schema,
row comparison, bless); `src/shape.rs` parses and compares the shape
section. Format self-tests and the corpus layout check are its unit
tests (`src/tests.rs`).
- `tests/gq_logic_tests.rs`: one libtest test per case, named
`case::<file>.gqt`, registered at run time by `datatest-stable`
(`harness = false`). A new case file is picked up without any Rust change.
Expand Down
3 changes: 3 additions & 0 deletions crates/omnigraph-gqt/cases/issue_563_aggregate_uncapped.gqt
Original file line number Diff line number Diff line change
Expand Up @@ -63,3 +63,6 @@ query recall_count($q: String) {

--- expect unordered
{"total": 20}

--- expect shape
total: I64
4 changes: 4 additions & 0 deletions crates/omnigraph-gqt/cases/issue_563_underfill_retry.gqt
Original file line number Diff line number Diff line change
Expand Up @@ -66,3 +66,7 @@ query recall($q: String) {
--- expect ordered
{"c.slug": "chunk-08", "a.slug": "art-0"}
{"c.slug": "chunk-09", "a.slug": "art-0"}

--- expect shape
c.slug: String
a.slug: String
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,6 @@ query not_prefixed() {

--- expect unordered
{"p.name": "bob"}

--- expect shape
p.name: String
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,9 @@ query knows_anyone_twice() {
{"p.name": "alice"}
{"p.name": "bob"}

--- expect shape
p.name: String

--- query
query bound_twice() {
match {
Expand All @@ -43,6 +46,9 @@ query bound_twice() {
--- expect unordered
{"p.name": "bob"}

--- expect shape
p.name: String

--- query
query bound_twice_in_negation() {
match {
Expand All @@ -55,6 +61,9 @@ query bound_twice_in_negation() {
--- expect unordered
{"p.name": "alice"}

--- expect shape
p.name: String

--- query
query self_loop_twice() {
match {
Expand All @@ -68,6 +77,9 @@ query self_loop_twice() {
--- expect unordered
{"p.name": "bob"}

--- expect shape
p.name: String

--- query
query rebind_inside_negation_non_root() {
match {
Expand All @@ -85,3 +97,6 @@ query rebind_inside_negation_non_root() {
--- expect unordered
{"p.name": "alice"}
{"p.name": "bob"}

--- expect shape
p.name: String
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ query exactly_two_hops() {

--- expect unordered

--- expect shape
p.name: String
f.name: String

--- query
query one_to_two_hops() {
match {
Expand All @@ -37,3 +41,7 @@ query one_to_two_hops() {
--- expect unordered
{"p.name": "alice", "f.name": "bob"}
{"p.name": "bob", "f.name": "bob"}

--- expect shape
p.name: String
f.name: String
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,10 @@ query point_values() {
--- expect unordered
{"p.name": "p", "p.score": 0.99, "p.embedding": [0.1, 0.2, 0.3], "p.at": "2024-01-01T11:54:56.789"}
{"p.name": "q", "p.score": 1, "p.embedding": [1, 2, 3], "p.at": "2024-01-01T00:00:00", "p.note": "kept"}

--- expect shape
p.name: String
p.score: F32
p.embedding: Vector(3)
p.at: DateTime
p.note: String?
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,10 @@ query distinct_aliases() {
--- expect unordered
{"who": "p", "how_many": 7}

--- expect shape
who: String
how_many: I64

--- query
query property_and_aliased_aggregate() {
match {
Expand All @@ -126,6 +130,10 @@ query property_and_aliased_aggregate() {
--- expect unordered
{"a.name": "p", "n": 1}

--- expect shape
a.name: String
n: I64

--- query
query two_aliased_literals() {
match {
Expand All @@ -136,3 +144,7 @@ query two_aliased_literals() {

--- expect unordered
{"one": 1, "two": 2}

--- expect shape
one: I64
two: I64
4 changes: 4 additions & 0 deletions crates/omnigraph-gqt/cases/ordered_two_key_sort.gqt
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,7 @@ query ranked() {
{"p.name": "dave", "p.rank": 1}
{"p.name": "alice", "p.rank": 2}
{"p.name": "carol", "p.rank": 2}

--- expect shape
p.name: String
p.rank: I64
3 changes: 3 additions & 0 deletions crates/omnigraph-gqt/cases/restart_survives_reopen.gqt
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,6 @@ query all_names() {
{"p.name": "alice"}
{"p.name": "bob"}
{"p.name": "carol"}

--- expect shape
p.name: String
Loading