Skip to content

Commit 0848e23

Browse files
committed
refactor(spatial): replace raw geometry bytes with typed Geometry throughout
The query geometry was previously carried as Vec<u8> / &[u8] across the entire spatial pipeline and parsed in the Data Plane handler, meaning malformed input was only caught at execution time with no validation feedback at the SQL or native protocol layer. Parse and validate the geometry once on the Control Plane — at the SQL planner (where_search) and native plan builder — then propagate the typed nodedb_types::geometry::Geometry value through SqlPlan, PhysicalPlan, and the distributed scatter coordinator. The Data Plane handler receives a pre-validated value and no longer needs to parse. Geometry validation (via nodedb_spatial::validate::validate_geometry) now runs at query entry points and returns a structured SQL error rather than an Internal error from the executor. Add integration test spatial_cp_dp_query covering the full CP→DP round trip for a spatial scan with attribute filters.
1 parent a81cb7e commit 0848e23

12 files changed

Lines changed: 112 additions & 44 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

nodedb-cluster/src/distributed_spatial/coordinator.rs

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -12,8 +12,8 @@ pub struct SpatialScatterPayload {
1212
pub collection: String,
1313
pub field: String,
1414
pub predicate: String,
15-
/// Raw query geometry bytes (GeoJSON, passed through as-is).
16-
pub query_geometry: Vec<u8>,
15+
/// Typed query geometry, parsed and validated on the originating CP.
16+
pub query_geometry: nodedb_types::geometry::Geometry,
1717
pub distance_meters: f64,
1818
pub limit: u32,
1919
}
@@ -44,15 +44,15 @@ impl SpatialScatterGather {
4444
collection: &str,
4545
field: &str,
4646
predicate: &str,
47-
query_geometry_json: &[u8],
47+
query_geometry: nodedb_types::geometry::Geometry,
4848
distance_meters: f64,
4949
limit: usize,
5050
) -> Vec<(u32, VShardEnvelope)> {
5151
let msg = SpatialScatterPayload {
5252
collection: collection.to_string(),
5353
field: field.to_string(),
5454
predicate: predicate.to_string(),
55-
query_geometry: query_geometry_json.to_vec(),
55+
query_geometry,
5656
distance_meters,
5757
limit: limit as u32,
5858
};
@@ -106,11 +106,14 @@ mod tests {
106106
#[test]
107107
fn scatter_envelopes_built() {
108108
let coord = SpatialScatterGather::new(1, vec![0, 1, 2]);
109-
let query =
110-
serde_json::to_vec(&serde_json::json!({"type": "Point", "coordinates": [0.0, 0.0]}))
111-
.unwrap();
112-
let envs =
113-
coord.build_scatter_envelopes("buildings", "geom", "st_dwithin", &query, 1000.0, 100);
109+
let envs = coord.build_scatter_envelopes(
110+
"buildings",
111+
"geom",
112+
"st_dwithin",
113+
nodedb_types::geometry::Geometry::point(0.0, 0.0),
114+
1000.0,
115+
100,
116+
);
114117
assert_eq!(envs.len(), 3);
115118
for (shard_id, env) in &envs {
116119
assert_eq!(env.msg_type, VShardMessageType::SpatialScatterRequest);

nodedb-sql/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ description = "SQL parser, planner, and optimizer for NodeDB"
99
[dependencies]
1010
nodedb-types = { workspace = true }
1111
nodedb-query = { workspace = true }
12+
nodedb-spatial = { workspace = true }
1213
sqlparser = { version = "0.61", features = ["visitor"] }
1314
thiserror = { workspace = true }
1415
chrono = { workspace = true }

nodedb-sql/src/planner/select/where_search.rs

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,16 @@ fn plan_spatial_from_where(
9797
context: name.into(),
9898
})?;
9999
let geom_str = extract_geometry_arg(geom_arg)?;
100+
let geometry: nodedb_types::geometry::Geometry =
101+
sonic_rs::from_str(&geom_str).map_err(|e| SqlError::InvalidFunction {
102+
detail: format!("invalid geometry in {name}: {e}"),
103+
})?;
104+
let issues = nodedb_spatial::validate::validate_geometry(&geometry);
105+
if !issues.is_empty() {
106+
return Err(SqlError::InvalidFunction {
107+
detail: format!("invalid geometry in {name}: {}", issues.join("; ")),
108+
});
109+
}
100110
let distance = if args.len() >= 3 {
101111
extract_float(&args[2]).unwrap_or(0.0)
102112
} else {
@@ -106,7 +116,7 @@ fn plan_spatial_from_where(
106116
collection: table.name.clone(),
107117
field,
108118
predicate,
109-
query_geometry: geom_str.into_bytes(),
119+
query_geometry: geometry,
110120
distance_meters: distance,
111121
attribute_filters: Vec::new(),
112122
limit: 1000,

nodedb-sql/src/types/plan.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -361,7 +361,7 @@ pub enum SqlPlan {
361361
collection: String,
362362
field: String,
363363
predicate: SpatialPredicate,
364-
query_geometry: Vec<u8>,
364+
query_geometry: nodedb_types::geometry::Geometry,
365365
distance_meters: f64,
366366
attribute_filters: Vec<Filter>,
367367
limit: usize,

nodedb/src/bridge/physical_plan/spatial.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
//! Spatial engine operations dispatched to the Data Plane.
22
3-
use nodedb_types::SurrogateBitmap;
3+
use nodedb_types::{SurrogateBitmap, geometry::Geometry};
44

55
/// Spatial predicate type for R-tree index scan.
66
#[derive(
@@ -42,8 +42,8 @@ pub enum SpatialOp {
4242
collection: String,
4343
field: String,
4444
predicate: SpatialPredicate,
45-
/// Query geometry serialized as GeoJSON bytes.
46-
query_geometry: Vec<u8>,
45+
/// Typed query geometry, parsed and validated on the Control Plane.
46+
query_geometry: Geometry,
4747
/// Distance threshold in meters (for ST_DWithin). 0 for non-distance predicates.
4848
distance_meters: f64,
4949
/// Additional attribute filters applied after spatial candidates.

nodedb/src/bridge/physical_plan/wire.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -208,7 +208,7 @@ mod tests {
208208
collection: "places".into(),
209209
field: "location".into(),
210210
predicate: SpatialPredicate::DWithin,
211-
query_geometry: b"{}".to_vec(),
211+
query_geometry: nodedb_types::geometry::Geometry::point(0.0, 0.0),
212212
distance_meters: 500.0,
213213
attribute_filters: vec![],
214214
limit: 20,

nodedb/src/control/planner/sql_plan_convert/scan.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -733,7 +733,7 @@ pub(super) fn convert_spatial_scan(p: SpatialScanParams<'_>) -> crate::Result<Ve
733733
collection: collection.into(),
734734
field: field.to_string(),
735735
predicate: sp,
736-
query_geometry: query_geometry.to_vec(),
736+
query_geometry: query_geometry.clone(),
737737
distance_meters: *distance_meters,
738738
attribute_filters: attr_bytes,
739739
limit: *limit,

nodedb/src/control/planner/sql_plan_convert/scan_params.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ pub(super) struct SpatialScanParams<'a> {
105105
pub collection: &'a str,
106106
pub field: &'a str,
107107
pub predicate: &'a nodedb_sql::types::SpatialPredicate,
108-
pub query_geometry: &'a [u8],
108+
pub query_geometry: &'a nodedb_types::geometry::Geometry,
109109
pub distance_meters: &'a f64,
110110
pub attribute_filters: &'a [Filter],
111111
pub limit: &'a usize,

nodedb/src/control/server/native/dispatch/plan_builder/spatial.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,24 @@ use crate::bridge::envelope::PhysicalPlan;
66
use crate::bridge::physical_plan::{SpatialOp, SpatialPredicate};
77

88
pub(crate) fn build_scan(fields: &TextFields, collection: &str) -> crate::Result<PhysicalPlan> {
9-
let query_geometry = fields
9+
let raw_bytes = fields
1010
.query_geometry
1111
.as_ref()
1212
.ok_or_else(|| crate::Error::BadRequest {
1313
detail: "missing 'query_geometry'".to_string(),
14-
})?
15-
.clone();
14+
})?;
15+
16+
let geometry: nodedb_types::geometry::Geometry =
17+
sonic_rs::from_slice(raw_bytes).map_err(|e| crate::Error::BadRequest {
18+
detail: format!("invalid query geometry: {e}"),
19+
})?;
20+
21+
let issues = nodedb_spatial::validate::validate_geometry(&geometry);
22+
if !issues.is_empty() {
23+
return Err(crate::Error::BadRequest {
24+
detail: format!("invalid query geometry: {}", issues.join("; ")),
25+
});
26+
}
1627

1728
let predicate_str = fields.spatial_predicate.as_deref().unwrap_or("dwithin");
1829
let predicate = match predicate_str.to_lowercase().as_str() {
@@ -35,7 +46,7 @@ pub(crate) fn build_scan(fields: &TextFields, collection: &str) -> crate::Result
3546
collection: collection.to_string(),
3647
field,
3748
predicate,
38-
query_geometry,
49+
query_geometry: geometry,
3950
distance_meters,
4051
attribute_filters: Vec::new(),
4152
limit,

0 commit comments

Comments
 (0)