You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "james-willis (via GitHub)" <gi...@apache.org> on 2026/05/19 04:47:59 UTC
[PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
james-willis opened a new pull request, #858:
URL: https://github.com/apache/sedona-db/pull/858
## Summary
Adds support for loading zarrs as rasters. Each chunk becomes one row, similar to how each tiff becomes one row.
```sql
SELECT raster FROM sd_read_zarr('file:///path/to/datacube.zarr');
SELECT count(*) FROM sd_read_zarr(
'file:///path/to/datacube.zarr',
'{"mode": "outdb", "rows_per_batch": 256}'
);
```
Rasters table is returns as a single column table with the column named `raster`. For now, chunks are eagerly loaded as part of the sd_read_zarr operation. When lazy loading support lands in #849 I will add lazy zarr support and make that the default.
We built on the zarrs 0.23 craterather than GDAL MultiDim because two GDAL gaps are independently disqualifying: Zarr v3 sharding is unreadable in GDAL ≤ 3.12 (sharded Zarr is a critical cloud-native feature IMO), and vlen-utf8 string coordinate variables are unreadable through GDAL MultiDim (climate/datacube Zarr uses these for band names).
## What's in this PR
- New crate sedona-raster-zarr with `group_to_indb_rasters(uri)` and `group_to_outdb_rasters(uri)` entry points.
- GeoZarr metadata parsing: proj:wkt2 / proj:projjson / proj:epsg (in that precedence) for CRS; spatial:transform + spatial:dims for affine + spatial axes.
- Group-constraint validation at load time: shared chunk grid, chunk shape, and dim names across the group; named errors on the offending array.
- `sd_read_zarr` UDTF registered in `SedonaContext::new_from_context`. JSON options: `mode` (default "indb"), `rows_per_batch` (default 1024), `num_partitions` (default 1).
- Empty/partial projection support via ProjectionExec wrap, so `SELECT count(*) FROM sd_read_zarr(...)` works.
## Out of scope (separate follow-up PR)
- lazy byte resolver — the function the byte-loading hook calls to fetch zarr chunks on demand. Depends on #849.
- Cloud storage backends (zarrs_object_store for s3://, gs://, az://, https://) and the async runtime story they need. Phase 1 errors clearly on cloud schemes.
- Round-robin chunk partitioning past `num_partitions = 1`.
## Open questions for reviewers
1. Hard zarrs dependency on the sedona crate. Should I make the zarr crate optional for the top level dependency
2. Naming. `sd_read_zarr` matches the existing `sd_random_geometry` UDTF in this repo. The wider DataFusion ecosystem also uses the <format>_scan pattern (e.g. delta_scan). Happy to rename to `sd_zarr_scan` if reviewers prefer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3277022724
##########
rust/sedona-raster-zarr/src/geozarr.rs:
##########
@@ -0,0 +1,228 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! GeoZarr attribute parsing for CRS and affine transform.
+//!
+//! Reads the `proj:*` (CRS) and `spatial:*` (transform / spatial dim
+//! mapping) attribute conventions from a Zarr group's attributes, mapping
+//! them onto SedonaDB's per-raster `crs` and `transform` fields.
+//!
+//! Attributes live at the group level and are inherited by every array.
+//! Per-array overrides are rejected by the group-constraint validator
+//! (see `loader`).
+
+use arrow_schema::ArrowError;
+
+/// Per-group geo metadata distilled from `proj:*` / `spatial:*` attributes.
+#[derive(Debug, Clone, PartialEq)]
+pub struct GroupGeoMetadata {
+ /// CRS string in PROJ or WKT format (whichever the group declared).
+ /// `None` if no `proj:wkt2` / `proj:projjson` / `proj:epsg` attribute
+ /// is present on the group.
+ pub crs: Option<String>,
+ /// Affine transform in GDAL GeoTransform order:
+ /// `[origin_x, scale_x, skew_x, origin_y, skew_y, scale_y]`. `None`
+ /// when no `spatial:transform` attribute is present.
+ pub transform: Option<[f64; 6]>,
Review Comment:
I filed this as a follow up in my notes. The best way to make this change would involve plumbing such an object through to the trait surface so I dont want to take that on in this PR because it would involve touching the core trait which doesnt seem in scope to me.
`GeoTransform` + `GeoTransformEx::apply/invert` already exist in `sedona-gdal` but I don't want to bring a dependency on gdal in here. So that would probably warrant a second refactor of crates I don't want to touch right now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3269074474
##########
rust/sedona-raster-zarr/src/loader.rs:
##########
@@ -0,0 +1,513 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Zarr group → N-D raster `StructArray` entry points.
+//!
+//! Both `group_to_indb_rasters` and `group_to_outdb_rasters` produce the
+//! same row shape: one raster row per chunk position, with one band per
+//! array in the group. They differ only in how each row's pixel bytes
+//! are delivered:
+//!
+//! - **InDb** — every chunk is fetched eagerly and copied into the
+//! Arrow `data` column. Heavy for large datacubes; intended for
+//! snapshots.
+//! - **OutDb** — `data` is left empty; each band's `outdb_uri` carries a
+//! chunk anchor (`zarr://<store-uri>/<array-path>#chunk=i0,i1,...`).
+//! Byte resolution awaits the format-keyed dispatch work in a
+//! follow-up PR.
+
+use std::sync::Arc;
+
+use arrow_array::StructArray;
+use arrow_schema::ArrowError;
+use sedona_common::sedona_internal_datafusion_err;
+use sedona_raster::builder::RasterBuilder;
+use sedona_schema::raster::BandDataType;
+use zarrs::array::{Array, ArrayBytes};
+use zarrs::group::Group;
+use zarrs_filesystem::FilesystemStore;
+
+use crate::dtype::zarr_to_band_data_type;
+use crate::geozarr::GroupGeoMetadata;
+use crate::source_uri::{build_chunk_anchor, group_uri_to_filesystem_path};
+
+/// Open a Zarr group and eagerly fetch every chunk's bytes into the
+/// returned `StructArray`. Each row holds one chunk position's data
+/// across every array in the group.
+pub fn group_to_indb_rasters(group_uri: &str) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::InDb)
+}
+
+/// Open a Zarr group and emit one row per chunk position with chunk-anchor
+/// URIs in each band's `outdb_uri`. The `data` column is empty; bytes
+/// resolve on demand through whichever OutDb loader is registered for
+/// the `zarr` format.
+pub fn group_to_outdb_rasters(group_uri: &str) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::OutDb)
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum Mode {
+ InDb,
+ OutDb,
+}
+
+/// Per-array metadata extracted once at group open and reused for every
+/// chunk position. Caching this avoids re-reading Zarr metadata for each
+/// of the (potentially thousands of) chunk rows.
+struct ArrayInfo {
+ /// Array path within the store, used to build chunk anchor URIs and
+ /// surface in band names.
+ path: String,
+ /// Open zarrs handle.
+ array: Array<FilesystemStore>,
+ /// SedonaDB BandDataType corresponding to this array's zarrs dtype.
+ data_type: BandDataType,
+ /// Dimension names in array order. Required to be `Some(_)` for every
+ /// dim; missing names error at validation time.
+ dim_names: Vec<String>,
+ /// Inner chunk grid shape, one entry per dimension. Used to enumerate
+ /// chunk positions and validated to match across arrays.
+ chunk_grid_shape: Vec<u64>,
+ /// Chunk shape (elements per chunk per dim). Same for every chunk
+ /// position in Phase 1 (no ragged final chunks emitted as separate
+ /// short rows).
+ chunk_shape: Vec<u64>,
+ /// Encoded fill value in native-endian byte representation, for the
+ /// `nodata` field. None when the array has no fill value declared.
+ nodata: Option<Vec<u8>>,
+}
+
+fn build_rasters(group_uri: &str, mode: Mode) -> Result<StructArray, ArrowError> {
+ let fs_path = group_uri_to_filesystem_path(group_uri)?;
+ let store = FilesystemStore::new(&fs_path).map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr filesystem store at {}: {e}",
+ fs_path.display()
+ )))
+ })?;
+ let storage: Arc<FilesystemStore> = Arc::new(store);
+
+ let group = Group::open(storage.clone(), "/").map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr group at {group_uri}: {e}"
+ )))
+ })?;
+
+ let geo = GroupGeoMetadata::from_attributes(group.attributes())?;
+
+ let arrays = group.child_arrays().map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to enumerate child arrays in {group_uri}: {e}"
+ )))
+ })?;
+ if arrays.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} has no child arrays"
+ )));
+ }
+
+ let array_infos = collect_array_infos(arrays)?;
+ validate_group_constraints(&array_infos)?;
+
+ // Spatial-dim resolution. Phase 1 supports two configurations:
+ // - dim_names ends with ["y", "x"] (canonical for georeferenced
+ // 2-D and time-series rasters); the spatial extent is the chunk's
+ // last two dims.
+ // - `spatial:dims` attribute on the group explicitly names them.
+ // Anything else errors with a clear message — silently picking dims
+ // would produce wrong per-row transforms.
+ let spatial_dim_indices =
+ resolve_spatial_dim_indices(&array_infos[0].dim_names, geo.spatial_dims.as_deref())?;
+ let spatial_dims_names: Vec<&str> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].dim_names[i].as_str())
+ .collect();
+ let chunk_spatial_shape: Vec<i64> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].chunk_shape[i] as i64)
+ .collect();
+
+ let group_transform = geo.transform.unwrap_or([0.0, 1.0, 0.0, 0.0, 0.0, -1.0]);
Review Comment:
Currently the type implementation does not allow null geotransforms so we need to provide this potentially misleading value here when geozarr metadata is not present.
Havent considered what to do about this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3284196455
##########
python/sedonadb-zarr/src/lib.rs:
##########
@@ -0,0 +1,126 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! `sedonadb-zarr` — Python plugin package wiring Zarr support into a
+//! `sedonadb` session.
+//!
+//! Two PyO3-exposed surfaces:
+//! - `zarr_udtf_capsule()` — builds a `PyCapsule` carrying the
+//! `sd_read_zarr` UDTF implementation. The Python wrapper hands it
+//! to sedonadb's `InternalContext.register_udtf_capsule` to attach
+//! the SQL function.
+//! - `PyZarrChunkReader` — a streaming reader producible from Python,
+//! exposing `__arrow_c_stream__` so it plugs into
+//! `ExternalFormatSpec.open_reader` (the `con.read_format(spec, uri)`
+//! surface).
+//!
+//! The Python `ZarrFormatSpec(ExternalFormatSpec)` class in
+//! `sedonadb_zarr/__init__.py` wraps `PyZarrChunkReader`; the Rust side
+//! here is intentionally thin.
+
+use std::ffi::CString;
+use std::sync::{Arc, Mutex};
+
+use arrow_array::ffi_stream::FFI_ArrowArrayStream;
+use datafusion::catalog::TableFunctionImpl;
+use pyo3::exceptions::{PyRuntimeError, PyValueError};
+use pyo3::prelude::*;
+use pyo3::types::PyCapsule;
+use sedona_raster_zarr::{ZarrChunkReader, ZarrReadFunction};
+// `sedonadb`'s rustc crate name is `_lib` (set by `[lib].name` so
+// maturin packages it as `sedonadb._lib`); import the plugin handoff
+// types under that name.
+use _lib::plugin::{UdtfCapsule, UDTF_CAPSULE_NAME};
+
+/// Build a PyCapsule carrying the Zarr UDTF for handoff to sedonadb's
+/// `InternalContext.register_udtf_capsule`.
+///
+/// Cross-extension `#[pyclass]` extraction doesn't work in PyO3 (each
+/// cdylib has its own type-id static), so we pass the UDTF
+/// implementation across the extension boundary via an opaque
+/// [`UdtfCapsule`]. The capsule owns its `Arc`; the consumer clones a
+/// fresh refcount before the capsule is dropped. The payload's leading
+/// magic sentinel lets the consumer fail fast if the capsule's name
+/// has been spoofed.
+#[pyfunction]
+fn zarr_udtf_capsule(py: Python<'_>) -> PyResult<Bound<'_, PyCapsule>> {
+ let udtf: Arc<dyn TableFunctionImpl> = Arc::new(ZarrReadFunction::default());
+ let payload = UdtfCapsule::new(udtf);
+ // CString::new requires owned bytes; reuse the host-side constant
+ // so the name (and its `.vN` ABI tag) lives in exactly one place.
+ let name = CString::new(UDTF_CAPSULE_NAME.to_bytes()).unwrap();
+ PyCapsule::new_with_destructor(py, payload, Some(name), |_v: UdtfCapsule, _ctx| {})
+}
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3284396120
##########
python/sedonadb-zarr/src/lib.rs:
##########
@@ -0,0 +1,177 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! `sedonadb-zarr` — Python plugin package wiring Zarr support into a
+//! `sedonadb` session.
+//!
+//! Two PyO3-exposed surfaces:
+//! - [`ZarrTableFunction`] — Python class with
+//! `__datafusion_table_function__(session)` that returns an
+//! `FFI_TableFunction` PyCapsule. sedonadb's
+//! `InternalContext.register_udtf` attaches it under
+//! `sd_read_zarr`.
+//! - [`PyZarrChunkReader`] — streaming reader producible from Python,
+//! exposing `__arrow_c_stream__` so it plugs into
+//! `ExternalFormatSpec.open_reader` (the `con.read_format(spec, uri)`
+//! surface).
+//!
+//! The Rust side carries no dependency on `sedonadb`'s host extension
+//! — UDTF handoff is via `datafusion-ffi`, which gives an ABI-stable
+//! C-level interface so plugin and host cdylibs don't need to share
+//! anything beyond Arrow C Data/Stream and the FFI structs.
+
+use std::ffi::CString;
+use std::ptr::NonNull;
+use std::sync::{Arc, Mutex};
+
+use arrow_array::ffi_stream::FFI_ArrowArrayStream;
+use datafusion_catalog::TableFunctionImpl;
+use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec;
+use datafusion_ffi::udtf::FFI_TableFunction;
+use pyo3::exceptions::{PyRuntimeError, PyValueError};
+use pyo3::prelude::*;
+use pyo3::types::PyCapsule;
+use sedona_raster_zarr::{ZarrChunkReader, ZarrReadFunction};
+
+/// `c"datafusion_table_function"` capsule name expected by sedonadb's
+/// (and datafusion-python's) host-side UDTF registration.
+const UDTF_CAPSULE_NAME: &std::ffi::CStr = c"datafusion_table_function";
+
+/// Codec capsule name the host passes to us via the `session` argument.
+const CODEC_CAPSULE_NAME: &std::ffi::CStr = c"datafusion_logical_extension_codec";
+
+/// Zarr UDTF surface. The Python plugin instantiates one of these and
+/// hands the instance to sedonadb's `con._impl.register_udtf` (Python
+/// wrapper). Sedonadb then calls `__datafusion_table_function__(session)`
+/// on it to obtain the FFI struct.
+#[pyclass(name = "ZarrTableFunction", module = "sedonadb_zarr")]
+#[derive(Default, Debug, Clone)]
+pub struct ZarrTableFunction;
Review Comment:
It doesnt need to exist, but I think its a much better interface.
I think it is fair to split it out into a separate PR that is lower priority. Thatll give us an opportunity to evaluate its integration with the plugin model better and maybe from someone with more subject matter expertise
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3276769423
##########
rust/sedona-raster-zarr/src/geozarr.rs:
##########
@@ -0,0 +1,228 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! GeoZarr attribute parsing for CRS and affine transform.
+//!
+//! Reads the `proj:*` (CRS) and `spatial:*` (transform / spatial dim
+//! mapping) attribute conventions from a Zarr group's attributes, mapping
+//! them onto SedonaDB's per-raster `crs` and `transform` fields.
+//!
+//! Attributes live at the group level and are inherited by every array.
+//! Per-array overrides are rejected by the group-constraint validator
+//! (see `loader`).
+
+use arrow_schema::ArrowError;
+
+/// Per-group geo metadata distilled from `proj:*` / `spatial:*` attributes.
+#[derive(Debug, Clone, PartialEq)]
+pub struct GroupGeoMetadata {
+ /// CRS string in PROJ or WKT format (whichever the group declared).
+ /// `None` if no `proj:wkt2` / `proj:projjson` / `proj:epsg` attribute
+ /// is present on the group.
+ pub crs: Option<String>,
+ /// Affine transform in GDAL GeoTransform order:
+ /// `[origin_x, scale_x, skew_x, origin_y, skew_y, scale_y]`. `None`
+ /// when no `spatial:transform` attribute is present.
+ pub transform: Option<[f64; 6]>,
Review Comment:
yeah the ND refactor lays them out in the arrow as a single array. we can add methods to the trait that indexes into the array for this.
ETA: I see my response is not totally responsive here. Ill look into if there exists such an object.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3277168148
##########
rust/sedona/src/zarr_read.rs:
##########
@@ -0,0 +1,553 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! `sd_read_zarr` table function — exposes Zarr-backed raster groups
+//! as SQL-queryable tables.
+//!
+//! ```sql
+//! SELECT * FROM sd_read_zarr('file:///path/to/datacube.zarr');
+//! SELECT count(*) FROM sd_read_zarr(
+//! 'file:///path/to/datacube.zarr',
+//! '{"indb": true, "rows_per_batch": 256}'
+//! );
+//! ```
+//!
+//! Returns a single-column table `raster: Raster` with one row per chunk
+//! position in the Zarr group's chunk grid. All existing `RS_*` UDFs
+//! operate on the column unchanged.
+//!
+//! `indb` defaults to `true` so byte-reading kernels work end-to-end
+//! without depending on a registered OutDb resolver. The default will
+//! flip to `false` once the format-keyed dispatcher lands.
+
+use std::any::Any;
+use std::sync::Arc;
+
+use arrow_array::{Array, RecordBatch, StructArray};
+use arrow_schema::{DataType, Schema, SchemaRef};
+use async_trait::async_trait;
+use datafusion::catalog::{Session, TableFunctionImpl, TableProvider};
+use datafusion::common::Result;
+use datafusion::datasource::TableType;
+use datafusion::execution::context::TaskContext;
+use datafusion::physical_expr::EquivalenceProperties;
+use datafusion::physical_plan::execution_plan::{Boundedness, EmissionType};
+use datafusion::physical_plan::expressions::Column;
+use datafusion::physical_plan::projection::ProjectionExec;
+use datafusion::physical_plan::stream::RecordBatchStreamAdapter;
+use datafusion::physical_plan::{
+ DisplayAs, DisplayFormatType, ExecutionPlan, Partitioning, PhysicalExpr, PlanProperties,
+ SendableRecordBatchStream,
+};
+use datafusion::prelude::Expr;
+use datafusion_common::{plan_err, DataFusionError, ScalarValue};
+use sedona_raster_zarr::{group_to_indb_rasters, group_to_outdb_rasters};
+use sedona_schema::datatypes::SedonaType;
+use serde::{Deserialize, Serialize};
+
+/// Table function `sd_read_zarr(uri[, options_json])`.
+///
+/// Accepts one or two string arguments:
+/// - `uri` (required) — Zarr group URI (e.g. `file:///path/to/foo.zarr`).
+/// - `options_json` (optional) — JSON string with any of:
+/// - `indb`: `true` (default) materializes chunk bytes into the
+/// Arrow `data` column eagerly; `false` emits chunk-anchor URIs
+/// only and defers byte resolution to the OutDb loader.
+/// - `rows_per_batch`: chunks per `RecordBatch` (default 1024)
+/// - `num_partitions`: scan partitions (default 1; > 1 currently errors)
+/// - `arrays`: optional list of array names to read. Default reads
+/// every multi-dimensional array (1-D coord variables are
+/// auto-skipped); an explicit list reads exactly those arrays.
+#[derive(Debug, Default)]
+pub struct ZarrReadFunction {}
Review Comment:
> Should we default to reading outdb since this will almost certainly be the future behaviour?
Done
> It may be worth leaving off the default row count
Done
Looking at the rest.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3277162197
##########
rust/sedona-raster-zarr/Cargo.toml:
##########
@@ -0,0 +1,53 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+[package]
+name = "sedona-raster-zarr"
+version.workspace = true
+license.workspace = true
+keywords.workspace = true
+categories.workspace = true
+homepage.workspace = true
+repository.workspace = true
+description.workspace = true
+readme.workspace = true
+edition.workspace = true
+rust-version.workspace = true
+
+[lints.clippy]
+result_large_err = "allow"
+
+[dependencies]
+arrow-array = { workspace = true }
+arrow-schema = { workspace = true }
+datafusion-common = { workspace = true }
+log = { workspace = true }
+sedona-common = { workspace = true }
+sedona-raster = { workspace = true }
+sedona-schema = { workspace = true }
+serde_json = { workspace = true }
+zarrs = { workspace = true, features = ["filesystem", "gzip", "zstd", "crc32c", "sharding", "transpose"] }
+zarrs_filesystem = { workspace = true }
+
+# `blosc` is gated off Windows: c-blosc (statically linked) bundles its own
+# `pthread_create` / `pthread_cond_*` symbols, which conflict with rtools45's
+# `libpthread.a` during the MinGW link of the R `sedonadb.dll`. Non-Windows
+# targets get the full blosc-compressed Zarr reading capability.
+[target.'cfg(not(target_os = "windows"))'.dependencies]
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3276758780
##########
rust/sedona-raster-zarr/src/loader.rs:
##########
@@ -0,0 +1,674 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Zarr group → N-D raster `StructArray` entry points.
+//!
+//! Both `group_to_indb_rasters` and `group_to_outdb_rasters` produce the
+//! same row shape: one raster row per chunk position, with one band per
+//! array in the group. They differ only in how each row's pixel bytes
+//! are delivered:
+//!
+//! - **InDb** — every chunk is fetched eagerly and copied into the
+//! Arrow `data` column. Heavy for large datacubes; intended for
+//! snapshots.
+//! - **OutDb** — `data` is left empty; each band's `outdb_uri` carries a
+//! chunk anchor (`zarr://<store-uri>/<array-path>#chunk=i0,i1,...`).
+//! Byte resolution awaits the format-keyed dispatch work in a
+//! follow-up PR.
+
+use std::sync::Arc;
+
+use arrow_array::StructArray;
+use arrow_schema::ArrowError;
+use sedona_common::sedona_internal_datafusion_err;
+use sedona_raster::builder::RasterBuilder;
+use sedona_schema::raster::BandDataType;
+use zarrs::array::{Array, ArrayBytes};
+use zarrs::group::Group;
+use zarrs_filesystem::FilesystemStore;
+
+use crate::dtype::zarr_to_band_data_type;
+use crate::geozarr::GroupGeoMetadata;
+use crate::source_uri::{build_chunk_anchor, group_uri_to_filesystem_path};
+
+/// Open a Zarr group and eagerly fetch every chunk's bytes into the
+/// returned `StructArray`. Each row holds one chunk position's data
+/// across the selected arrays in the group.
+///
+/// `arrays`:
+/// - `None` — read every multi-dimensional array. 1-D arrays (typical
+/// coord variables in xarray-style datacubes) are auto-skipped.
+/// - `Some(names)` — read exactly the named arrays, in the order
+/// returned by the store's listing (which is then sorted by path). An
+/// unknown name errors. 1-D arrays are always rejected (a raster band
+/// needs ≥ 2 dimensions); naming one explicitly errors at parse time
+/// with a clear message.
+pub fn group_to_indb_rasters(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::InDb, arrays)
+}
+
+/// Open a Zarr group and emit one row per chunk position with chunk-anchor
+/// URIs in each band's `outdb_uri`. The `data` column is empty; bytes
+/// resolve on demand through whichever OutDb loader is registered for
+/// the `zarr` format. See [`group_to_indb_rasters`] for the meaning of
+/// `arrays`.
+pub fn group_to_outdb_rasters(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::OutDb, arrays)
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum Mode {
+ InDb,
+ OutDb,
+}
+
+/// Per-array metadata extracted once at group open and reused for every
+/// chunk position. Caching this avoids re-reading Zarr metadata for each
+/// of the (potentially thousands of) chunk rows.
+struct ArrayInfo {
+ /// Array path within the store, used to build chunk anchor URIs and
+ /// surface in band names.
+ path: String,
+ /// Open zarrs handle.
+ array: Array<FilesystemStore>,
+ /// SedonaDB BandDataType corresponding to this array's zarrs dtype.
+ data_type: BandDataType,
+ /// Dimension names in array order. Required to be `Some(_)` for every
+ /// dim; missing names error at validation time.
+ dim_names: Vec<String>,
+ /// Inner chunk grid shape, one entry per dimension. Used to enumerate
+ /// chunk positions and validated to match across arrays.
+ chunk_grid_shape: Vec<u64>,
+ /// Chunk shape (elements per chunk per dim). Same for every chunk
+ /// position — ragged final chunks are not emitted as separate short
+ /// rows.
+ chunk_shape: Vec<u64>,
+ /// Encoded fill value in native-endian byte representation, for the
+ /// `nodata` field. None when the array has no fill value declared.
+ nodata: Option<Vec<u8>>,
+}
+
+fn build_rasters(
+ group_uri: &str,
+ mode: Mode,
+ arrays_filter: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ let fs_path = group_uri_to_filesystem_path(group_uri)?;
+ let store = FilesystemStore::new(&fs_path).map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr filesystem store at {}: {e}",
+ fs_path.display()
+ )))
+ })?;
+ let storage: Arc<FilesystemStore> = Arc::new(store);
+
+ let group = Group::open(storage.clone(), "/").map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr group at {group_uri}: {e}"
+ )))
+ })?;
+
+ let geo = GroupGeoMetadata::from_attributes(group.attributes())?;
+
+ // CRS-without-transform is almost always a malformed-metadata bug —
+ // the user thinks they have full georef but downstream spatial joins
+ // will silently use the identity-pixel-coords default. Error loudly
+ // so they fix the metadata rather than getting empty result sets.
+ if geo.crs.is_some() && geo.transform.is_none() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} declares a CRS but no `spatial:transform` \
+ attribute; refusing to fall back to the identity transform because \
+ that would silently produce wrong results in spatial joins. Declare \
+ `spatial:transform` on the group or remove the CRS to read this as \
+ a non-georeferenced datacube."
+ )));
+ }
+
+ let arrays = group.child_arrays().map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to enumerate child arrays in {group_uri}: {e}"
+ )))
+ })?;
+ if arrays.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} has no child arrays"
+ )));
+ }
+
+ let arrays = select_arrays(arrays, arrays_filter, group_uri)?;
+ let array_infos = collect_array_infos(arrays)?;
+ validate_group_constraints(&array_infos)?;
+
+ // Spatial-dim resolution. Two configurations are accepted:
+ // - dim_names ends with ["y", "x"] (canonical for georeferenced
+ // 2-D and time-series rasters); the spatial extent is the chunk's
+ // last two dims.
+ // - `spatial:dims` attribute on the group explicitly names them.
+ // Anything else errors with a clear message — silently picking dims
+ // would produce wrong per-row transforms.
+ let spatial_dim_indices =
+ resolve_spatial_dim_indices(&array_infos[0].dim_names, geo.spatial_dims.as_deref())?;
+ let spatial_dims_names: Vec<&str> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].dim_names[i].as_str())
+ .collect();
+ let chunk_spatial_shape: Vec<i64> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].chunk_shape[i] as i64)
+ .collect();
+
+ let group_transform = match geo.transform {
+ Some(t) => t,
+ None => {
+ // Both `spatial:transform` and `proj:*` are absent (the
+ // CRS-only case errored above). Fall back to identity pixel
+ // coordinates and breadcrumb a warning so spatial-join
+ // surprises are debuggable.
+ log::warn!(
+ "Zarr group at {group_uri} has no `spatial:transform`; falling back \
+ to the identity pixel-coordinate transform [0, 1, 0, 0, 0, -1]. \
+ Spatial operations against this raster will use pixel coordinates."
+ );
+ [0.0, 1.0, 0.0, 0.0, 0.0, -1.0]
+ }
+ };
+
+ // chunk_grid_shape comes from untrusted Zarr metadata; bound-check the
+ // product so a hostile or malformed grid can't drive RasterBuilder
+ // capacity to overflow (and trigger an OOM preallocation).
+ let total_rows = array_infos[0]
+ .chunk_grid_shape
+ .iter()
+ .try_fold(1usize, |acc, &n| {
+ usize::try_from(n).ok().and_then(|n| acc.checked_mul(n))
+ })
+ .ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk grid shape {:?} overflows usize",
+ array_infos[0].chunk_grid_shape
+ ))
+ })?;
+ let mut builder = RasterBuilder::new(total_rows);
+
+ // Walk the chunk grid in row-major (C-order) order. The outer-most
+ // axis varies slowest, the innermost fastest — same convention used
+ // for byte strides in `BandRefImpl`.
+ let mut chunk_indices = vec![0u64; array_infos[0].chunk_grid_shape.len()];
+ loop {
+ let row_transform = compute_row_transform(
+ &group_transform,
+ &chunk_indices,
+ &array_infos[0].chunk_shape,
+ &spatial_dim_indices,
+ );
+ let crs_str = geo.crs.as_deref();
+ builder.start_raster_nd(
+ &row_transform,
+ &spatial_dims_names,
+ &chunk_spatial_shape,
+ crs_str,
+ )?;
+
+ for info in &array_infos {
+ let dim_names_ref: Vec<&str> = info.dim_names.iter().map(String::as_str).collect();
+ let nodata_ref = info.nodata.as_deref();
+ // Chunk anchor is provenance metadata; InDb vs OutDb is
+ // discriminated by `data.is_empty()`, not by these fields.
+ let anchor = build_chunk_anchor(group_uri, &info.path, &chunk_indices);
+ builder.start_band_nd(
+ Some(info.path.as_str()),
+ &dim_names_ref,
+ &info.chunk_shape,
+ info.data_type,
+ nodata_ref,
+ Some(anchor.as_str()),
+ Some("zarr"),
+ )?;
+ match mode {
+ Mode::InDb => {
+ let bytes = retrieve_chunk_bytes(&info.array, &chunk_indices)?;
+ builder.band_data_writer().append_value(&bytes);
+ }
Review Comment:
Yeah this is quite bad. This exists as a temporary way to make it possible to load a zarr while we wait for all the outdb machinery.
My intention is to replace this with the `RS_EnsureLoaded` call once all the outdb loading + zarr integration work lands so `load_eager` effectively means load outdb then inject a Project operator with a `RS_EnsureLoaded` call.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3277297055
##########
rust/sedona-raster-zarr/src/loader.rs:
##########
@@ -0,0 +1,674 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Zarr group → N-D raster `StructArray` entry points.
+//!
+//! Both `group_to_indb_rasters` and `group_to_outdb_rasters` produce the
+//! same row shape: one raster row per chunk position, with one band per
+//! array in the group. They differ only in how each row's pixel bytes
+//! are delivered:
+//!
+//! - **InDb** — every chunk is fetched eagerly and copied into the
+//! Arrow `data` column. Heavy for large datacubes; intended for
+//! snapshots.
+//! - **OutDb** — `data` is left empty; each band's `outdb_uri` carries a
+//! chunk anchor (`zarr://<store-uri>/<array-path>#chunk=i0,i1,...`).
+//! Byte resolution awaits the format-keyed dispatch work in a
+//! follow-up PR.
+
+use std::sync::Arc;
+
+use arrow_array::StructArray;
+use arrow_schema::ArrowError;
+use sedona_common::sedona_internal_datafusion_err;
+use sedona_raster::builder::RasterBuilder;
+use sedona_schema::raster::BandDataType;
+use zarrs::array::{Array, ArrayBytes};
+use zarrs::group::Group;
+use zarrs_filesystem::FilesystemStore;
+
+use crate::dtype::zarr_to_band_data_type;
+use crate::geozarr::GroupGeoMetadata;
+use crate::source_uri::{build_chunk_anchor, group_uri_to_filesystem_path};
+
+/// Open a Zarr group and eagerly fetch every chunk's bytes into the
+/// returned `StructArray`. Each row holds one chunk position's data
+/// across the selected arrays in the group.
+///
+/// `arrays`:
+/// - `None` — read every multi-dimensional array. 1-D arrays (typical
+/// coord variables in xarray-style datacubes) are auto-skipped.
+/// - `Some(names)` — read exactly the named arrays, in the order
+/// returned by the store's listing (which is then sorted by path). An
+/// unknown name errors. 1-D arrays are always rejected (a raster band
+/// needs ≥ 2 dimensions); naming one explicitly errors at parse time
+/// with a clear message.
+pub fn group_to_indb_rasters(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::InDb, arrays)
+}
+
+/// Open a Zarr group and emit one row per chunk position with chunk-anchor
+/// URIs in each band's `outdb_uri`. The `data` column is empty; bytes
+/// resolve on demand through whichever OutDb loader is registered for
+/// the `zarr` format. See [`group_to_indb_rasters`] for the meaning of
+/// `arrays`.
+pub fn group_to_outdb_rasters(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::OutDb, arrays)
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum Mode {
+ InDb,
+ OutDb,
+}
+
+/// Per-array metadata extracted once at group open and reused for every
+/// chunk position. Caching this avoids re-reading Zarr metadata for each
+/// of the (potentially thousands of) chunk rows.
+struct ArrayInfo {
+ /// Array path within the store, used to build chunk anchor URIs and
+ /// surface in band names.
+ path: String,
+ /// Open zarrs handle.
+ array: Array<FilesystemStore>,
+ /// SedonaDB BandDataType corresponding to this array's zarrs dtype.
+ data_type: BandDataType,
+ /// Dimension names in array order. Required to be `Some(_)` for every
+ /// dim; missing names error at validation time.
+ dim_names: Vec<String>,
+ /// Inner chunk grid shape, one entry per dimension. Used to enumerate
+ /// chunk positions and validated to match across arrays.
+ chunk_grid_shape: Vec<u64>,
+ /// Chunk shape (elements per chunk per dim). Same for every chunk
+ /// position — ragged final chunks are not emitted as separate short
+ /// rows.
+ chunk_shape: Vec<u64>,
+ /// Encoded fill value in native-endian byte representation, for the
+ /// `nodata` field. None when the array has no fill value declared.
+ nodata: Option<Vec<u8>>,
+}
+
+fn build_rasters(
+ group_uri: &str,
+ mode: Mode,
+ arrays_filter: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ let fs_path = group_uri_to_filesystem_path(group_uri)?;
+ let store = FilesystemStore::new(&fs_path).map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr filesystem store at {}: {e}",
+ fs_path.display()
+ )))
+ })?;
+ let storage: Arc<FilesystemStore> = Arc::new(store);
+
+ let group = Group::open(storage.clone(), "/").map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr group at {group_uri}: {e}"
+ )))
+ })?;
+
+ let geo = GroupGeoMetadata::from_attributes(group.attributes())?;
+
+ // CRS-without-transform is almost always a malformed-metadata bug —
+ // the user thinks they have full georef but downstream spatial joins
+ // will silently use the identity-pixel-coords default. Error loudly
+ // so they fix the metadata rather than getting empty result sets.
+ if geo.crs.is_some() && geo.transform.is_none() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} declares a CRS but no `spatial:transform` \
+ attribute; refusing to fall back to the identity transform because \
+ that would silently produce wrong results in spatial joins. Declare \
+ `spatial:transform` on the group or remove the CRS to read this as \
+ a non-georeferenced datacube."
+ )));
+ }
+
+ let arrays = group.child_arrays().map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to enumerate child arrays in {group_uri}: {e}"
+ )))
+ })?;
+ if arrays.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} has no child arrays"
+ )));
+ }
+
+ let arrays = select_arrays(arrays, arrays_filter, group_uri)?;
+ let array_infos = collect_array_infos(arrays)?;
+ validate_group_constraints(&array_infos)?;
+
+ // Spatial-dim resolution. Two configurations are accepted:
+ // - dim_names ends with ["y", "x"] (canonical for georeferenced
+ // 2-D and time-series rasters); the spatial extent is the chunk's
+ // last two dims.
+ // - `spatial:dims` attribute on the group explicitly names them.
+ // Anything else errors with a clear message — silently picking dims
+ // would produce wrong per-row transforms.
+ let spatial_dim_indices =
+ resolve_spatial_dim_indices(&array_infos[0].dim_names, geo.spatial_dims.as_deref())?;
+ let spatial_dims_names: Vec<&str> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].dim_names[i].as_str())
+ .collect();
+ let chunk_spatial_shape: Vec<i64> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].chunk_shape[i] as i64)
+ .collect();
+
+ let group_transform = match geo.transform {
+ Some(t) => t,
+ None => {
+ // Both `spatial:transform` and `proj:*` are absent (the
+ // CRS-only case errored above). Fall back to identity pixel
+ // coordinates and breadcrumb a warning so spatial-join
+ // surprises are debuggable.
+ log::warn!(
+ "Zarr group at {group_uri} has no `spatial:transform`; falling back \
+ to the identity pixel-coordinate transform [0, 1, 0, 0, 0, -1]. \
+ Spatial operations against this raster will use pixel coordinates."
+ );
+ [0.0, 1.0, 0.0, 0.0, 0.0, -1.0]
+ }
+ };
+
+ // chunk_grid_shape comes from untrusted Zarr metadata; bound-check the
+ // product so a hostile or malformed grid can't drive RasterBuilder
+ // capacity to overflow (and trigger an OOM preallocation).
+ let total_rows = array_infos[0]
+ .chunk_grid_shape
+ .iter()
+ .try_fold(1usize, |acc, &n| {
+ usize::try_from(n).ok().and_then(|n| acc.checked_mul(n))
+ })
+ .ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk grid shape {:?} overflows usize",
+ array_infos[0].chunk_grid_shape
+ ))
+ })?;
+ let mut builder = RasterBuilder::new(total_rows);
+
+ // Walk the chunk grid in row-major (C-order) order. The outer-most
+ // axis varies slowest, the innermost fastest — same convention used
+ // for byte strides in `BandRefImpl`.
+ let mut chunk_indices = vec![0u64; array_infos[0].chunk_grid_shape.len()];
+ loop {
+ let row_transform = compute_row_transform(
+ &group_transform,
+ &chunk_indices,
+ &array_infos[0].chunk_shape,
+ &spatial_dim_indices,
+ );
+ let crs_str = geo.crs.as_deref();
+ builder.start_raster_nd(
+ &row_transform,
+ &spatial_dims_names,
+ &chunk_spatial_shape,
+ crs_str,
+ )?;
Review Comment:
this is somewhat mitigated by getting rid of the indb path as described in other comments. the metadata for all chunks being sync work will be resolved by the migration to ExternalFormatSpec
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3283700048
##########
rust/sedona-raster-zarr/Cargo.toml:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+[package]
+name = "sedona-raster-zarr"
+version.workspace = true
+license.workspace = true
+keywords.workspace = true
+categories.workspace = true
+homepage.workspace = true
+repository.workspace = true
+description.workspace = true
+readme.workspace = true
+edition.workspace = true
+rust-version.workspace = true
+
+[lints.clippy]
+result_large_err = "allow"
+
+[dependencies]
+arrow-array = { workspace = true }
+arrow-schema = { workspace = true }
+async-trait = { workspace = true }
+datafusion = { workspace = true, features = ["sql"] }
+datafusion-catalog = { workspace = true }
+datafusion-common = { workspace = true }
+datafusion-expr = { workspace = true }
+futures = { workspace = true }
+log = { workspace = true }
+sedona-common = { workspace = true }
+sedona-datasource = { workspace = true }
+sedona-raster = { workspace = true }
+sedona-schema = { workspace = true }
+serde = { workspace = true }
+serde_json = { workspace = true }
+zarrs = { workspace = true, features = ["filesystem", "gzip", "zstd", "crc32c", "sharding", "transpose"] }
+zarrs_filesystem = { workspace = true }
+
+# `blosc` is gated off the Windows MinGW (gnu) toolchain only: c-blosc
+# (statically linked) bundles its own `pthread_create` / `pthread_cond_*`
+# symbols, which conflict with rtools45's `libpthread.a` during the MinGW
+# link of the R `sedonadb.dll`. The MSVC toolchain (Python wheels, etc.)
+# uses a different pthread implementation and is fine.
+[target.'cfg(not(all(target_os = "windows", target_env = "gnu")))'.dependencies]
+zarrs = { workspace = true, features = ["blosc"] }
Review Comment:
Should we backlog a task for r zarr support?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3284220968
##########
rust/sedona-raster-zarr/Cargo.toml:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+[package]
+name = "sedona-raster-zarr"
+version.workspace = true
+license.workspace = true
+keywords.workspace = true
+categories.workspace = true
+homepage.workspace = true
+repository.workspace = true
+description.workspace = true
+readme.workspace = true
+edition.workspace = true
+rust-version.workspace = true
+
+[lints.clippy]
+result_large_err = "allow"
+
+[dependencies]
+arrow-array = { workspace = true }
+arrow-schema = { workspace = true }
+async-trait = { workspace = true }
+datafusion = { workspace = true, features = ["sql"] }
+datafusion-catalog = { workspace = true }
+datafusion-common = { workspace = true }
+datafusion-expr = { workspace = true }
+futures = { workspace = true }
+log = { workspace = true }
+sedona-common = { workspace = true }
+sedona-datasource = { workspace = true }
+sedona-raster = { workspace = true }
+sedona-schema = { workspace = true }
+serde = { workspace = true }
+serde_json = { workspace = true }
+zarrs = { workspace = true, features = ["filesystem", "gzip", "zstd", "crc32c", "sharding", "transpose"] }
+zarrs_filesystem = { workspace = true }
+
+# `blosc` is gated off the Windows MinGW (gnu) toolchain only: c-blosc
+# (statically linked) bundles its own `pthread_create` / `pthread_cond_*`
+# symbols, which conflict with rtools45's `libpthread.a` during the MinGW
+# link of the R `sedonadb.dll`. The MSVC toolchain (Python wheels, etc.)
+# uses a different pthread implementation and is fine.
+[target.'cfg(not(all(target_os = "windows", target_env = "gnu")))'.dependencies]
+zarrs = { workspace = true, features = ["blosc"] }
Review Comment:
removed for now
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3276280081
##########
rust/sedona-raster-zarr/Cargo.toml:
##########
@@ -0,0 +1,53 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+[package]
+name = "sedona-raster-zarr"
+version.workspace = true
+license.workspace = true
+keywords.workspace = true
+categories.workspace = true
+homepage.workspace = true
+repository.workspace = true
+description.workspace = true
+readme.workspace = true
+edition.workspace = true
+rust-version.workspace = true
+
+[lints.clippy]
+result_large_err = "allow"
+
+[dependencies]
+arrow-array = { workspace = true }
+arrow-schema = { workspace = true }
+datafusion-common = { workspace = true }
+log = { workspace = true }
+sedona-common = { workspace = true }
+sedona-raster = { workspace = true }
+sedona-schema = { workspace = true }
+serde_json = { workspace = true }
+zarrs = { workspace = true, features = ["filesystem", "gzip", "zstd", "crc32c", "sharding", "transpose"] }
+zarrs_filesystem = { workspace = true }
+
+# `blosc` is gated off Windows: c-blosc (statically linked) bundles its own
+# `pthread_create` / `pthread_cond_*` symbols, which conflict with rtools45's
+# `libpthread.a` during the MinGW link of the R `sedonadb.dll`. Non-Windows
+# targets get the full blosc-compressed Zarr reading capability.
+[target.'cfg(not(target_os = "windows"))'.dependencies]
Review Comment:
Can you use a more targeted cfg so that Windows Python users can still do this? I think the triplet for mingw is windows-gnu or something
##########
rust/sedona/src/zarr_read.rs:
##########
@@ -0,0 +1,553 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! `sd_read_zarr` table function — exposes Zarr-backed raster groups
+//! as SQL-queryable tables.
+//!
+//! ```sql
+//! SELECT * FROM sd_read_zarr('file:///path/to/datacube.zarr');
+//! SELECT count(*) FROM sd_read_zarr(
+//! 'file:///path/to/datacube.zarr',
+//! '{"indb": true, "rows_per_batch": 256}'
+//! );
+//! ```
+//!
+//! Returns a single-column table `raster: Raster` with one row per chunk
+//! position in the Zarr group's chunk grid. All existing `RS_*` UDFs
+//! operate on the column unchanged.
+//!
+//! `indb` defaults to `true` so byte-reading kernels work end-to-end
+//! without depending on a registered OutDb resolver. The default will
+//! flip to `false` once the format-keyed dispatcher lands.
+
+use std::any::Any;
+use std::sync::Arc;
+
+use arrow_array::{Array, RecordBatch, StructArray};
+use arrow_schema::{DataType, Schema, SchemaRef};
+use async_trait::async_trait;
+use datafusion::catalog::{Session, TableFunctionImpl, TableProvider};
+use datafusion::common::Result;
+use datafusion::datasource::TableType;
+use datafusion::execution::context::TaskContext;
+use datafusion::physical_expr::EquivalenceProperties;
+use datafusion::physical_plan::execution_plan::{Boundedness, EmissionType};
+use datafusion::physical_plan::expressions::Column;
+use datafusion::physical_plan::projection::ProjectionExec;
+use datafusion::physical_plan::stream::RecordBatchStreamAdapter;
+use datafusion::physical_plan::{
+ DisplayAs, DisplayFormatType, ExecutionPlan, Partitioning, PhysicalExpr, PlanProperties,
+ SendableRecordBatchStream,
+};
+use datafusion::prelude::Expr;
+use datafusion_common::{plan_err, DataFusionError, ScalarValue};
+use sedona_raster_zarr::{group_to_indb_rasters, group_to_outdb_rasters};
+use sedona_schema::datatypes::SedonaType;
+use serde::{Deserialize, Serialize};
+
+/// Table function `sd_read_zarr(uri[, options_json])`.
+///
+/// Accepts one or two string arguments:
+/// - `uri` (required) — Zarr group URI (e.g. `file:///path/to/foo.zarr`).
+/// - `options_json` (optional) — JSON string with any of:
+/// - `indb`: `true` (default) materializes chunk bytes into the
+/// Arrow `data` column eagerly; `false` emits chunk-anchor URIs
+/// only and defers byte resolution to the OutDb loader.
+/// - `rows_per_batch`: chunks per `RecordBatch` (default 1024)
+/// - `num_partitions`: scan partitions (default 1; > 1 currently errors)
+/// - `arrays`: optional list of array names to read. Default reads
+/// every multi-dimensional array (1-D coord variables are
+/// auto-skipped); an explicit list reads exactly those arrays.
+#[derive(Debug, Default)]
+pub struct ZarrReadFunction {}
Review Comment:
Rather than a UDTF in the sedona crate (which make zarr everybody's problem), how about:
- The ability to resolve a `Box<dyn RecordBatchReader>` based on these JSON options is exposed as a function in sedona-raster-zarr
- Use the pre-existing `ExternalFormatSpec` to write the FileFormat/TableProvider for you:
https://github.com/apache/sedona-db/blob/c2613699056b67c001a7ef6899e8568f91bc6596/python/sedonadb/python/sedonadb/datasource.py#L24-L34
- Build a separate Python package `sedonadb-zarr` that exposes your `Box<dyn RecordBatchReader>` to Python and wire it together there. You can add to the ExternalFormatSpec if you need more features.
Either way the UDTF in the sedona-raster-zarr crate is a better fit (make sure it is feature flagged here under `zarr` at the very very least). We can still build it into another Python package using the DataFusion FFI but it's more work and in this case you don't really need any bells and whistles of the DataFusion UDTF because there are not a lot of total rows.
Should we default to reading outdb since this will almost certainly be the future behaviour?
It may be worth leaving off the default row count (you may well want to have this be adaptive based on the byte size you need to load)
##########
rust/sedona-raster-zarr/src/loader.rs:
##########
@@ -0,0 +1,674 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Zarr group → N-D raster `StructArray` entry points.
+//!
+//! Both `group_to_indb_rasters` and `group_to_outdb_rasters` produce the
+//! same row shape: one raster row per chunk position, with one band per
+//! array in the group. They differ only in how each row's pixel bytes
+//! are delivered:
+//!
+//! - **InDb** — every chunk is fetched eagerly and copied into the
+//! Arrow `data` column. Heavy for large datacubes; intended for
+//! snapshots.
+//! - **OutDb** — `data` is left empty; each band's `outdb_uri` carries a
+//! chunk anchor (`zarr://<store-uri>/<array-path>#chunk=i0,i1,...`).
+//! Byte resolution awaits the format-keyed dispatch work in a
+//! follow-up PR.
+
+use std::sync::Arc;
+
+use arrow_array::StructArray;
+use arrow_schema::ArrowError;
+use sedona_common::sedona_internal_datafusion_err;
+use sedona_raster::builder::RasterBuilder;
+use sedona_schema::raster::BandDataType;
+use zarrs::array::{Array, ArrayBytes};
+use zarrs::group::Group;
+use zarrs_filesystem::FilesystemStore;
+
+use crate::dtype::zarr_to_band_data_type;
+use crate::geozarr::GroupGeoMetadata;
+use crate::source_uri::{build_chunk_anchor, group_uri_to_filesystem_path};
+
+/// Open a Zarr group and eagerly fetch every chunk's bytes into the
+/// returned `StructArray`. Each row holds one chunk position's data
+/// across the selected arrays in the group.
+///
+/// `arrays`:
+/// - `None` — read every multi-dimensional array. 1-D arrays (typical
+/// coord variables in xarray-style datacubes) are auto-skipped.
+/// - `Some(names)` — read exactly the named arrays, in the order
+/// returned by the store's listing (which is then sorted by path). An
+/// unknown name errors. 1-D arrays are always rejected (a raster band
+/// needs ≥ 2 dimensions); naming one explicitly errors at parse time
+/// with a clear message.
+pub fn group_to_indb_rasters(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::InDb, arrays)
+}
+
+/// Open a Zarr group and emit one row per chunk position with chunk-anchor
+/// URIs in each band's `outdb_uri`. The `data` column is empty; bytes
+/// resolve on demand through whichever OutDb loader is registered for
+/// the `zarr` format. See [`group_to_indb_rasters`] for the meaning of
+/// `arrays`.
+pub fn group_to_outdb_rasters(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::OutDb, arrays)
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum Mode {
+ InDb,
+ OutDb,
+}
+
+/// Per-array metadata extracted once at group open and reused for every
+/// chunk position. Caching this avoids re-reading Zarr metadata for each
+/// of the (potentially thousands of) chunk rows.
+struct ArrayInfo {
+ /// Array path within the store, used to build chunk anchor URIs and
+ /// surface in band names.
+ path: String,
+ /// Open zarrs handle.
+ array: Array<FilesystemStore>,
+ /// SedonaDB BandDataType corresponding to this array's zarrs dtype.
+ data_type: BandDataType,
+ /// Dimension names in array order. Required to be `Some(_)` for every
+ /// dim; missing names error at validation time.
+ dim_names: Vec<String>,
+ /// Inner chunk grid shape, one entry per dimension. Used to enumerate
+ /// chunk positions and validated to match across arrays.
+ chunk_grid_shape: Vec<u64>,
+ /// Chunk shape (elements per chunk per dim). Same for every chunk
+ /// position — ragged final chunks are not emitted as separate short
+ /// rows.
+ chunk_shape: Vec<u64>,
+ /// Encoded fill value in native-endian byte representation, for the
+ /// `nodata` field. None when the array has no fill value declared.
+ nodata: Option<Vec<u8>>,
+}
+
+fn build_rasters(
+ group_uri: &str,
+ mode: Mode,
+ arrays_filter: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ let fs_path = group_uri_to_filesystem_path(group_uri)?;
+ let store = FilesystemStore::new(&fs_path).map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr filesystem store at {}: {e}",
+ fs_path.display()
+ )))
+ })?;
+ let storage: Arc<FilesystemStore> = Arc::new(store);
+
+ let group = Group::open(storage.clone(), "/").map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr group at {group_uri}: {e}"
+ )))
+ })?;
+
+ let geo = GroupGeoMetadata::from_attributes(group.attributes())?;
+
+ // CRS-without-transform is almost always a malformed-metadata bug —
+ // the user thinks they have full georef but downstream spatial joins
+ // will silently use the identity-pixel-coords default. Error loudly
+ // so they fix the metadata rather than getting empty result sets.
+ if geo.crs.is_some() && geo.transform.is_none() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} declares a CRS but no `spatial:transform` \
+ attribute; refusing to fall back to the identity transform because \
+ that would silently produce wrong results in spatial joins. Declare \
+ `spatial:transform` on the group or remove the CRS to read this as \
+ a non-georeferenced datacube."
+ )));
+ }
+
+ let arrays = group.child_arrays().map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to enumerate child arrays in {group_uri}: {e}"
+ )))
+ })?;
+ if arrays.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} has no child arrays"
+ )));
+ }
+
+ let arrays = select_arrays(arrays, arrays_filter, group_uri)?;
+ let array_infos = collect_array_infos(arrays)?;
+ validate_group_constraints(&array_infos)?;
+
+ // Spatial-dim resolution. Two configurations are accepted:
+ // - dim_names ends with ["y", "x"] (canonical for georeferenced
+ // 2-D and time-series rasters); the spatial extent is the chunk's
+ // last two dims.
+ // - `spatial:dims` attribute on the group explicitly names them.
+ // Anything else errors with a clear message — silently picking dims
+ // would produce wrong per-row transforms.
+ let spatial_dim_indices =
+ resolve_spatial_dim_indices(&array_infos[0].dim_names, geo.spatial_dims.as_deref())?;
+ let spatial_dims_names: Vec<&str> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].dim_names[i].as_str())
+ .collect();
+ let chunk_spatial_shape: Vec<i64> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].chunk_shape[i] as i64)
+ .collect();
+
+ let group_transform = match geo.transform {
+ Some(t) => t,
+ None => {
+ // Both `spatial:transform` and `proj:*` are absent (the
+ // CRS-only case errored above). Fall back to identity pixel
+ // coordinates and breadcrumb a warning so spatial-join
+ // surprises are debuggable.
+ log::warn!(
+ "Zarr group at {group_uri} has no `spatial:transform`; falling back \
+ to the identity pixel-coordinate transform [0, 1, 0, 0, 0, -1]. \
+ Spatial operations against this raster will use pixel coordinates."
+ );
+ [0.0, 1.0, 0.0, 0.0, 0.0, -1.0]
+ }
+ };
+
+ // chunk_grid_shape comes from untrusted Zarr metadata; bound-check the
+ // product so a hostile or malformed grid can't drive RasterBuilder
+ // capacity to overflow (and trigger an OOM preallocation).
+ let total_rows = array_infos[0]
+ .chunk_grid_shape
+ .iter()
+ .try_fold(1usize, |acc, &n| {
+ usize::try_from(n).ok().and_then(|n| acc.checked_mul(n))
+ })
+ .ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk grid shape {:?} overflows usize",
+ array_infos[0].chunk_grid_shape
+ ))
+ })?;
+ let mut builder = RasterBuilder::new(total_rows);
+
+ // Walk the chunk grid in row-major (C-order) order. The outer-most
+ // axis varies slowest, the innermost fastest — same convention used
+ // for byte strides in `BandRefImpl`.
+ let mut chunk_indices = vec![0u64; array_infos[0].chunk_grid_shape.len()];
+ loop {
+ let row_transform = compute_row_transform(
+ &group_transform,
+ &chunk_indices,
+ &array_infos[0].chunk_shape,
+ &spatial_dim_indices,
+ );
+ let crs_str = geo.crs.as_deref();
+ builder.start_raster_nd(
+ &row_transform,
+ &spatial_dims_names,
+ &chunk_spatial_shape,
+ crs_str,
+ )?;
Review Comment:
This step involves getting all chunks of a Zarr into memory before emitting them. Is this always a sufficiently small number of chunks that this is safe/the behaviour we want? (We could also have a zarr chunk iterator that can be pulled from)
##########
python/sedonadb/tests/test_funcs.py:
##########
@@ -24,3 +24,35 @@ def test_random_geometry(con):
# Ensure the output is reproducible
assert df.to_arrow_table() == df.to_arrow_table()
+
+
+def test_read_zarr(con, tmp_path):
+ # Skip cleanly if the optional `zarr` Python lib isn't installed —
+ # the binding is exercised by the Rust-side integration tests; this
+ # test only verifies the Python wrapper threads arguments through.
+ import pytest
+
+ zarr = pytest.importorskip("zarr")
+ np = pytest.importorskip("numpy")
Review Comment:
This can be a top-level import (zarr can stay importorskip)
##########
rust/sedona-raster-zarr/src/geozarr.rs:
##########
@@ -0,0 +1,228 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! GeoZarr attribute parsing for CRS and affine transform.
+//!
+//! Reads the `proj:*` (CRS) and `spatial:*` (transform / spatial dim
+//! mapping) attribute conventions from a Zarr group's attributes, mapping
+//! them onto SedonaDB's per-raster `crs` and `transform` fields.
+//!
+//! Attributes live at the group level and are inherited by every array.
+//! Per-array overrides are rejected by the group-constraint validator
+//! (see `loader`).
+
+use arrow_schema::ArrowError;
+
+/// Per-group geo metadata distilled from `proj:*` / `spatial:*` attributes.
+#[derive(Debug, Clone, PartialEq)]
+pub struct GroupGeoMetadata {
+ /// CRS string in PROJ or WKT format (whichever the group declared).
+ /// `None` if no `proj:wkt2` / `proj:projjson` / `proj:epsg` attribute
+ /// is present on the group.
+ pub crs: Option<String>,
+ /// Affine transform in GDAL GeoTransform order:
+ /// `[origin_x, scale_x, skew_x, origin_y, skew_y, scale_y]`. `None`
+ /// when no `spatial:transform` attribute is present.
+ pub transform: Option<[f64; 6]>,
Review Comment:
I think at one point we had an object for this to help encapsulate what you can do with this? If not, we should add one and use it.
##########
rust/sedona-raster-zarr/src/loader.rs:
##########
@@ -0,0 +1,674 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Zarr group → N-D raster `StructArray` entry points.
+//!
+//! Both `group_to_indb_rasters` and `group_to_outdb_rasters` produce the
+//! same row shape: one raster row per chunk position, with one band per
+//! array in the group. They differ only in how each row's pixel bytes
+//! are delivered:
+//!
+//! - **InDb** — every chunk is fetched eagerly and copied into the
+//! Arrow `data` column. Heavy for large datacubes; intended for
+//! snapshots.
+//! - **OutDb** — `data` is left empty; each band's `outdb_uri` carries a
+//! chunk anchor (`zarr://<store-uri>/<array-path>#chunk=i0,i1,...`).
+//! Byte resolution awaits the format-keyed dispatch work in a
+//! follow-up PR.
+
+use std::sync::Arc;
+
+use arrow_array::StructArray;
+use arrow_schema::ArrowError;
+use sedona_common::sedona_internal_datafusion_err;
+use sedona_raster::builder::RasterBuilder;
+use sedona_schema::raster::BandDataType;
+use zarrs::array::{Array, ArrayBytes};
+use zarrs::group::Group;
+use zarrs_filesystem::FilesystemStore;
+
+use crate::dtype::zarr_to_band_data_type;
+use crate::geozarr::GroupGeoMetadata;
+use crate::source_uri::{build_chunk_anchor, group_uri_to_filesystem_path};
+
+/// Open a Zarr group and eagerly fetch every chunk's bytes into the
+/// returned `StructArray`. Each row holds one chunk position's data
+/// across the selected arrays in the group.
+///
+/// `arrays`:
+/// - `None` — read every multi-dimensional array. 1-D arrays (typical
+/// coord variables in xarray-style datacubes) are auto-skipped.
+/// - `Some(names)` — read exactly the named arrays, in the order
+/// returned by the store's listing (which is then sorted by path). An
+/// unknown name errors. 1-D arrays are always rejected (a raster band
+/// needs ≥ 2 dimensions); naming one explicitly errors at parse time
+/// with a clear message.
+pub fn group_to_indb_rasters(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::InDb, arrays)
+}
+
+/// Open a Zarr group and emit one row per chunk position with chunk-anchor
+/// URIs in each band's `outdb_uri`. The `data` column is empty; bytes
+/// resolve on demand through whichever OutDb loader is registered for
+/// the `zarr` format. See [`group_to_indb_rasters`] for the meaning of
+/// `arrays`.
+pub fn group_to_outdb_rasters(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::OutDb, arrays)
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum Mode {
+ InDb,
+ OutDb,
+}
+
+/// Per-array metadata extracted once at group open and reused for every
+/// chunk position. Caching this avoids re-reading Zarr metadata for each
+/// of the (potentially thousands of) chunk rows.
+struct ArrayInfo {
+ /// Array path within the store, used to build chunk anchor URIs and
+ /// surface in band names.
+ path: String,
+ /// Open zarrs handle.
+ array: Array<FilesystemStore>,
+ /// SedonaDB BandDataType corresponding to this array's zarrs dtype.
+ data_type: BandDataType,
+ /// Dimension names in array order. Required to be `Some(_)` for every
+ /// dim; missing names error at validation time.
+ dim_names: Vec<String>,
+ /// Inner chunk grid shape, one entry per dimension. Used to enumerate
+ /// chunk positions and validated to match across arrays.
+ chunk_grid_shape: Vec<u64>,
+ /// Chunk shape (elements per chunk per dim). Same for every chunk
+ /// position — ragged final chunks are not emitted as separate short
+ /// rows.
+ chunk_shape: Vec<u64>,
+ /// Encoded fill value in native-endian byte representation, for the
+ /// `nodata` field. None when the array has no fill value declared.
+ nodata: Option<Vec<u8>>,
+}
+
+fn build_rasters(
+ group_uri: &str,
+ mode: Mode,
+ arrays_filter: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ let fs_path = group_uri_to_filesystem_path(group_uri)?;
+ let store = FilesystemStore::new(&fs_path).map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr filesystem store at {}: {e}",
+ fs_path.display()
+ )))
+ })?;
+ let storage: Arc<FilesystemStore> = Arc::new(store);
+
+ let group = Group::open(storage.clone(), "/").map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr group at {group_uri}: {e}"
+ )))
+ })?;
+
+ let geo = GroupGeoMetadata::from_attributes(group.attributes())?;
+
+ // CRS-without-transform is almost always a malformed-metadata bug —
+ // the user thinks they have full georef but downstream spatial joins
+ // will silently use the identity-pixel-coords default. Error loudly
+ // so they fix the metadata rather than getting empty result sets.
+ if geo.crs.is_some() && geo.transform.is_none() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} declares a CRS but no `spatial:transform` \
+ attribute; refusing to fall back to the identity transform because \
+ that would silently produce wrong results in spatial joins. Declare \
+ `spatial:transform` on the group or remove the CRS to read this as \
+ a non-georeferenced datacube."
+ )));
+ }
+
+ let arrays = group.child_arrays().map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to enumerate child arrays in {group_uri}: {e}"
+ )))
+ })?;
+ if arrays.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} has no child arrays"
+ )));
+ }
+
+ let arrays = select_arrays(arrays, arrays_filter, group_uri)?;
+ let array_infos = collect_array_infos(arrays)?;
+ validate_group_constraints(&array_infos)?;
+
+ // Spatial-dim resolution. Two configurations are accepted:
+ // - dim_names ends with ["y", "x"] (canonical for georeferenced
+ // 2-D and time-series rasters); the spatial extent is the chunk's
+ // last two dims.
+ // - `spatial:dims` attribute on the group explicitly names them.
+ // Anything else errors with a clear message — silently picking dims
+ // would produce wrong per-row transforms.
+ let spatial_dim_indices =
+ resolve_spatial_dim_indices(&array_infos[0].dim_names, geo.spatial_dims.as_deref())?;
+ let spatial_dims_names: Vec<&str> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].dim_names[i].as_str())
+ .collect();
+ let chunk_spatial_shape: Vec<i64> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].chunk_shape[i] as i64)
+ .collect();
+
+ let group_transform = match geo.transform {
+ Some(t) => t,
+ None => {
+ // Both `spatial:transform` and `proj:*` are absent (the
+ // CRS-only case errored above). Fall back to identity pixel
+ // coordinates and breadcrumb a warning so spatial-join
+ // surprises are debuggable.
+ log::warn!(
+ "Zarr group at {group_uri} has no `spatial:transform`; falling back \
+ to the identity pixel-coordinate transform [0, 1, 0, 0, 0, -1]. \
+ Spatial operations against this raster will use pixel coordinates."
+ );
+ [0.0, 1.0, 0.0, 0.0, 0.0, -1.0]
+ }
+ };
+
+ // chunk_grid_shape comes from untrusted Zarr metadata; bound-check the
+ // product so a hostile or malformed grid can't drive RasterBuilder
+ // capacity to overflow (and trigger an OOM preallocation).
+ let total_rows = array_infos[0]
+ .chunk_grid_shape
+ .iter()
+ .try_fold(1usize, |acc, &n| {
+ usize::try_from(n).ok().and_then(|n| acc.checked_mul(n))
+ })
+ .ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk grid shape {:?} overflows usize",
+ array_infos[0].chunk_grid_shape
+ ))
+ })?;
+ let mut builder = RasterBuilder::new(total_rows);
+
+ // Walk the chunk grid in row-major (C-order) order. The outer-most
+ // axis varies slowest, the innermost fastest — same convention used
+ // for byte strides in `BandRefImpl`.
+ let mut chunk_indices = vec![0u64; array_infos[0].chunk_grid_shape.len()];
+ loop {
+ let row_transform = compute_row_transform(
+ &group_transform,
+ &chunk_indices,
+ &array_infos[0].chunk_shape,
+ &spatial_dim_indices,
+ );
+ let crs_str = geo.crs.as_deref();
+ builder.start_raster_nd(
+ &row_transform,
+ &spatial_dims_names,
+ &chunk_spatial_shape,
+ crs_str,
+ )?;
+
+ for info in &array_infos {
+ let dim_names_ref: Vec<&str> = info.dim_names.iter().map(String::as_str).collect();
+ let nodata_ref = info.nodata.as_deref();
+ // Chunk anchor is provenance metadata; InDb vs OutDb is
+ // discriminated by `data.is_empty()`, not by these fields.
+ let anchor = build_chunk_anchor(group_uri, &info.path, &chunk_indices);
+ builder.start_band_nd(
+ Some(info.path.as_str()),
+ &dim_names_ref,
+ &info.chunk_shape,
+ info.data_type,
+ nodata_ref,
+ Some(anchor.as_str()),
+ Some("zarr"),
+ )?;
+ match mode {
+ Mode::InDb => {
+ let bytes = retrieve_chunk_bytes(&info.array, &chunk_indices)?;
+ builder.band_data_writer().append_value(&bytes);
+ }
Review Comment:
This part you definitely don't want in a forward synchronous loop that does serial blocking IO. If you're building this as a TableProvider or UDTF, you can have this be `async` so the runtime is provided for you and you can `await` or implement the lower level RecordBatchStream to handle the polling yourself. In any case you probably want
```rust
struct ZarrChunkIterator {}
impl RecordBatchStream for ZarrChunkIterator {}
// or
impl RecordBatchReader for ZarrChunkIterator {}
```
As written it's a bit of a footgun because this type of loop can't be cancelled after a user realizes they've tried to load a few GB by accident
##########
python/sedonadb/tests/test_funcs.py:
##########
@@ -24,3 +24,35 @@ def test_random_geometry(con):
# Ensure the output is reproducible
assert df.to_arrow_table() == df.to_arrow_table()
+
+
+def test_read_zarr(con, tmp_path):
+ # Skip cleanly if the optional `zarr` Python lib isn't installed —
+ # the binding is exercised by the Rust-side integration tests; this
+ # test only verifies the Python wrapper threads arguments through.
+ import pytest
+
+ zarr = pytest.importorskip("zarr")
+ np = pytest.importorskip("numpy")
+
+ # Build a 2x2 UInt8 array with two chunks, dim_names=["y","x"], inside
+ # a group at the temp path. Matches the minimal shape sd_read_zarr
+ # expects (2-D array with [y, x] suffix).
+ root = zarr.open_group(str(tmp_path), mode="w")
+ arr = root.create_array(
+ "temperature",
+ shape=(2, 2),
+ chunks=(1, 2),
+ dtype="uint8",
+ dimension_names=["y", "x"],
+ )
+ arr[:] = np.array([[10, 11], [20, 21]], dtype=np.uint8)
+
+ # Default-mode (InDb) read — every chunk materialized into the
+ # Arrow `data` column.
+ df = con.funcs.table.sd_read_zarr(f"file://{tmp_path}")
+ assert df.count() == 2
+
+ # Options thread through: rows_per_batch slices the output.
+ df = con.funcs.table.sd_read_zarr(f"file://{tmp_path}", rows_per_batch=1)
+ assert df.count() == 2
Review Comment:
Make sure you also materialize these via `to_arrow_table()` or `to_pandas()`. We don't have a great way to inspect these but `arrow_tab["raster"][0].as_py()` should get you a Python `dict` you can do some basic asserts on.
##########
python/sedonadb/tests/test_funcs.py:
##########
@@ -24,3 +24,35 @@ def test_random_geometry(con):
# Ensure the output is reproducible
assert df.to_arrow_table() == df.to_arrow_table()
+
+
+def test_read_zarr(con, tmp_path):
+ # Skip cleanly if the optional `zarr` Python lib isn't installed —
+ # the binding is exercised by the Rust-side integration tests; this
+ # test only verifies the Python wrapper threads arguments through.
+ import pytest
Review Comment:
This can be a top-level import
##########
rust/sedona/Cargo.toml:
##########
@@ -50,6 +50,14 @@ s2geography = ["dep:sedona-s2geography", "dep:sedona-spatial-join-geography", "s
tempfile = { workspace = true }
tokio = { workspace = true, features = ["rt-multi-thread"] }
rstest = { workspace = true }
+sedona-raster = { workspace = true }
+zarrs = { workspace = true, features = ["filesystem", "gzip", "zstd", "crc32c", "sharding", "transpose"] }
+zarrs_filesystem = { workspace = true }
+
+# See sedona-raster-zarr/Cargo.toml — `blosc` is gated off Windows to avoid
+# a libwinpthreads/c-blosc symbol collision in the R `sedonadb.dll` link.
+[target.'cfg(not(target_os = "windows"))'.dev-dependencies]
+zarrs = { workspace = true, features = ["blosc"] }
Review Comment:
I think if you move these to `python/sedona-zarr` we can avoid making this crate take on every single piece of functionality we'll need here
##########
rust/sedona-raster-zarr/src/loader.rs:
##########
@@ -0,0 +1,674 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Zarr group → N-D raster `StructArray` entry points.
+//!
+//! Both `group_to_indb_rasters` and `group_to_outdb_rasters` produce the
+//! same row shape: one raster row per chunk position, with one band per
+//! array in the group. They differ only in how each row's pixel bytes
+//! are delivered:
+//!
+//! - **InDb** — every chunk is fetched eagerly and copied into the
+//! Arrow `data` column. Heavy for large datacubes; intended for
+//! snapshots.
+//! - **OutDb** — `data` is left empty; each band's `outdb_uri` carries a
+//! chunk anchor (`zarr://<store-uri>/<array-path>#chunk=i0,i1,...`).
+//! Byte resolution awaits the format-keyed dispatch work in a
+//! follow-up PR.
+
+use std::sync::Arc;
+
+use arrow_array::StructArray;
+use arrow_schema::ArrowError;
+use sedona_common::sedona_internal_datafusion_err;
+use sedona_raster::builder::RasterBuilder;
+use sedona_schema::raster::BandDataType;
+use zarrs::array::{Array, ArrayBytes};
+use zarrs::group::Group;
+use zarrs_filesystem::FilesystemStore;
+
+use crate::dtype::zarr_to_band_data_type;
+use crate::geozarr::GroupGeoMetadata;
+use crate::source_uri::{build_chunk_anchor, group_uri_to_filesystem_path};
+
+/// Open a Zarr group and eagerly fetch every chunk's bytes into the
+/// returned `StructArray`. Each row holds one chunk position's data
+/// across the selected arrays in the group.
+///
+/// `arrays`:
+/// - `None` — read every multi-dimensional array. 1-D arrays (typical
+/// coord variables in xarray-style datacubes) are auto-skipped.
+/// - `Some(names)` — read exactly the named arrays, in the order
+/// returned by the store's listing (which is then sorted by path). An
+/// unknown name errors. 1-D arrays are always rejected (a raster band
+/// needs ≥ 2 dimensions); naming one explicitly errors at parse time
+/// with a clear message.
+pub fn group_to_indb_rasters(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::InDb, arrays)
+}
+
+/// Open a Zarr group and emit one row per chunk position with chunk-anchor
+/// URIs in each band's `outdb_uri`. The `data` column is empty; bytes
+/// resolve on demand through whichever OutDb loader is registered for
+/// the `zarr` format. See [`group_to_indb_rasters`] for the meaning of
+/// `arrays`.
+pub fn group_to_outdb_rasters(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::OutDb, arrays)
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum Mode {
+ InDb,
+ OutDb,
+}
+
+/// Per-array metadata extracted once at group open and reused for every
+/// chunk position. Caching this avoids re-reading Zarr metadata for each
+/// of the (potentially thousands of) chunk rows.
+struct ArrayInfo {
+ /// Array path within the store, used to build chunk anchor URIs and
+ /// surface in band names.
+ path: String,
+ /// Open zarrs handle.
+ array: Array<FilesystemStore>,
+ /// SedonaDB BandDataType corresponding to this array's zarrs dtype.
+ data_type: BandDataType,
+ /// Dimension names in array order. Required to be `Some(_)` for every
+ /// dim; missing names error at validation time.
+ dim_names: Vec<String>,
+ /// Inner chunk grid shape, one entry per dimension. Used to enumerate
+ /// chunk positions and validated to match across arrays.
+ chunk_grid_shape: Vec<u64>,
+ /// Chunk shape (elements per chunk per dim). Same for every chunk
+ /// position — ragged final chunks are not emitted as separate short
+ /// rows.
+ chunk_shape: Vec<u64>,
+ /// Encoded fill value in native-endian byte representation, for the
+ /// `nodata` field. None when the array has no fill value declared.
+ nodata: Option<Vec<u8>>,
+}
+
+fn build_rasters(
+ group_uri: &str,
+ mode: Mode,
+ arrays_filter: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ let fs_path = group_uri_to_filesystem_path(group_uri)?;
+ let store = FilesystemStore::new(&fs_path).map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr filesystem store at {}: {e}",
+ fs_path.display()
+ )))
+ })?;
+ let storage: Arc<FilesystemStore> = Arc::new(store);
+
+ let group = Group::open(storage.clone(), "/").map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr group at {group_uri}: {e}"
+ )))
+ })?;
+
+ let geo = GroupGeoMetadata::from_attributes(group.attributes())?;
+
+ // CRS-without-transform is almost always a malformed-metadata bug —
+ // the user thinks they have full georef but downstream spatial joins
+ // will silently use the identity-pixel-coords default. Error loudly
+ // so they fix the metadata rather than getting empty result sets.
+ if geo.crs.is_some() && geo.transform.is_none() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} declares a CRS but no `spatial:transform` \
+ attribute; refusing to fall back to the identity transform because \
+ that would silently produce wrong results in spatial joins. Declare \
+ `spatial:transform` on the group or remove the CRS to read this as \
+ a non-georeferenced datacube."
+ )));
+ }
+
+ let arrays = group.child_arrays().map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to enumerate child arrays in {group_uri}: {e}"
+ )))
+ })?;
+ if arrays.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} has no child arrays"
+ )));
+ }
+
+ let arrays = select_arrays(arrays, arrays_filter, group_uri)?;
+ let array_infos = collect_array_infos(arrays)?;
+ validate_group_constraints(&array_infos)?;
+
+ // Spatial-dim resolution. Two configurations are accepted:
+ // - dim_names ends with ["y", "x"] (canonical for georeferenced
+ // 2-D and time-series rasters); the spatial extent is the chunk's
+ // last two dims.
+ // - `spatial:dims` attribute on the group explicitly names them.
+ // Anything else errors with a clear message — silently picking dims
+ // would produce wrong per-row transforms.
+ let spatial_dim_indices =
+ resolve_spatial_dim_indices(&array_infos[0].dim_names, geo.spatial_dims.as_deref())?;
+ let spatial_dims_names: Vec<&str> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].dim_names[i].as_str())
+ .collect();
+ let chunk_spatial_shape: Vec<i64> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].chunk_shape[i] as i64)
+ .collect();
+
+ let group_transform = match geo.transform {
+ Some(t) => t,
+ None => {
+ // Both `spatial:transform` and `proj:*` are absent (the
+ // CRS-only case errored above). Fall back to identity pixel
+ // coordinates and breadcrumb a warning so spatial-join
+ // surprises are debuggable.
+ log::warn!(
+ "Zarr group at {group_uri} has no `spatial:transform`; falling back \
+ to the identity pixel-coordinate transform [0, 1, 0, 0, 0, -1]. \
+ Spatial operations against this raster will use pixel coordinates."
+ );
+ [0.0, 1.0, 0.0, 0.0, 0.0, -1.0]
+ }
+ };
+
+ // chunk_grid_shape comes from untrusted Zarr metadata; bound-check the
+ // product so a hostile or malformed grid can't drive RasterBuilder
+ // capacity to overflow (and trigger an OOM preallocation).
+ let total_rows = array_infos[0]
+ .chunk_grid_shape
+ .iter()
+ .try_fold(1usize, |acc, &n| {
+ usize::try_from(n).ok().and_then(|n| acc.checked_mul(n))
+ })
+ .ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk grid shape {:?} overflows usize",
+ array_infos[0].chunk_grid_shape
+ ))
+ })?;
+ let mut builder = RasterBuilder::new(total_rows);
+
+ // Walk the chunk grid in row-major (C-order) order. The outer-most
+ // axis varies slowest, the innermost fastest — same convention used
+ // for byte strides in `BandRefImpl`.
+ let mut chunk_indices = vec![0u64; array_infos[0].chunk_grid_shape.len()];
+ loop {
+ let row_transform = compute_row_transform(
+ &group_transform,
+ &chunk_indices,
+ &array_infos[0].chunk_shape,
+ &spatial_dim_indices,
+ );
+ let crs_str = geo.crs.as_deref();
+ builder.start_raster_nd(
+ &row_transform,
+ &spatial_dims_names,
+ &chunk_spatial_shape,
+ crs_str,
+ )?;
+
+ for info in &array_infos {
+ let dim_names_ref: Vec<&str> = info.dim_names.iter().map(String::as_str).collect();
+ let nodata_ref = info.nodata.as_deref();
+ // Chunk anchor is provenance metadata; InDb vs OutDb is
+ // discriminated by `data.is_empty()`, not by these fields.
+ let anchor = build_chunk_anchor(group_uri, &info.path, &chunk_indices);
+ builder.start_band_nd(
+ Some(info.path.as_str()),
+ &dim_names_ref,
+ &info.chunk_shape,
+ info.data_type,
+ nodata_ref,
+ Some(anchor.as_str()),
+ Some("zarr"),
+ )?;
+ match mode {
+ Mode::InDb => {
+ let bytes = retrieve_chunk_bytes(&info.array, &chunk_indices)?;
+ builder.band_data_writer().append_value(&bytes);
+ }
+ Mode::OutDb => {
+ // Schema-OutDb: empty `data` column. Byte resolution
+ // routes through the OutDb loader when a downstream
+ // consumer calls contiguous_data / nd_buffer.
+ builder.band_data_writer().append_value([0u8; 0]);
+ }
+ }
+ builder.finish_band()?;
+ }
+ builder.finish_raster()?;
+
+ if !advance_chunk_indices(&mut chunk_indices, &array_infos[0].chunk_grid_shape) {
+ break;
+ }
+ }
+
+ builder.finish()
+}
+
+/// Collect per-array metadata from open zarrs `Array` handles.
+///
+/// Sorts arrays by path so band ordering across rows is deterministic
+/// (zarrs's underlying store listing order is implementation-defined —
Review Comment:
This seems like something you might want to leave to DataFusion (`ORDER_BY RS_BandPath(rast)`). You can have this as a testing utility too if that's the use case.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3277143477
##########
rust/sedona-raster-zarr/src/loader.rs:
##########
@@ -0,0 +1,674 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Zarr group → N-D raster `StructArray` entry points.
+//!
+//! Both `group_to_indb_rasters` and `group_to_outdb_rasters` produce the
+//! same row shape: one raster row per chunk position, with one band per
+//! array in the group. They differ only in how each row's pixel bytes
+//! are delivered:
+//!
+//! - **InDb** — every chunk is fetched eagerly and copied into the
+//! Arrow `data` column. Heavy for large datacubes; intended for
+//! snapshots.
+//! - **OutDb** — `data` is left empty; each band's `outdb_uri` carries a
+//! chunk anchor (`zarr://<store-uri>/<array-path>#chunk=i0,i1,...`).
+//! Byte resolution awaits the format-keyed dispatch work in a
+//! follow-up PR.
+
+use std::sync::Arc;
+
+use arrow_array::StructArray;
+use arrow_schema::ArrowError;
+use sedona_common::sedona_internal_datafusion_err;
+use sedona_raster::builder::RasterBuilder;
+use sedona_schema::raster::BandDataType;
+use zarrs::array::{Array, ArrayBytes};
+use zarrs::group::Group;
+use zarrs_filesystem::FilesystemStore;
+
+use crate::dtype::zarr_to_band_data_type;
+use crate::geozarr::GroupGeoMetadata;
+use crate::source_uri::{build_chunk_anchor, group_uri_to_filesystem_path};
+
+/// Open a Zarr group and eagerly fetch every chunk's bytes into the
+/// returned `StructArray`. Each row holds one chunk position's data
+/// across the selected arrays in the group.
+///
+/// `arrays`:
+/// - `None` — read every multi-dimensional array. 1-D arrays (typical
+/// coord variables in xarray-style datacubes) are auto-skipped.
+/// - `Some(names)` — read exactly the named arrays, in the order
+/// returned by the store's listing (which is then sorted by path). An
+/// unknown name errors. 1-D arrays are always rejected (a raster band
+/// needs ≥ 2 dimensions); naming one explicitly errors at parse time
+/// with a clear message.
+pub fn group_to_indb_rasters(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::InDb, arrays)
+}
+
+/// Open a Zarr group and emit one row per chunk position with chunk-anchor
+/// URIs in each band's `outdb_uri`. The `data` column is empty; bytes
+/// resolve on demand through whichever OutDb loader is registered for
+/// the `zarr` format. See [`group_to_indb_rasters`] for the meaning of
+/// `arrays`.
+pub fn group_to_outdb_rasters(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::OutDb, arrays)
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum Mode {
+ InDb,
+ OutDb,
+}
+
+/// Per-array metadata extracted once at group open and reused for every
+/// chunk position. Caching this avoids re-reading Zarr metadata for each
+/// of the (potentially thousands of) chunk rows.
+struct ArrayInfo {
+ /// Array path within the store, used to build chunk anchor URIs and
+ /// surface in band names.
+ path: String,
+ /// Open zarrs handle.
+ array: Array<FilesystemStore>,
+ /// SedonaDB BandDataType corresponding to this array's zarrs dtype.
+ data_type: BandDataType,
+ /// Dimension names in array order. Required to be `Some(_)` for every
+ /// dim; missing names error at validation time.
+ dim_names: Vec<String>,
+ /// Inner chunk grid shape, one entry per dimension. Used to enumerate
+ /// chunk positions and validated to match across arrays.
+ chunk_grid_shape: Vec<u64>,
+ /// Chunk shape (elements per chunk per dim). Same for every chunk
+ /// position — ragged final chunks are not emitted as separate short
+ /// rows.
+ chunk_shape: Vec<u64>,
+ /// Encoded fill value in native-endian byte representation, for the
+ /// `nodata` field. None when the array has no fill value declared.
+ nodata: Option<Vec<u8>>,
+}
+
+fn build_rasters(
+ group_uri: &str,
+ mode: Mode,
+ arrays_filter: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ let fs_path = group_uri_to_filesystem_path(group_uri)?;
+ let store = FilesystemStore::new(&fs_path).map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr filesystem store at {}: {e}",
+ fs_path.display()
+ )))
+ })?;
+ let storage: Arc<FilesystemStore> = Arc::new(store);
+
+ let group = Group::open(storage.clone(), "/").map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr group at {group_uri}: {e}"
+ )))
+ })?;
+
+ let geo = GroupGeoMetadata::from_attributes(group.attributes())?;
+
+ // CRS-without-transform is almost always a malformed-metadata bug —
+ // the user thinks they have full georef but downstream spatial joins
+ // will silently use the identity-pixel-coords default. Error loudly
+ // so they fix the metadata rather than getting empty result sets.
+ if geo.crs.is_some() && geo.transform.is_none() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} declares a CRS but no `spatial:transform` \
+ attribute; refusing to fall back to the identity transform because \
+ that would silently produce wrong results in spatial joins. Declare \
+ `spatial:transform` on the group or remove the CRS to read this as \
+ a non-georeferenced datacube."
+ )));
+ }
+
+ let arrays = group.child_arrays().map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to enumerate child arrays in {group_uri}: {e}"
+ )))
+ })?;
+ if arrays.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} has no child arrays"
+ )));
+ }
+
+ let arrays = select_arrays(arrays, arrays_filter, group_uri)?;
+ let array_infos = collect_array_infos(arrays)?;
+ validate_group_constraints(&array_infos)?;
+
+ // Spatial-dim resolution. Two configurations are accepted:
+ // - dim_names ends with ["y", "x"] (canonical for georeferenced
+ // 2-D and time-series rasters); the spatial extent is the chunk's
+ // last two dims.
+ // - `spatial:dims` attribute on the group explicitly names them.
+ // Anything else errors with a clear message — silently picking dims
+ // would produce wrong per-row transforms.
+ let spatial_dim_indices =
+ resolve_spatial_dim_indices(&array_infos[0].dim_names, geo.spatial_dims.as_deref())?;
+ let spatial_dims_names: Vec<&str> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].dim_names[i].as_str())
+ .collect();
+ let chunk_spatial_shape: Vec<i64> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].chunk_shape[i] as i64)
+ .collect();
+
+ let group_transform = match geo.transform {
+ Some(t) => t,
+ None => {
+ // Both `spatial:transform` and `proj:*` are absent (the
+ // CRS-only case errored above). Fall back to identity pixel
+ // coordinates and breadcrumb a warning so spatial-join
+ // surprises are debuggable.
+ log::warn!(
+ "Zarr group at {group_uri} has no `spatial:transform`; falling back \
+ to the identity pixel-coordinate transform [0, 1, 0, 0, 0, -1]. \
+ Spatial operations against this raster will use pixel coordinates."
+ );
+ [0.0, 1.0, 0.0, 0.0, 0.0, -1.0]
+ }
+ };
+
+ // chunk_grid_shape comes from untrusted Zarr metadata; bound-check the
+ // product so a hostile or malformed grid can't drive RasterBuilder
+ // capacity to overflow (and trigger an OOM preallocation).
+ let total_rows = array_infos[0]
+ .chunk_grid_shape
+ .iter()
+ .try_fold(1usize, |acc, &n| {
+ usize::try_from(n).ok().and_then(|n| acc.checked_mul(n))
+ })
+ .ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk grid shape {:?} overflows usize",
+ array_infos[0].chunk_grid_shape
+ ))
+ })?;
+ let mut builder = RasterBuilder::new(total_rows);
+
+ // Walk the chunk grid in row-major (C-order) order. The outer-most
+ // axis varies slowest, the innermost fastest — same convention used
+ // for byte strides in `BandRefImpl`.
+ let mut chunk_indices = vec![0u64; array_infos[0].chunk_grid_shape.len()];
+ loop {
+ let row_transform = compute_row_transform(
+ &group_transform,
+ &chunk_indices,
+ &array_infos[0].chunk_shape,
+ &spatial_dim_indices,
+ );
+ let crs_str = geo.crs.as_deref();
+ builder.start_raster_nd(
+ &row_transform,
+ &spatial_dims_names,
+ &chunk_spatial_shape,
+ crs_str,
+ )?;
+
+ for info in &array_infos {
+ let dim_names_ref: Vec<&str> = info.dim_names.iter().map(String::as_str).collect();
+ let nodata_ref = info.nodata.as_deref();
+ // Chunk anchor is provenance metadata; InDb vs OutDb is
+ // discriminated by `data.is_empty()`, not by these fields.
+ let anchor = build_chunk_anchor(group_uri, &info.path, &chunk_indices);
+ builder.start_band_nd(
+ Some(info.path.as_str()),
+ &dim_names_ref,
+ &info.chunk_shape,
+ info.data_type,
+ nodata_ref,
+ Some(anchor.as_str()),
+ Some("zarr"),
+ )?;
+ match mode {
+ Mode::InDb => {
+ let bytes = retrieve_chunk_bytes(&info.array, &chunk_indices)?;
+ builder.band_data_writer().append_value(&bytes);
+ }
Review Comment:
I decided to just rip all pixel access out now and instead wait for outdb + RS_EnsureLoaded to land. at that point we will use that stack to make load_eager=true work. for now we will just error out when load_eager is true.
This PR will not allow pixel retrieval from zarrs
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3284284032
##########
python/sedonadb/Cargo.toml:
##########
@@ -23,10 +23,35 @@ edition = "2021"
[lib]
name = "_lib"
-crate-type = ["cdylib"]
+# `cdylib` is what maturin packages into the Python wheel. `rlib` is
+# what plugin crates (e.g. `python/sedonadb-zarr`) link against so they
+# can extract `InternalContext` from Python objects via PyO3.
+crate-type = ["cdylib", "rlib"]
[features]
-default = ["mimalloc"]
+# `extension-module` gates the `#[pymodule] fn _lib` symbol so plugin
+# crates (e.g. `sedonadb-zarr`) can link this as an rlib without
+# duplicating `PyInit__lib` in their own cdylib. NOT a default
+# feature: Cargo's workspace-wide feature unification means a default
+# would force it on for everyone, including the plugin link. Maturin
+# enables it explicitly via `[tool.maturin] features` in
+# `pyproject.toml` for the wheel build.
+#
+# `mimalloc` is intentionally *not* in `default`: when sedonadb is
+# loaded alongside a plugin cdylib (e.g. `sedonadb-zarr`) the
+# global-allocator mismatch corrupts the heap. Concrete Arrow types
+# like `RecordBatch` are monomorphised per-cdylib, so the drop site
+# runs the consumer's allocator regardless of who allocated, and
+# mimalloc-free on a libc-malloc pointer segfaults.
+#
+# !! WARNING: opting in with `--features mimalloc` re-introduces the
+# segfault for any user who imports a plugin alongside this wheel.
+# This isn't a perf knob — it's a "I'm shipping a closed binary that
+# will never load a plugin cdylib" knob. The plugin-safe path is to
+# link mimalloc *dynamically* (one shared `libmimalloc.so` for the
+# whole process), which is a packaging change, not a feature flag.
Review Comment:
datafusion ffi solved this one! plugin + mimalloc is working.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sedona-zarr plugin [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3293049230
##########
python/sedonadb/python/sedonadb/context.py:
##########
@@ -333,6 +346,50 @@ def read_pyogrio(
self.options,
)
+ def read_format(
+ self,
+ spec: "ExternalFormatSpec",
+ table_paths: Union[str, Path, Iterable[str]],
+ check_extension: bool = False,
+ ) -> DataFrame:
+ """Read one or more paths using a Python-defined `ExternalFormatSpec`.
Review Comment:
I almost forgot to create it! Would have made this feature challenging to use 😆
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3284104217
##########
.github/workflows/packaging.yml:
##########
@@ -159,7 +159,11 @@ jobs:
- name: Install dev SedonaDB Python
env:
- MATURIN_PEP517_ARGS: "--features s2geography,pyo3/extension-module"
+ # `extension-module` is sedonadb's own feature gating the
+ # `#[pymodule] fn _lib` symbol; required for the wheel to load
+ # as a Python module. Plugin crates linking the rlib leave it
+ # off to avoid `PyInit__lib` duplication.
+ MATURIN_PEP517_ARGS: "--features s2geography,pyo3/extension-module,extension-module"
Review Comment:
Can we remove this?
##########
ci/scripts/wheels-build-linux.sh:
##########
@@ -48,7 +48,7 @@ BEFORE_ALL_MANYLINUX="yum install -y curl zip unzip tar clang perl"
# add quite a bit of complexity but could save time if we build wheels for linux frequently.
# The native and Rust builds are cached on each image such that compile work is effectively
# cached between Python versions (just not between invocations of this script).
-export CIBW_ENVIRONMENT_LINUX="VCPKG_ROOT=/vcpkg VCPKG_REF=$VCPKG_REF VCPKG_DEFAULT_TRIPLET=$VCPKG_DEFAULT_TRIPLET CMAKE_TOOLCHAIN_FILE=/vcpkg/scripts/buildsystems/vcpkg.cmake PKG_CONFIG_PATH=/vcpkg/installed/$VCPKG_DEFAULT_TRIPLET/lib/pkgconfig LD_LIBRARY_PATH=/vcpkg/installed/$VCPKG_DEFAULT_TRIPLET/lib MATURIN_PEP517_ARGS='--features s2geography,pyo3/extension-module'"
+export CIBW_ENVIRONMENT_LINUX="VCPKG_ROOT=/vcpkg VCPKG_REF=$VCPKG_REF VCPKG_DEFAULT_TRIPLET=$VCPKG_DEFAULT_TRIPLET CMAKE_TOOLCHAIN_FILE=/vcpkg/scripts/buildsystems/vcpkg.cmake PKG_CONFIG_PATH=/vcpkg/installed/$VCPKG_DEFAULT_TRIPLET/lib/pkgconfig LD_LIBRARY_PATH=/vcpkg/installed/$VCPKG_DEFAULT_TRIPLET/lib MATURIN_PEP517_ARGS='--features s2geography,pyo3/extension-module,extension-module'"
Review Comment:
Can you remove extension-module here?
##########
ci/scripts/wheels-build-windows.ps1:
##########
@@ -73,7 +73,7 @@ $env:PATH += ";$scriptDirectory\windows"
$env:GEOS_LIB_DIR = "$vcpkgLibDirectory"
$env:GEOS_VERSION = "3.13.0"
-$env:MATURIN_PEP517_ARGS='--features s2geography,pyo3/extension-module'
+$env:MATURIN_PEP517_ARGS='--features s2geography,pyo3/extension-module,extension-module'
Review Comment:
Can you remove extension-module here?
##########
python/sedonadb/src/context.rs:
##########
@@ -200,4 +212,39 @@ impl InternalContext {
.to_string(),
))
}
+
+ /// Register a UDTF (table function) defined in a separate Python
+ /// extension (e.g. `sedonadb-zarr`).
+ ///
+ /// `spec` is the Python object exposing
+ /// `__datafusion_table_function__(session) -> PyCapsule`. The host
+ /// hands the plugin a session capsule carrying an
+ /// `FFI_LogicalExtensionCodec`; the plugin returns a capsule named
+ /// `"datafusion_table_function"` wrapping an
+ /// `FFI_TableFunction`. `datafusion-ffi` handles the actual
+ /// cross-cdylib trait-object conversion — concrete `RecordBatch`
+ /// allocations flow through Arrow C Stream's release callbacks, so
+ /// the plugin and host can use different allocators.
+ pub fn register_udtf_capsule(
+ &self,
+ py: Python<'_>,
+ name: &str,
+ spec: Bound<'_, PyAny>,
+ ) -> Result<(), PySedonaError> {
+ use pyo3::types::PyCapsule;
+
+ let session_capsule = crate::plugin::create_session_capsule(py, &self.udtf_task_provider)?;
+ let returned = spec
+ .getattr(crate::plugin::UDTF_ATTR)?
+ .call1((session_capsule,))?;
+ let returned = returned.downcast::<PyCapsule>().map_err(|e| {
+ PySedonaError::SedonaPython(format!(
+ "plugin's {} must return a PyCapsule, got {e}",
+ crate::plugin::UDTF_ATTR
+ ))
Review Comment:
Might be nice to have this follow `import_ffi_scalar_udf()` and use any of the utilities in that file if it's possible to do so
##########
.github/workflows/python.yml:
##########
@@ -117,8 +117,18 @@ jobs:
- name: Install
run: |
# Keep this export in sync with the export in dev/release/verify-release-candidate.sh
- export MATURIN_PEP517_ARGS="--features s2geography"
+ # `extension-module` is sedonadb's own feature gating the
+ # `#[pymodule] fn _lib` symbol — required for the wheel to
+ # load as a Python module. Plugin crates linking the rlib
+ # leave it off to avoid `PyInit__lib` duplication.
+ export MATURIN_PEP517_ARGS="--features s2geography,extension-module"
Review Comment:
Do you still need extension-module here?
##########
ci/scripts/wheels-build-macos.sh:
##########
@@ -47,7 +47,7 @@ source ./wheels-bootstrap-vcpkg.sh
export CIBW_REPAIR_WHEEL_COMMAND_MACOS="DYLD_LIBRARY_PATH=$VCPKG_INSTALL_NAME_DIR delocate-listdeps {wheel} && DYLD_LIBRARY_PATH=$VCPKG_INSTALL_NAME_DIR delocate-wheel --require-archs {delocate_archs} -w {dest_dir} {wheel}"
# Pass on environment variables specifically for the build
-export CIBW_ENVIRONMENT_MACOS="$CIBW_ENVIRONMENT_MACOS _PYTHON_HOST_PLATFORM=macosx-12.0-${SEDONADB_MACOS_ARCH} MACOSX_DEPLOYMENT_TARGET=12.0 CMAKE_TOOLCHAIN_FILE=${CMAKE_TOOLCHAIN_FILE} MATURIN_PEP517_ARGS='--features s2geography,pyo3/extension-module'"
+export CIBW_ENVIRONMENT_MACOS="$CIBW_ENVIRONMENT_MACOS _PYTHON_HOST_PLATFORM=macosx-12.0-${SEDONADB_MACOS_ARCH} MACOSX_DEPLOYMENT_TARGET=12.0 CMAKE_TOOLCHAIN_FILE=${CMAKE_TOOLCHAIN_FILE} MATURIN_PEP517_ARGS='--features s2geography,pyo3/extension-module,extension-module'"
Review Comment:
Can you remove extension-module here?
##########
python/sedonadb-zarr/python/sedonadb_zarr/__init__.py:
##########
@@ -0,0 +1,117 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Zarr support for SedonaDB.
+
+Activate by calling :func:`register` on a SedonaDB connection. After
+registration, two surfaces work:
+
+1. ``con.sql("SELECT * FROM sd_read_zarr('s3://...')")`` — SQL UDTF.
+2. ``con.read_format(ZarrFormatSpec(), uri)`` — DataFrame API via
+ ``ExternalFormatSpec``.
+
+>>> import sedonadb
+>>> import sedonadb_zarr
+>>> con = sedonadb.connect()
+>>> sedonadb_zarr.register(con)
+>>> con.sql("SELECT count(*) FROM sd_read_zarr('file:///path/to/foo.zarr')").show() # doctest: +SKIP
+
+The plugin is opt-in: SedonaDB itself does not bundle Zarr support, so
+applications that don't import ``sedonadb_zarr`` pay no zarr build or
+runtime cost.
+"""
+
+import json
+from typing import Any, Mapping, Optional
+
+from sedonadb.datasource import ExternalFormatSpec
+
+from sedonadb_zarr._zarr_lib import PyZarrChunkReader, ZarrTableFunction
+
+
+def register(con) -> None:
+ """Attach Zarr SQL support to a SedonaDB connection.
+
+ After this call, ``con.sql("SELECT * FROM sd_read_zarr(...)")``
+ works. Idempotent — calling twice on the same connection re-
+ registers the UDTF without error.
+
+ Parameters
+ ----------
+ con
+ A ``sedonadb`` ``Context`` (the object returned by
+ ``sedonadb.connect()``). Internally, this function hands a
+ :class:`ZarrTableFunction` instance to sedonadb's
+ ``register_udtf_capsule``, which uses ``datafusion-ffi``'s
+ ``FFI_TableFunction`` to bridge the cdylib boundary.
Review Comment:
Here too
##########
python/sedonadb-zarr/python/sedonadb_zarr/__init__.py:
##########
@@ -0,0 +1,117 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Zarr support for SedonaDB.
+
+Activate by calling :func:`register` on a SedonaDB connection. After
+registration, two surfaces work:
+
+1. ``con.sql("SELECT * FROM sd_read_zarr('s3://...')")`` — SQL UDTF.
+2. ``con.read_format(ZarrFormatSpec(), uri)`` — DataFrame API via
+ ``ExternalFormatSpec``.
+
+>>> import sedonadb
+>>> import sedonadb_zarr
+>>> con = sedonadb.connect()
+>>> sedonadb_zarr.register(con)
+>>> con.sql("SELECT count(*) FROM sd_read_zarr('file:///path/to/foo.zarr')").show() # doctest: +SKIP
+
+The plugin is opt-in: SedonaDB itself does not bundle Zarr support, so
+applications that don't import ``sedonadb_zarr`` pay no zarr build or
+runtime cost.
Review Comment:
This is written using ReST cross-linking but mkdocs is building this using Markdown
##########
python/sedonadb-zarr/pyproject.toml:
##########
@@ -0,0 +1,49 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[build-system]
+requires = ["maturin>=1.8,<2.0"]
+build-backend = "maturin"
+
+[project]
+name = "sedonadb-zarr"
+readme = "README.md"
+requires-python = ">=3.9"
+classifiers = [
+ "Programming Language :: Rust",
+ "Programming Language :: Python :: Implementation :: CPython",
+ "Programming Language :: Python :: Implementation :: PyPy",
+]
+dynamic = ["version"]
+dependencies = [
+ "sedonadb",
+]
+
+[project.optional-dependencies]
+test = [
+ "pytest",
+ "zarr",
+ "numpy",
+]
+
+[tool.maturin]
+features = ["pyo3/extension-module"]
+# `_zarr_lib` rather than `_lib` so the generated PyInit symbol
+# doesn't collide with sedonadb's `_lib` under cargo workspace
+# feature unification. The Python wrapper imports from this path.
+module-name = "sedonadb_zarr._zarr_lib"
+python-source = "python"
Review Comment:
This should be able to go back to a normal line without a comment
##########
python/sedonadb-zarr/python/sedonadb_zarr/__init__.py:
##########
@@ -0,0 +1,117 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Zarr support for SedonaDB.
+
+Activate by calling :func:`register` on a SedonaDB connection. After
+registration, two surfaces work:
+
+1. ``con.sql("SELECT * FROM sd_read_zarr('s3://...')")`` — SQL UDTF.
+2. ``con.read_format(ZarrFormatSpec(), uri)`` — DataFrame API via
+ ``ExternalFormatSpec``.
+
+>>> import sedonadb
+>>> import sedonadb_zarr
+>>> con = sedonadb.connect()
+>>> sedonadb_zarr.register(con)
+>>> con.sql("SELECT count(*) FROM sd_read_zarr('file:///path/to/foo.zarr')").show() # doctest: +SKIP
+
+The plugin is opt-in: SedonaDB itself does not bundle Zarr support, so
+applications that don't import ``sedonadb_zarr`` pay no zarr build or
+runtime cost.
+"""
+
+import json
+from typing import Any, Mapping, Optional
+
+from sedonadb.datasource import ExternalFormatSpec
+
+from sedonadb_zarr._zarr_lib import PyZarrChunkReader, ZarrTableFunction
+
+
+def register(con) -> None:
+ """Attach Zarr SQL support to a SedonaDB connection.
+
+ After this call, ``con.sql("SELECT * FROM sd_read_zarr(...)")``
+ works. Idempotent — calling twice on the same connection re-
+ registers the UDTF without error.
+
+ Parameters
+ ----------
+ con
+ A ``sedonadb`` ``Context`` (the object returned by
+ ``sedonadb.connect()``). Internally, this function hands a
+ :class:`ZarrTableFunction` instance to sedonadb's
+ ``register_udtf_capsule``, which uses ``datafusion-ffi``'s
+ ``FFI_TableFunction`` to bridge the cdylib boundary.
+ """
+ internal_ctx = getattr(con, "_impl", None)
+ if internal_ctx is None:
+ raise TypeError(
+ "sedonadb_zarr.register: could not locate the InternalContext on "
+ f"{type(con).__name__}; expected attribute `_impl`."
+ )
+ internal_ctx.register_udtf_capsule("sd_read_zarr", ZarrTableFunction())
+
+
+class ZarrFormatSpec(ExternalFormatSpec):
+ """`ExternalFormatSpec` for Zarr groups.
+
+ Use with ``con.read_format(spec, uri)``:
+
+ >>> con.read_format(ZarrFormatSpec(), "file:///path/to/foo.zarr") # doctest: +SKIP
+
+ Supported options (via :meth:`with_options`):
+
+ - ``arrays`` (``list[str]`` or JSON-string) — explicit subset of
+ group arrays to read.
+ """
Review Comment:
This is also ReST and not markdown
##########
python/sedonadb-zarr/Cargo.toml:
##########
@@ -0,0 +1,43 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[package]
+name = "sedonadb-zarr"
+version = { workspace = true }
+publish = false
+edition = "2021"
+
+[lib]
+# rustc crate name. Distinct from `sedonadb`'s `_lib` to avoid the
+# rustdoc output-path collision on `cargo doc --workspace`. The
+# matching pymodule function in `src/lib.rs` is named `_zarr_lib`
+# so its `PyInit__zarr_lib` symbol doesn't collide with sedonadb's
+# `PyInit__lib` under cargo workspace feature unification.
+name = "_zarr_lib"
+crate-type = ["cdylib"]
+
+[dependencies]
+arrow-array = { workspace = true }
+# UDTF handoff to sedonadb is via `datafusion-ffi`'s `FFI_TableFunction`
+# (the `datafusion-python` pattern). The plugin and host don't share
+# any Rust types directly — concrete `RecordBatch` traffic flows
+# through Arrow C with release callbacks owned by the producing
+# cdylib, so plugin and host can use different allocators.
+datafusion-catalog = { workspace = true }
Review Comment:
You should be able to remove these comments since this is now back to a normal Python with maturin crate
##########
python/sedonadb-zarr/src/lib.rs:
##########
@@ -0,0 +1,177 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! `sedonadb-zarr` — Python plugin package wiring Zarr support into a
+//! `sedonadb` session.
+//!
+//! Two PyO3-exposed surfaces:
+//! - [`ZarrTableFunction`] — Python class with
+//! `__datafusion_table_function__(session)` that returns an
+//! `FFI_TableFunction` PyCapsule. sedonadb's
+//! `InternalContext.register_udtf` attaches it under
+//! `sd_read_zarr`.
+//! - [`PyZarrChunkReader`] — streaming reader producible from Python,
+//! exposing `__arrow_c_stream__` so it plugs into
+//! `ExternalFormatSpec.open_reader` (the `con.read_format(spec, uri)`
+//! surface).
+//!
+//! The Rust side carries no dependency on `sedonadb`'s host extension
+//! — UDTF handoff is via `datafusion-ffi`, which gives an ABI-stable
+//! C-level interface so plugin and host cdylibs don't need to share
+//! anything beyond Arrow C Data/Stream and the FFI structs.
+
+use std::ffi::CString;
+use std::ptr::NonNull;
+use std::sync::{Arc, Mutex};
+
+use arrow_array::ffi_stream::FFI_ArrowArrayStream;
+use datafusion_catalog::TableFunctionImpl;
+use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec;
+use datafusion_ffi::udtf::FFI_TableFunction;
+use pyo3::exceptions::{PyRuntimeError, PyValueError};
+use pyo3::prelude::*;
+use pyo3::types::PyCapsule;
+use sedona_raster_zarr::{ZarrChunkReader, ZarrReadFunction};
+
+/// `c"datafusion_table_function"` capsule name expected by sedonadb's
+/// (and datafusion-python's) host-side UDTF registration.
+const UDTF_CAPSULE_NAME: &std::ffi::CStr = c"datafusion_table_function";
+
+/// Codec capsule name the host passes to us via the `session` argument.
+const CODEC_CAPSULE_NAME: &std::ffi::CStr = c"datafusion_logical_extension_codec";
+
+/// Zarr UDTF surface. The Python plugin instantiates one of these and
+/// hands the instance to sedonadb's `con._impl.register_udtf` (Python
+/// wrapper). Sedonadb then calls `__datafusion_table_function__(session)`
+/// on it to obtain the FFI struct.
+#[pyclass(name = "ZarrTableFunction", module = "sedonadb_zarr")]
+#[derive(Default, Debug, Clone)]
+pub struct ZarrTableFunction;
Review Comment:
This table function is causing a lot of code and isn't necessary to access this from Python. Are you sure it needs to exist or can you push it to a follow up and evaluate the priority later? We don't have a `read_parquet()` either.
##########
python/sedonadb-zarr/src/lib.rs:
##########
@@ -0,0 +1,177 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! `sedonadb-zarr` — Python plugin package wiring Zarr support into a
+//! `sedonadb` session.
+//!
+//! Two PyO3-exposed surfaces:
+//! - [`ZarrTableFunction`] — Python class with
+//! `__datafusion_table_function__(session)` that returns an
+//! `FFI_TableFunction` PyCapsule. sedonadb's
+//! `InternalContext.register_udtf` attaches it under
+//! `sd_read_zarr`.
+//! - [`PyZarrChunkReader`] — streaming reader producible from Python,
+//! exposing `__arrow_c_stream__` so it plugs into
+//! `ExternalFormatSpec.open_reader` (the `con.read_format(spec, uri)`
+//! surface).
+//!
+//! The Rust side carries no dependency on `sedonadb`'s host extension
+//! — UDTF handoff is via `datafusion-ffi`, which gives an ABI-stable
+//! C-level interface so plugin and host cdylibs don't need to share
+//! anything beyond Arrow C Data/Stream and the FFI structs.
+
+use std::ffi::CString;
+use std::ptr::NonNull;
+use std::sync::{Arc, Mutex};
+
+use arrow_array::ffi_stream::FFI_ArrowArrayStream;
+use datafusion_catalog::TableFunctionImpl;
+use datafusion_ffi::proto::logical_extension_codec::FFI_LogicalExtensionCodec;
+use datafusion_ffi::udtf::FFI_TableFunction;
+use pyo3::exceptions::{PyRuntimeError, PyValueError};
+use pyo3::prelude::*;
+use pyo3::types::PyCapsule;
+use sedona_raster_zarr::{ZarrChunkReader, ZarrReadFunction};
+
+/// `c"datafusion_table_function"` capsule name expected by sedonadb's
+/// (and datafusion-python's) host-side UDTF registration.
+const UDTF_CAPSULE_NAME: &std::ffi::CStr = c"datafusion_table_function";
+
+/// Codec capsule name the host passes to us via the `session` argument.
+const CODEC_CAPSULE_NAME: &std::ffi::CStr = c"datafusion_logical_extension_codec";
+
+/// Zarr UDTF surface. The Python plugin instantiates one of these and
+/// hands the instance to sedonadb's `con._impl.register_udtf` (Python
+/// wrapper). Sedonadb then calls `__datafusion_table_function__(session)`
+/// on it to obtain the FFI struct.
+#[pyclass(name = "ZarrTableFunction", module = "sedonadb_zarr")]
+#[derive(Default, Debug, Clone)]
+pub struct ZarrTableFunction;
+
+#[pymethods]
+impl ZarrTableFunction {
+ #[new]
+ fn new() -> Self {
+ Self
+ }
+
+ fn __datafusion_table_function__<'py>(
+ &self,
+ py: Python<'py>,
+ session: Bound<'py, PyAny>,
+ ) -> PyResult<Bound<'py, PyCapsule>> {
+ let codec = ffi_logical_codec_from_pycapsule(&session)?;
+ let udtf: Arc<dyn TableFunctionImpl> = Arc::new(ZarrReadFunction::default());
+ let provider = FFI_TableFunction::new_with_ffi_codec(udtf, None, codec);
+ let name = CString::new(UDTF_CAPSULE_NAME.to_bytes()).unwrap();
+ PyCapsule::new(py, provider, Some(name))
+ }
+}
+
+/// Extract an [`FFI_LogicalExtensionCodec`] from a session capsule the
+/// host hands us. Accepts either a raw capsule named
+/// `"datafusion_logical_extension_codec"` or any object exposing
+/// `__datafusion_logical_extension_codec__()` that returns one.
+fn ffi_logical_codec_from_pycapsule(obj: &Bound<'_, PyAny>) -> PyResult<FFI_LogicalExtensionCodec> {
+ let attr_name = "__datafusion_logical_extension_codec__";
+ let capsule = if obj.hasattr(attr_name)? {
+ obj.getattr(attr_name)?.call0()?
+ } else {
+ obj.clone()
+ };
+ let capsule = capsule.downcast::<PyCapsule>().map_err(|e| {
+ PyValueError::new_err(format!(
+ "session capsule must be a PyCapsule (or expose {attr_name}), got {e}"
+ ))
+ })?;
+ let name = capsule
+ .name()?
+ .ok_or_else(|| PyValueError::new_err("session capsule has no name"))?;
+ if name != CODEC_CAPSULE_NAME {
+ return Err(PyValueError::new_err(format!(
+ "session capsule name mismatch: expected {CODEC_CAPSULE_NAME:?}, got {name:?}"
+ )));
+ }
+ let ptr = capsule.pointer() as *mut FFI_LogicalExtensionCodec;
+ let ptr = NonNull::new(ptr)
+ .ok_or_else(|| PyValueError::new_err("session capsule pointer is null"))?;
+ // SAFETY: capsule name was verified above; PyO3 keeps the value
+ // alive for the lifetime of the capsule. `clone()` runs the FFI
+ // release-aware clone hook so we end up with an independent codec
+ // that outlives this scope.
+ let codec = unsafe { ptr.as_ref().clone() };
+ Ok(codec)
+}
+
+/// Python-callable wrapper around `ZarrChunkReader` that exposes
+/// `__arrow_c_stream__`. Consumed on first call (subsequent calls
+/// error) — matching pyarrow's `RecordBatchReader` convention.
+///
+/// The Python `ZarrFormatSpec.open_reader` returns one of these; the
+/// `ExternalFormatSpec` framework then drives the chunk grid via the
+/// Arrow C stream protocol.
+#[pyclass]
+pub struct PyZarrChunkReader {
+ inner: Mutex<Option<ZarrChunkReader>>,
+}
+
+#[pymethods]
+impl PyZarrChunkReader {
+ #[new]
+ #[pyo3(signature = (uri, arrays=None, batch_size=8192))]
+ fn new(uri: &str, arrays: Option<Vec<String>>, batch_size: usize) -> PyResult<Self> {
+ let reader = ZarrChunkReader::try_new(uri, arrays.as_deref(), batch_size)
+ .map_err(|e| PyValueError::new_err(e.to_string()))?;
+ Ok(Self {
+ inner: Mutex::new(Some(reader)),
+ })
+ }
+
+ #[pyo3(signature = (requested_schema=None))]
+ fn __arrow_c_stream__<'py>(
+ &self,
+ py: Python<'py>,
+ #[allow(unused_variables)] requested_schema: Option<Bound<'py, PyCapsule>>,
+ ) -> PyResult<Bound<'py, PyCapsule>> {
+ let reader = self
+ .inner
+ .lock()
+ .map_err(|_| PyRuntimeError::new_err("PyZarrChunkReader mutex poisoned"))?
+ .take()
+ .ok_or_else(|| {
+ PyRuntimeError::new_err(
+ "PyZarrChunkReader has already been consumed; \
+ a RecordBatchReader can only be exported once.",
+ )
+ })?;
+ let ffi_stream = FFI_ArrowArrayStream::new(Box::new(reader));
+ let capsule_name = CString::new("arrow_array_stream")
+ .map_err(|e| PyRuntimeError::new_err(e.to_string()))?;
+ PyCapsule::new(py, ffi_stream, Some(capsule_name))
+ }
+}
+
+// Named `_zarr_lib` (not `_lib`) so the generated `PyInit__zarr_lib`
+// symbol doesn't collide with sedonadb's `PyInit__lib` when cargo's
+// workspace feature unification (e.g. `cargo build --all-features`)
+// brings sedonadb's pymodule into our cdylib's link.
+#[pymodule]
+fn _zarr_lib(m: &Bound<'_, PyModule>) -> PyResult<()> {
+ m.add_class::<ZarrTableFunction>()?;
Review Comment:
This can go back to `_lib`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3281631173
##########
python/sedonadb-zarr/python/sedonadb_zarr/__init__.py:
##########
@@ -0,0 +1,127 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Zarr support for SedonaDB.
+
+Activate by calling :func:`register` on a SedonaDB connection. After
+registration, two surfaces work:
+
+1. ``con.sql("SELECT * FROM sd_read_zarr('s3://...')")`` — SQL UDTF.
+2. ``con.read_format(ZarrFormatSpec(), uri)`` — DataFrame API via
+ ``ExternalFormatSpec``.
+
+>>> import sedonadb
+>>> import sedonadb_zarr
+>>> con = sedonadb.connect()
+>>> sedonadb_zarr.register(con)
+>>> con.sql("SELECT count(*) FROM sd_read_zarr('file:///path/to/foo.zarr')").show() # doctest: +SKIP
+
+The plugin is opt-in: SedonaDB itself does not bundle Zarr support, so
+applications that don't import ``sedonadb_zarr`` pay no zarr build or
+runtime cost.
+"""
+
+import json
+from typing import Any, Mapping, Optional
+
+from sedonadb.datasource import ExternalFormatSpec
+
+from sedonadb_zarr._zarr_lib import PyZarrChunkReader, zarr_udtf_capsule
+
+
+def register(con) -> None:
+ """Attach Zarr SQL support to a SedonaDB connection.
+
+ After this call, ``con.sql("SELECT * FROM sd_read_zarr(...)")``
+ works. Idempotent — calling twice on the same connection re-
+ registers the UDTF without error.
+
+ Parameters
+ ----------
+ con
+ A ``sedonadb`` ``Context`` (the object returned by
+ ``sedonadb.connect()``). Internally, this function extracts
+ the underlying ``InternalContext`` PyO3 handle and registers
+ the UDTF on its DataFusion ``SessionContext`` via a
+ ``PyCapsule`` handoff — the only viable cross-extension
+ transport for the UDTF trait object.
+ """
+ internal_ctx = getattr(con, "_impl", None)
+ if internal_ctx is None:
+ raise TypeError(
+ "sedonadb_zarr.register: could not locate the InternalContext on "
+ f"{type(con).__name__}; expected attribute `_impl`."
+ )
+ capsule = zarr_udtf_capsule()
+ internal_ctx.register_udtf_capsule("sd_read_zarr", capsule)
+
+
+class ZarrFormatSpec(ExternalFormatSpec):
+ """`ExternalFormatSpec` for Zarr groups.
+
+ Use with ``con.read_format(spec, uri)``:
+
+ >>> con.read_format(ZarrFormatSpec(), "file:///path/to/foo.zarr") # doctest: +SKIP
+
+ Supported options (via :meth:`with_options`):
+
+ - ``load_eager`` (``bool``) — ``False`` (default) emits chunk-anchor
+ URIs only; ``True`` currently errors pending the async resolver.
+ - ``arrays`` (``list[str]`` or JSON-string) — explicit subset of
+ group arrays to read.
+ """
+
+ def __init__(self, options: Optional[Mapping[str, Any]] = None):
+ self._options: dict = dict(options) if options else {}
+
+ @property
+ def extension(self) -> str:
+ return ".zarr"
+
+ @property
+ def list_single_object(self) -> bool:
+ # A Zarr group is a directory, not a file. The DataFusion
+ # listing layer would enumerate its contents (zarr.json, chunk
+ # shards, ...), none of which carry the `.zarr` extension. The
+ # Rust `SingleObjectExternalTable` path skips listing and
+ # hands the URI straight to `open_reader`.
+ return True
+
+ def with_options(self, options: Mapping[str, Any]) -> "ZarrFormatSpec":
+ merged = {**self._options, **options}
+ return ZarrFormatSpec(merged)
+
+ def open_reader(self, args: Any) -> PyZarrChunkReader:
+ uri = args.src.to_url()
+ if uri is None:
+ raise ValueError(
+ "ZarrFormatSpec: could not resolve a URL from the source object"
+ )
+ if self._options.get("load_eager"):
+ raise NotImplementedError(
+ "ZarrFormatSpec: load_eager=True is not yet supported. "
+ "Pixel-byte materialisation lands with the async RS_EnsureLoaded "
+ "resolver."
+ )
Review Comment:
We could also just not provide this option yet?
##########
python/sedonadb-zarr/src/lib.rs:
##########
@@ -0,0 +1,126 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! `sedonadb-zarr` — Python plugin package wiring Zarr support into a
+//! `sedonadb` session.
+//!
+//! Two PyO3-exposed surfaces:
+//! - `zarr_udtf_capsule()` — builds a `PyCapsule` carrying the
+//! `sd_read_zarr` UDTF implementation. The Python wrapper hands it
+//! to sedonadb's `InternalContext.register_udtf_capsule` to attach
+//! the SQL function.
+//! - `PyZarrChunkReader` — a streaming reader producible from Python,
+//! exposing `__arrow_c_stream__` so it plugs into
+//! `ExternalFormatSpec.open_reader` (the `con.read_format(spec, uri)`
+//! surface).
+//!
+//! The Python `ZarrFormatSpec(ExternalFormatSpec)` class in
+//! `sedonadb_zarr/__init__.py` wraps `PyZarrChunkReader`; the Rust side
+//! here is intentionally thin.
+
+use std::ffi::CString;
+use std::sync::{Arc, Mutex};
+
+use arrow_array::ffi_stream::FFI_ArrowArrayStream;
+use datafusion::catalog::TableFunctionImpl;
+use pyo3::exceptions::{PyRuntimeError, PyValueError};
+use pyo3::prelude::*;
+use pyo3::types::PyCapsule;
+use sedona_raster_zarr::{ZarrChunkReader, ZarrReadFunction};
+// `sedonadb`'s rustc crate name is `_lib` (set by `[lib].name` so
+// maturin packages it as `sedonadb._lib`); import the plugin handoff
+// types under that name.
+use _lib::plugin::{UdtfCapsule, UDTF_CAPSULE_NAME};
+
+/// Build a PyCapsule carrying the Zarr UDTF for handoff to sedonadb's
+/// `InternalContext.register_udtf_capsule`.
+///
+/// Cross-extension `#[pyclass]` extraction doesn't work in PyO3 (each
+/// cdylib has its own type-id static), so we pass the UDTF
+/// implementation across the extension boundary via an opaque
+/// [`UdtfCapsule`]. The capsule owns its `Arc`; the consumer clones a
+/// fresh refcount before the capsule is dropped. The payload's leading
+/// magic sentinel lets the consumer fail fast if the capsule's name
+/// has been spoofed.
+#[pyfunction]
+fn zarr_udtf_capsule(py: Python<'_>) -> PyResult<Bound<'_, PyCapsule>> {
+ let udtf: Arc<dyn TableFunctionImpl> = Arc::new(ZarrReadFunction::default());
+ let payload = UdtfCapsule::new(udtf);
+ // CString::new requires owned bytes; reuse the host-side constant
+ // so the name (and its `.vN` ABI tag) lives in exactly one place.
+ let name = CString::new(UDTF_CAPSULE_NAME.to_bytes()).unwrap();
+ PyCapsule::new_with_destructor(py, payload, Some(name), |_v: UdtfCapsule, _ctx| {})
+}
Review Comment:
I think you can use the built-in pattern for UDTF capsules in datafusion-python:
https://github.com/apache/datafusion-python/blob/dac9ec6230dba8717d8a0d27de19a141600486b1/examples/datafusion-ffi-example/src/table_function.rs#L30-L66
We already support this in SedonaDB Python for scalar_udf and table provider if I remember correctly.
##########
python/sedonadb-zarr/Cargo.toml:
##########
@@ -0,0 +1,44 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[package]
+name = "sedonadb-zarr"
+version = { workspace = true }
+publish = false
+edition = "2021"
+
+[lib]
+# rustc crate name. Distinct from `sedonadb`'s `_lib` to avoid the
+# rustdoc output-path collision on `cargo doc --workspace`. The
+# matching pymodule function in `src/lib.rs` is named `_zarr_lib`
+# so its `PyInit__zarr_lib` symbol doesn't collide with sedonadb's
+# `PyInit__lib` under cargo workspace feature unification.
+name = "_zarr_lib"
+crate-type = ["cdylib"]
+
+[dependencies]
+arrow-array = { workspace = true }
+datafusion = { workspace = true }
+pyo3 = { version = "0.25.1" }
+sedona-raster-zarr = { workspace = true }
+# The sedonadb Python extension; its rustc crate name is `_lib` per
+# the `[lib].name` override in `python/sedonadb/Cargo.toml`. We import
+# it in `src/lib.rs` via the leading `::_lib` syntax to disambiguate
+# from our own crate's `_lib` pymodule. Inlined here (not via
+# `workspace = true`) to avoid a self-referential workspace dep that
+# trips `cargo metadata` in cibuildwheel's isolated build env.
+sedonadb = { path = "../sedonadb", default-features = false }
Review Comment:
This part we need to remove (we can't take on a dependency to another Python package's private crate in another Python package to safely separate the builds). I believe the UDTF FFI suggestion I made above will eliminate the need for this one. If that doesn't work, eliminating the UDTF is probably better than this (i.e., just do the externalformatspec).
##########
python/sedonadb-zarr/pyproject.toml:
##########
@@ -0,0 +1,49 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[build-system]
+requires = ["maturin>=1.8,<2.0"]
+build-backend = "maturin"
+
+[project]
+name = "sedonadb-zarr"
+readme = "README.md"
+requires-python = ">=3.9"
+classifiers = [
+ "Programming Language :: Rust",
+ "Programming Language :: Python :: Implementation :: CPython",
+ "Programming Language :: Python :: Implementation :: PyPy",
+]
+dynamic = ["version"]
+dependencies = [
+ "sedonadb",
+]
Review Comment:
Don't do this if it causes problems in CI, but this will need to be `sedonadb>=0.4.0` because it relies on newly added features.
##########
python/sedonadb/Cargo.toml:
##########
@@ -23,10 +23,35 @@ edition = "2021"
[lib]
name = "_lib"
-crate-type = ["cdylib"]
+# `cdylib` is what maturin packages into the Python wheel. `rlib` is
+# what plugin crates (e.g. `python/sedonadb-zarr`) link against so they
+# can extract `InternalContext` from Python objects via PyO3.
+crate-type = ["cdylib", "rlib"]
[features]
-default = ["mimalloc"]
+# `extension-module` gates the `#[pymodule] fn _lib` symbol so plugin
+# crates (e.g. `sedonadb-zarr`) can link this as an rlib without
+# duplicating `PyInit__lib` in their own cdylib. NOT a default
+# feature: Cargo's workspace-wide feature unification means a default
+# would force it on for everyone, including the plugin link. Maturin
+# enables it explicitly via `[tool.maturin] features` in
+# `pyproject.toml` for the wheel build.
+#
+# `mimalloc` is intentionally *not* in `default`: when sedonadb is
+# loaded alongside a plugin cdylib (e.g. `sedonadb-zarr`) the
+# global-allocator mismatch corrupts the heap. Concrete Arrow types
+# like `RecordBatch` are monomorphised per-cdylib, so the drop site
+# runs the consumer's allocator regardless of who allocated, and
+# mimalloc-free on a libc-malloc pointer segfaults.
+#
+# !! WARNING: opting in with `--features mimalloc` re-introduces the
+# segfault for any user who imports a plugin alongside this wheel.
+# This isn't a perf knob — it's a "I'm shipping a closed binary that
+# will never load a plugin cdylib" knob. The plugin-safe path is to
+# link mimalloc *dynamically* (one shared `libmimalloc.so` for the
+# whole process), which is a packaging change, not a feature flag.
Review Comment:
Hopefully this goes away with the DataFusionFFI approach. If not, we should remove the table function completely because mimalloc is more important (mimalloc is what makes returning `Vec<>`s in a loop much faster than with a system allocator).
##########
rust/sedona-datasource/src/provider.rs:
##########
@@ -110,3 +149,153 @@ impl ReadOptions<'_> for RecordBatchReaderTableOptions {
.await
}
}
+
+/// [`TableProvider`] that treats each input URI as one opaque object.
+///
+/// Built when [`ExternalFormatSpec::list_single_object`] is `true`. The
+/// listing layer is bypassed entirely: each URI is synthesised into a
+/// single-element [`PartitionedFile`] whose `object_meta.location` is
+/// the URL's path within the [`ObjectStoreUrl`]. The format's
+/// [`ExternalFileFormat::file_source`] then drives the same scan path
+/// used by [`ListingTable`] — projections, filter pushdown, and
+/// streaming behaviour are identical.
Review Comment:
This is very cool, although I wonder if `SELECT * FROM 'file://path/to/something.zarr'` will work (not really an issue if it doesn't, and we can customize the filesystem catalog if we really want it to work)
##########
rust/sedona-raster-zarr/src/source_uri.rs:
##########
@@ -0,0 +1,285 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Zarr URI helpers — group locators and per-chunk OutDb anchors.
+//!
+//! Two URI shapes flow through the loader:
+//!
+//! 1. **Group URI** (the loader entry-point argument). Identifies a Zarr
+//! group on a backend, e.g. `file:///tmp/datacube.zarr` or a bare
+//! local path. Cloud schemes (`s3://`, `gs://`, `az://`, `https://`)
+//! will be accepted by a future revision; today only `file://` and
+//! bare-path are recognised.
+//! 2. **Chunk anchor URI** (written into a band's `outdb_uri`). Addresses
+//! one chunk in one array within a group:
+//! `<store-uri>#array=<array-path>&chunk=<i0>,<i1>,...`. The store URI
+//! is the original group URI verbatim (whatever scheme that uses);
+//! array path and chunk indices both live in the fragment so the
+//! store URI is unambiguous even when both contain `/` (e.g.
+//! `s3://bucket/foo.zarr/2024` + `subgroup/B01`). Inner-chunk indices
+//! always — sharding is a storage detail the resolver handles.
+//!
+//! The "this is a zarr anchor" signal lives in the band's
+//! `outdb_format = "zarr"` field, not in a URI scheme prefix — matches
+//! the GDAL OutDb convention and lets the format-keyed dispatcher
+//! route without parsing URI schemes.
+
+use arrow_schema::ArrowError;
+
+/// Parts of a chunk-anchor URI.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct ChunkAnchor {
+ /// Original store URI for the *group* (e.g. `file:///tmp/foo.zarr`,
+ /// `s3://bucket/foo.zarr/2024`). Same value across every chunk of every
+ /// array in a group.
+ pub store_uri: String,
+ /// Array's path within the store (e.g. `temperature`, `subgroup/B01`).
+ pub array_path: String,
+ /// Chunk's position in the array's inner chunk grid (one index per
+ /// array dimension).
+ pub chunk_indices: Vec<u64>,
+}
+
+/// Build a chunk anchor URI from its parts.
+///
+/// Format: `<store_uri>#array=<array_path>&chunk=<i0>,<i1>,...`. The
+/// store URI is the original group URI verbatim; array path and chunk
+/// indices live in the fragment so the store URI is unambiguous even
+/// when path components themselves contain `/`. "This is zarr" lives
+/// in the band's `outdb_format` field rather than in a URI scheme.
+pub fn build_chunk_anchor(store_uri: &str, array_path: &str, chunk_indices: &[u64]) -> String {
+ let indices = chunk_indices
+ .iter()
+ .map(|i| i.to_string())
+ .collect::<Vec<_>>()
+ .join(",");
+ let array = array_path.trim_start_matches('/');
+ format!("{store_uri}#array={array}&chunk={indices}")
+}
+
+/// Parse a chunk-anchor URI back into its parts.
+///
+/// Strict: rejects URIs that don't carry both `array=` and `chunk=`
+/// fragment parameters with valid values.
+pub fn parse_chunk_anchor(uri: &str) -> Result<ChunkAnchor, ArrowError> {
+ let (store_uri, fragment) = uri.split_once('#').ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk anchor URI missing `#array=...&chunk=...` fragment: {uri}"
+ ))
+ })?;
+
+ let mut array_path: Option<&str> = None;
+ let mut chunk_str: Option<&str> = None;
+ for pair in fragment.split('&') {
+ if let Some(v) = pair.strip_prefix("array=") {
+ array_path = Some(v);
+ } else if let Some(v) = pair.strip_prefix("chunk=") {
+ chunk_str = Some(v);
+ }
+ // Unknown fragment params are ignored — forward-compatible for
+ // future per-anchor extensions (e.g. version pins).
+ }
+
+ let array_path = array_path
+ .ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk anchor URI fragment missing `array=` parameter: {uri}"
+ ))
+ })?
+ .to_string();
+ let indices_str = chunk_str.ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk anchor URI fragment missing `chunk=` parameter: {uri}"
+ ))
+ })?;
+ if indices_str.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "chunk anchor URI has empty index list: {uri}"
+ )));
+ }
+ let chunk_indices = indices_str
+ .split(',')
+ .map(|s| {
+ s.parse::<u64>().map_err(|_| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk index `{s}` is not a non-negative integer in {uri}"
+ ))
+ })
+ })
+ .collect::<Result<Vec<_>, _>>()?;
+
+ Ok(ChunkAnchor {
+ store_uri: store_uri.to_string(),
+ array_path,
+ chunk_indices,
+ })
+}
+
+/// Normalize a user-supplied group URI into a local filesystem path.
+///
+/// Only `file://` and bare-path URIs are supported. Cloud schemes
+/// (`s3://`, `gs://`, `az://`, `https://`) error with a clear message.
+pub fn group_uri_to_filesystem_path(uri: &str) -> Result<std::path::PathBuf, ArrowError> {
Review Comment:
One of the things we may need to figure out how to do is get the object store properly across the FFI boundary (which we're currently doing with URIs, but there are issues with bridging the authentication between the store in sedonadb and sedonadb-zarr). Not a problem for this PR.
##########
rust/sedona-raster-zarr/src/loader.rs:
##########
@@ -0,0 +1,814 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Zarr group → N-D raster `StructArray` entry points.
+//!
+//! Both `group_to_indb_rasters` and `group_to_outdb_rasters` produce the
+//! same row shape: one raster row per chunk position, with one band per
+//! array in the group. They differ only in how each row's pixel bytes
+//! are delivered:
+//!
+//! - **InDb** — every chunk is fetched eagerly and copied into the
+//! Arrow `data` column. Heavy for large datacubes; intended for
+//! snapshots.
+//! - **OutDb** — `data` is left empty; each band's `outdb_uri` carries a
+//! chunk anchor (`zarr://<store-uri>/<array-path>#chunk=i0,i1,...`).
+//! Byte resolution awaits the format-keyed dispatch work in a
+//! follow-up PR.
+
+use std::sync::Arc;
+
+use arrow_array::{RecordBatch, RecordBatchReader};
+use arrow_schema::{ArrowError, Schema, SchemaRef};
+use sedona_common::sedona_internal_datafusion_err;
+use sedona_raster::builder::RasterBuilder;
+use sedona_schema::datatypes::SedonaType;
+use sedona_schema::raster::BandDataType;
+use zarrs::array::{Array, ArrayBytes};
+use zarrs::group::Group;
+use zarrs_filesystem::FilesystemStore;
+
+use crate::dtype::zarr_to_band_data_type;
+use crate::geozarr::GroupGeoMetadata;
+use crate::source_uri::{build_chunk_anchor, group_uri_to_filesystem_path};
+
+/// Streaming reader over the chunk grid of a Zarr group.
+///
+/// Each `next()` call emits one `RecordBatch` containing up to
+/// `batch_size` rows; one row per chunk position. The reader holds the
+/// open group, parsed metadata, and the current chunk-grid position —
+/// metadata parsing happens once in [`ZarrChunkReader::try_new`] and
+/// the per-row work is just transform arithmetic + anchor URI
+/// formatting.
+///
+/// Rows always emit OutDb-style: `data` is empty, `outdb_uri` carries
+/// a chunk anchor that the async OutDb resolver (registered separately)
+/// resolves to bytes on demand.
+pub struct ZarrChunkReader {
+ schema: SchemaRef,
+ group_uri: String,
+ array_infos: Vec<ArrayInfo>,
+ geo: GroupGeoMetadata,
+ group_transform: [f64; 6],
+ spatial_dim_indices: Vec<usize>,
+ /// Owned copy of `array_infos[0].dim_names[i]` for `i` in
+ /// `spatial_dim_indices`. Kept here so `next()` doesn't need to
+ /// rebuild the `&str` slice each row.
+ spatial_dims_names: Vec<String>,
+ chunk_spatial_shape: Vec<i64>,
+ /// Current position in the chunk grid (row-major, C-order).
+ chunk_indices: Vec<u64>,
+ /// Whether the grid has been exhausted.
+ exhausted: bool,
+ batch_size: usize,
+}
+
+impl std::fmt::Debug for ZarrChunkReader {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ f.debug_struct("ZarrChunkReader")
+ .field("group_uri", &self.group_uri)
+ .field("num_arrays", &self.array_infos.len())
+ .field("chunk_indices", &self.chunk_indices)
+ .field("exhausted", &self.exhausted)
+ .field("batch_size", &self.batch_size)
+ .finish()
+ }
+}
+
+impl ZarrChunkReader {
+ /// Open a Zarr group and prepare a streaming reader over its chunk
+ /// grid. Performs all I/O for metadata up front (open group, parse
+ /// attributes, list arrays, validate constraints) so per-batch work
+ /// is cheap.
+ ///
+ /// `arrays`:
+ /// - `None` — read every multi-dimensional array. 1-D arrays
+ /// (typical xarray coord variables) are auto-skipped.
+ /// - `Some(names)` — read exactly the named arrays. Unknown names
+ /// error. 1-D arrays are always rejected (a raster band needs
+ /// ≥ 2 dimensions); naming one explicitly errors with a clear
+ /// message.
+ ///
+ /// `batch_size` controls how many chunk rows are emitted per
+ /// `RecordBatch`. Must be ≥ 1; callers typically pass
+ /// `SessionConfig::batch_size` (defaults to 8192).
+ pub fn try_new(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+ batch_size: usize,
+ ) -> Result<Self, ArrowError> {
+ let batch_size = batch_size.max(1);
+ let OpenedGroup {
+ array_infos,
+ geo,
+ group_transform,
+ spatial_dim_indices,
+ } = open_and_validate(group_uri, arrays)?;
+
+ let spatial_dims_names: Vec<String> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].dim_names[i].clone())
+ .collect();
+ let chunk_spatial_shape: Vec<i64> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].chunk_shape[i] as i64)
+ .collect();
+
+ let raster_field = SedonaType::Raster
+ .to_storage_field("raster", true)
+ .map_err(|e| ArrowError::ExternalError(Box::new(e)))?;
+ let schema = Arc::new(Schema::new(vec![raster_field]));
+
+ let chunk_indices = vec![0u64; array_infos[0].chunk_grid_shape.len()];
+
+ Ok(Self {
+ schema,
+ group_uri: group_uri.to_string(),
+ array_infos,
+ geo,
+ group_transform,
+ spatial_dim_indices,
+ spatial_dims_names,
+ chunk_spatial_shape,
+ chunk_indices,
+ exhausted: false,
+ batch_size,
+ })
+ }
+
+ /// Append one raster row at the current `chunk_indices` to the
+ /// in-progress builder. Does not advance the cursor.
+ fn emit_one_row(&self, builder: &mut RasterBuilder) -> Result<(), ArrowError> {
+ let row_transform = compute_row_transform(
+ &self.group_transform,
+ &self.chunk_indices,
+ &self.array_infos[0].chunk_shape,
+ &self.spatial_dim_indices,
+ );
+ let spatial_dims_ref: Vec<&str> =
+ self.spatial_dims_names.iter().map(String::as_str).collect();
+ let crs_str = self.geo.crs.as_deref();
+ builder.start_raster_nd(
+ &row_transform,
+ &spatial_dims_ref,
+ &self.chunk_spatial_shape,
+ crs_str,
+ )?;
+
+ for info in &self.array_infos {
+ let dim_names_ref: Vec<&str> = info.dim_names.iter().map(String::as_str).collect();
+ let nodata_ref = info.nodata.as_deref();
+ // Every band gets its chunk-anchor URI populated as
+ // provenance metadata. `data.is_empty()` is the InDb/OutDb
+ // discriminator; this reader always emits empty `data` and
+ // defers pixel-byte resolution to the OutDb resolver.
+ let anchor = build_chunk_anchor(&self.group_uri, &info.path, &self.chunk_indices);
+ builder.start_band_nd(
+ Some(info.path.as_str()),
+ &dim_names_ref,
+ &info.chunk_shape,
+ info.data_type,
+ nodata_ref,
+ Some(anchor.as_str()),
+ Some("zarr"),
+ )?;
+ builder.band_data_writer().append_value([0u8; 0]);
+ builder.finish_band()?;
+ }
+ builder.finish_raster()
+ }
+}
+
+impl Iterator for ZarrChunkReader {
+ type Item = Result<RecordBatch, ArrowError>;
+
+ fn next(&mut self) -> Option<Self::Item> {
+ if self.exhausted {
+ return None;
+ }
+ let mut builder = RasterBuilder::new(self.batch_size);
+ let mut rows_emitted = 0usize;
+ while rows_emitted < self.batch_size {
+ if let Err(e) = self.emit_one_row(&mut builder) {
+ self.exhausted = true;
+ return Some(Err(e));
+ }
+ rows_emitted += 1;
+ if !advance_chunk_indices(
+ &mut self.chunk_indices,
+ &self.array_infos[0].chunk_grid_shape,
+ ) {
+ self.exhausted = true;
+ break;
+ }
+ }
+ if rows_emitted == 0 {
+ return None;
+ }
+ let struct_arr = match builder.finish() {
+ Ok(arr) => arr,
+ Err(e) => return Some(Err(e)),
+ };
+ Some(RecordBatch::try_new(
+ self.schema.clone(),
+ vec![Arc::new(struct_arr)],
+ ))
+ }
+}
+
+impl RecordBatchReader for ZarrChunkReader {
+ fn schema(&self) -> SchemaRef {
+ self.schema.clone()
+ }
+}
+
+/// Per-array metadata extracted once at group open and reused for every
+/// chunk position. Caching this avoids re-reading Zarr metadata for each
+/// of the (potentially thousands of) chunk rows.
+struct ArrayInfo {
+ /// Array path within the store, used to build chunk anchor URIs and
+ /// surface in band names.
+ path: String,
+ /// SedonaDB BandDataType corresponding to this array's zarrs dtype.
+ data_type: BandDataType,
+ /// Dimension names in array order. Required to be `Some(_)` for every
+ /// dim; missing names error at validation time.
+ dim_names: Vec<String>,
+ /// Inner chunk grid shape, one entry per dimension. Used to enumerate
+ /// chunk positions and validated to match across arrays.
+ chunk_grid_shape: Vec<u64>,
+ /// Chunk shape (elements per chunk per dim). Same for every chunk
+ /// position — ragged final chunks are not emitted as separate short
+ /// rows.
+ chunk_shape: Vec<u64>,
+ /// Encoded fill value in native-endian byte representation, for the
+ /// `nodata` field. None when the array has no fill value declared.
+ nodata: Option<Vec<u8>>,
+}
+
+/// Bundle of group-level state returned by [`open_and_validate`].
+struct OpenedGroup {
+ array_infos: Vec<ArrayInfo>,
+ geo: GroupGeoMetadata,
+ group_transform: [f64; 6],
+ spatial_dim_indices: Vec<usize>,
+}
+
+/// Open the Zarr group, parse and validate group metadata, and return
+/// everything `ZarrChunkReader` needs to iterate without further I/O
+/// (apart from per-chunk byte fetches by future resolvers).
+fn open_and_validate(
+ group_uri: &str,
+ arrays_filter: Option<&[String]>,
+) -> Result<OpenedGroup, ArrowError> {
+ let fs_path = group_uri_to_filesystem_path(group_uri)?;
+ let store = FilesystemStore::new(&fs_path).map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr filesystem store at {}: {e}",
+ fs_path.display()
+ )))
+ })?;
+ let storage: Arc<FilesystemStore> = Arc::new(store);
+
+ let group = Group::open(storage.clone(), "/").map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr group at {group_uri}: {e}"
+ )))
+ })?;
+
+ let geo = GroupGeoMetadata::from_attributes(group.attributes())?;
+
+ // CRS-without-transform is almost always a malformed-metadata bug —
+ // the user thinks they have full georef but downstream spatial joins
+ // will silently use the identity-pixel-coords default. Error loudly
+ // so they fix the metadata rather than getting empty result sets.
+ if geo.crs.is_some() && geo.transform.is_none() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} declares a CRS but no `spatial:transform` \
+ attribute; refusing to fall back to the identity transform because \
+ that would silently produce wrong results in spatial joins. Declare \
+ `spatial:transform` on the group or remove the CRS to read this as \
+ a non-georeferenced datacube."
+ )));
+ }
+
+ let arrays = group.child_arrays().map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to enumerate child arrays in {group_uri}: {e}"
+ )))
+ })?;
+ if arrays.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} has no child arrays"
+ )));
+ }
+
+ let arrays = select_arrays(arrays, arrays_filter, group_uri)?;
+ let array_infos = collect_array_infos(arrays)?;
+ validate_group_constraints(&array_infos)?;
+
+ // Spatial-dim resolution. Two configurations are accepted:
+ // - dim_names ends with ["y", "x"] (canonical for georeferenced
+ // 2-D and time-series rasters); the spatial extent is the chunk's
+ // last two dims.
+ // - `spatial:dims` attribute on the group explicitly names them.
+ // Anything else errors with a clear message — silently picking dims
+ // would produce wrong per-row transforms.
+ let spatial_dim_indices =
+ resolve_spatial_dim_indices(&array_infos[0].dim_names, geo.spatial_dims.as_deref())?;
+
+ let group_transform = match geo.transform {
+ Some(t) => t,
+ None => {
+ // Both `spatial:transform` and `proj:*` are absent (the
+ // CRS-only case errored above). Fall back to identity pixel
+ // coordinates and breadcrumb a warning so spatial-join
+ // surprises are debuggable.
+ log::warn!(
+ "Zarr group at {group_uri} has no `spatial:transform`; falling back \
+ to the identity pixel-coordinate transform [0, 1, 0, 0, 0, -1]. \
+ Spatial operations against this raster will use pixel coordinates."
+ );
+ [0.0, 1.0, 0.0, 0.0, 0.0, -1.0]
+ }
+ };
+
+ // chunk_grid_shape comes from untrusted Zarr metadata; bound-check the
+ // product so a hostile or malformed grid can't make per-batch
+ // RasterBuilder capacity (or downstream consumers) overflow.
+ array_infos[0]
+ .chunk_grid_shape
+ .iter()
+ .try_fold(1usize, |acc, &n| {
+ usize::try_from(n).ok().and_then(|n| acc.checked_mul(n))
+ })
+ .ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk grid shape {:?} overflows usize",
+ array_infos[0].chunk_grid_shape
+ ))
+ })?;
+
+ Ok(OpenedGroup {
+ array_infos,
+ geo,
+ group_transform,
+ spatial_dim_indices,
+ })
+}
+
+/// Collect per-array metadata from open zarrs `Array` handles.
+///
+/// Sorts arrays by path so band ordering across rows is deterministic
+/// (zarrs's underlying store listing order is implementation-defined —
+/// filesystem stores currently happen to enumerate alphabetically, but
+/// that's not part of the contract we want consumers to rely on).
+fn collect_array_infos(
+ mut arrays: Vec<Array<FilesystemStore>>,
+) -> Result<Vec<ArrayInfo>, ArrowError> {
+ arrays.sort_by(|a, b| a.path().as_str().cmp(b.path().as_str()));
+ let mut out = Vec::with_capacity(arrays.len());
+ for array in arrays {
+ let path = array.path().to_string();
+ let data_type = zarr_to_band_data_type(array.data_type())?;
+ let dim_names = resolve_dim_names(&array, &path)?;
+ let chunk_grid_shape = array.chunk_grid_shape().to_vec();
+ let chunk_shape = array
+ .chunk_shape(&vec![0u64; chunk_grid_shape.len()])
+ .map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "array {path}: failed to query chunk shape: {e}"
+ )))
+ })?
+ .iter()
+ .map(|n| n.get())
+ .collect();
+ let fill_bytes = array.fill_value().as_ne_bytes();
+ let nodata = if fill_bytes.is_empty() {
+ None
+ } else {
+ Some(fill_bytes.to_vec())
+ };
+ out.push(ArrayInfo {
+ path,
+ data_type,
+ dim_names,
+ chunk_grid_shape,
+ chunk_shape,
+ nodata,
+ });
+ }
+ Ok(out)
+}
+
+/// Apply the array-selection rules. 1-D arrays (typical xarray-style
+/// coord variables) are always dropped — a raster band requires at
+/// least 2 dimensions, so reading a 1-D array could never succeed.
+/// - Explicit filter: keep arrays whose path (with leading `/` stripped)
+/// matches one of the requested names. Unknown names error so users
+/// don't silently get an empty group from a typo; naming a 1-D array
+/// errors with a clear message rather than producing a confusing
+/// "no spatial axes" failure downstream.
+/// - No filter: read every multi-dimensional array.
+fn select_arrays(
+ arrays: Vec<Array<FilesystemStore>>,
+ filter: Option<&[String]>,
+ group_uri: &str,
+) -> Result<Vec<Array<FilesystemStore>>, ArrowError> {
+ if let Some(names) = filter {
+ let available: Vec<String> = arrays
+ .iter()
+ .map(|a| a.path().as_str().trim_start_matches('/').to_string())
+ .collect();
+ for requested in names {
+ let needle = requested.trim_start_matches('/');
+ match arrays
+ .iter()
+ .find(|a| a.path().as_str().trim_start_matches('/') == needle)
+ {
+ None => {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} has no array named {requested:?}; \
+ available arrays: {available:?}"
+ )));
+ }
+ Some(a) if a.shape().len() < 2 => {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "array {requested:?} has rank {} (shape {:?}); a raster band \
+ requires at least 2 dimensions and cannot be read.",
+ a.shape().len(),
+ a.shape()
+ )));
+ }
+ Some(_) => {}
+ }
+ }
+ let kept: Vec<_> = arrays
+ .into_iter()
+ .filter(|a| {
+ let path = a.path().as_str().trim_start_matches('/').to_string();
+ names.iter().any(|n| n.trim_start_matches('/') == path)
+ })
+ .collect();
+ return Ok(kept);
+ }
+
+ let kept: Vec<_> = arrays.into_iter().filter(|a| a.shape().len() > 1).collect();
+ if kept.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} contains only 1-D arrays (typical \
+ xarray-style coord variables); a raster band requires at least \
+ 2 dimensions, so this group has nothing readable as a raster"
+ )));
+ }
+ Ok(kept)
+}
+
+/// Resolve dimension names for an array, supporting both Zarr v3
+/// (first-class `dimension_names` field) and Zarr v2 with the xarray
+/// `_ARRAY_DIMENSIONS` attribute. Errors if neither carries a complete
+/// set of named dimensions matching the array's rank.
+fn resolve_dim_names<S: ?Sized>(array: &Array<S>, path: &str) -> Result<Vec<String>, ArrowError> {
+ let rank = array.shape().len();
+
+ if let Some(names) = array.dimension_names() {
+ return names
+ .iter()
+ .enumerate()
+ .map(|(i, n)| {
+ n.clone().ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "array {path}: dimension {i} has no name; every Zarr array \
+ dimension must be named",
+ ))
+ })
+ })
+ .collect();
+ }
+
+ // Zarr v2 fallback: xarray's `_ARRAY_DIMENSIONS` convention. v2 had no
+ // first-class dimension_names field and zarrs's v2->v3 converter
+ // preserves attributes verbatim without lifting this one.
+ if let Some(value) = array.attributes().get("_ARRAY_DIMENSIONS") {
+ let arr = value.as_array().ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "array {path}: _ARRAY_DIMENSIONS must be a JSON array of strings; got {value}"
+ ))
+ })?;
+ if arr.len() != rank {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "array {path}: _ARRAY_DIMENSIONS has {} entries but array has rank {rank}",
+ arr.len()
+ )));
+ }
+ return arr
+ .iter()
+ .enumerate()
+ .map(|(i, v)| {
+ v.as_str().map(str::to_string).ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "array {path}: _ARRAY_DIMENSIONS[{i}] must be a string; got {v}"
+ ))
+ })
+ })
+ .collect();
+ }
+
+ Err(ArrowError::InvalidArgumentError(format!(
+ "array {path}: no dimension names found. Zarr v3 arrays must set \
+ `dimension_names`; Zarr v2 arrays must set the `_ARRAY_DIMENSIONS` \
+ attribute (xarray convention)."
+ )))
+}
+
+/// Enforce group constraints. All arrays must agree on chunk grid shape,
+/// chunk shape, and dimension names. We do NOT enforce shared element
+/// shape (`array.shape()`) because users routinely group arrays with
+/// the same chunk grid but different totals (e.g. a coord variable with
+/// one fewer dim is rejected here anyway by the dim-name check).
+fn validate_group_constraints(infos: &[ArrayInfo]) -> Result<(), ArrowError> {
+ let first = &infos[0];
+ for other in &infos[1..] {
+ if other.chunk_grid_shape != first.chunk_grid_shape {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "arrays {} and {} have different chunk grid shapes ({:?} vs {:?}); \
+ every array in the group must share the same chunk grid. \
+ Pass `arrays = [...]` to read only compatible arrays.",
+ first.path, other.path, first.chunk_grid_shape, other.chunk_grid_shape
+ )));
+ }
+ if other.chunk_shape != first.chunk_shape {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "arrays {} and {} have different chunk shapes ({:?} vs {:?}); \
+ every array in the group must share the same chunk shape. \
+ Pass `arrays = [...]` to read only compatible arrays.",
+ first.path, other.path, first.chunk_shape, other.chunk_shape
+ )));
+ }
+ if other.dim_names != first.dim_names {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "arrays {} and {} have different dimension names ({:?} vs {:?}); \
+ every array in the group must declare identical dim_names. \
+ Pass `arrays = [...]` to read only compatible arrays.",
+ first.path, other.path, first.dim_names, other.dim_names
+ )));
+ }
+ }
+ Ok(())
+}
+
+/// Pick the `(y_index, x_index)` axes of an array's dim_names.
+///
+/// If `spatial_dims` is provided via the group's `spatial:dims` attribute,
+/// look up those names by position. Otherwise, default to the last two
+/// dims and require they be named `y` and `x` (in that order) — the
+/// canonical GeoZarr-2D convention. Anything else errors.
+fn resolve_spatial_dim_indices(
+ dim_names: &[String],
+ spatial_dims: Option<&[String]>,
+) -> Result<Vec<usize>, ArrowError> {
+ if let Some(spatial) = spatial_dims {
+ let mut idx = Vec::with_capacity(spatial.len());
+ for s in spatial {
+ let i = dim_names.iter().position(|n| n == s).ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "spatial:dims declared name {s:?} not found in array dim_names {dim_names:?}",
+ ))
+ })?;
+ idx.push(i);
+ }
+ return Ok(idx);
+ }
+ let n = dim_names.len();
+ if n < 2 {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "at least 2 dimensions are required to resolve spatial axes; got {dim_names:?}",
+ )));
+ }
+ if dim_names[n - 2] != "y" || dim_names[n - 1] != "x" {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "the last two dim_names must be [\"y\", \"x\"] when `spatial:dims` is \
+ not declared; got {dim_names:?}",
+ )));
+ }
+ Ok(vec![n - 2, n - 1])
+}
+
+/// Per-chunk transform: translate the group's transform so the chunk's
+/// `[0, 0]` element maps to the chunk's spatial origin.
+fn compute_row_transform(
+ group_transform: &[f64; 6],
+ chunk_indices: &[u64],
+ chunk_shape: &[u64],
+ spatial_dim_indices: &[usize],
+) -> [f64; 6] {
+ // GDAL GeoTransform layout: [origin_x, scale_x, skew_x, origin_y, skew_y, scale_y].
+ // Translation along x = chunk_x_index × chunk_x_size in pixel-coordinate space,
+ // converted to world coordinates via the affine.
+ //
+ // spatial_dim_indices is validated upstream to be [y_index, x_index].
+ // Index 0 is the y axis, index 1 is the x axis.
+ let y_axis = spatial_dim_indices[0];
+ let x_axis = spatial_dim_indices[1];
+ let x_offset = (chunk_indices[x_axis] * chunk_shape[x_axis]) as f64;
+ let y_offset = (chunk_indices[y_axis] * chunk_shape[y_axis]) as f64;
+ let [ox, sx, kx, oy, ky, sy] = *group_transform;
+ [
+ ox + sx * x_offset + kx * y_offset,
+ sx,
+ kx,
+ oy + ky * x_offset + sy * y_offset,
+ ky,
+ sy,
+ ]
+}
+
+/// Advance `chunk_indices` row-major over `chunk_grid_shape`. Returns
+/// `true` while there are positions left, `false` when the grid is
+/// exhausted (and the indices wrap back to all-zero).
+fn advance_chunk_indices(chunk_indices: &mut [u64], chunk_grid_shape: &[u64]) -> bool {
+ for i in (0..chunk_indices.len()).rev() {
+ chunk_indices[i] += 1;
+ if chunk_indices[i] < chunk_grid_shape[i] {
+ return true;
+ }
+ chunk_indices[i] = 0;
+ }
+ false
+}
+
+/// Retrieve a single chunk's bytes as a fresh `Vec<u8>`.
+///
+/// Uses `ArrayBytes::Fixed`, so this errors for variable-length element
+/// types — those don't have a `BandDataType` counterpart anyway, so the
+/// dtype check in `collect_array_infos` rejects them upstream.
+///
+/// This is the only pixel-byte read primitive in the crate. The loader
+/// itself never calls it today — it always emits OutDb anchors — but
+/// the async `RS_EnsureLoaded` resolver (follow-up PR) will. Kept here
+/// rather than dropped because (a) it's tested directly below and (b)
+/// the resolver lives in this crate and wants a sync chunk-fetch
+/// helper as a starting point.
+#[allow(dead_code)]
Review Comment:
Instead of allowing dead code here you can do `#[cfg(test)]` (if this is tested) or copy/paste into a follow-up issue (if it isn't)
##########
python/sedonadb/tests/test_no_zarr_by_default.py:
##########
@@ -0,0 +1,43 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Architectural regression test: `sedonadb` alone has no Zarr.
+
+This locks in the plugin separation. A fresh sedonadb connection,
+created without importing `sedonadb_zarr`, must not know about the
+`sd_read_zarr` UDTF. If a future change accidentally re-bundles Zarr
+into the main package — for example by adding a convenience import or
+re-attaching the registration to `SedonaContext::new_from_context` —
+this test fails and forces a conscious decision.
Review Comment:
I appreciate the comprehensiveness but you can probably remove this test
##########
python/sedonadb/src/context.rs:
##########
@@ -200,4 +200,60 @@ impl InternalContext {
.to_string(),
))
}
+
+ /// Register a UDTF (table function) whose implementation is passed
+ /// through a `PyCapsule` containing an [`UdtfCapsule`].
+ ///
+ /// Plugin-handoff API: format-specific Python packages (e.g.
+ /// `sedonadb-zarr`) build their UDTF in their own PyO3 extension
+ /// and pass it across as an opaque capsule, sidestepping the
+ /// cross-extension `#[pyclass]` type-id mismatch.
+ ///
+ /// Two checks gate the dereference: the capsule's name must equal
+ /// [`UDTF_CAPSULE_NAME`] (a public string, so this is mostly an
+ /// ABI tag), and the payload's leading [`UDTF_CAPSULE_MAGIC`]
+ /// sentinel must match (much harder to forge from outside this
+ /// crate). The consumer clones the inner `Arc` to take its own
+ /// refcount; the capsule keeps a parallel refcount until Python
+ /// GC drops it.
+ ///
+ /// [`UdtfCapsule`]: crate::plugin::UdtfCapsule
+ /// [`UDTF_CAPSULE_NAME`]: crate::plugin::UDTF_CAPSULE_NAME
+ /// [`UDTF_CAPSULE_MAGIC`]: crate::plugin::UDTF_CAPSULE_MAGIC
+ pub fn register_udtf_capsule(
+ &self,
+ name: &str,
+ capsule: &Bound<'_, pyo3::types::PyCapsule>,
+ ) -> Result<(), PySedonaError> {
+ use crate::plugin::{UdtfCapsule, UDTF_CAPSULE_MAGIC, UDTF_CAPSULE_NAME};
+
+ let actual = capsule
Review Comment:
For this side you can use the DataFusion Python pattern:
https://github.com/apache/datafusion-python/blob/dac9ec6230dba8717d8a0d27de19a141600486b1/crates/core/src/udtf.rs#L50-L91
##########
python/sedonadb/src/datasource.rs:
##########
@@ -143,7 +153,28 @@ impl PyExternalFormat {
#[new]
fn new<'py>(py: Python<'py>, py_spec: PyObject) -> Result<Self, PySedonaError> {
let extension = py_spec.getattr(py, "extension")?.extract::<String>(py)?;
- Ok(Self { extension, py_spec })
+ let list_single_object = read_list_single_object(py, &py_spec)?;
+ Ok(Self {
+ extension,
+ list_single_object,
+ py_spec,
+ })
+ }
+}
+
+/// Read the `list_single_object` attribute on a Python spec, defaulting
+/// to `false` if the attribute is missing (older spec implementations
+/// that predate the directory-format path).
+fn read_list_single_object<'py>(
Review Comment:
You can also just update the pyogrio one (it's the only one so far)
##########
rust/sedona-raster-zarr/src/dtype.rs:
##########
@@ -0,0 +1,66 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Mapping between Zarr datatypes and SedonaDB's `BandDataType`.
+//!
+//! zarrs 0.23 models `DataType` as a wrapper around `Arc<dyn DataTypeTraits>`,
+//! so we discriminate via the type-erased `is::<T>()` check rather than
+//! pattern-matching an enum.
+
+use arrow_schema::ArrowError;
+use sedona_schema::raster::BandDataType;
+use zarrs::array::data_type::{
+ BoolDataType, Float32DataType, Float64DataType, Int16DataType, Int32DataType, Int64DataType,
+ Int8DataType, UInt16DataType, UInt32DataType, UInt64DataType, UInt8DataType,
+};
+use zarrs::array::DataType as ZarrDataType;
+
+/// Map a Zarr `DataType` to a SedonaDB `BandDataType`.
+///
+/// Bool maps to UInt8 losslessly (Zarr packs bools to one byte each, matching
+/// our representation). Variable-length, complex, and extended-precision
+/// types error with `NotYetImplemented` — they have no numeric counterpart
+/// in `BandDataType` today.
+pub fn zarr_to_band_data_type(dt: &ZarrDataType) -> Result<BandDataType, ArrowError> {
+ if dt.is::<BoolDataType>() {
+ Ok(BandDataType::UInt8)
Review Comment:
Not a blocker, but you can parameterize your smoke test with numpy dytpes to sanity check these
##########
rust/sedona-raster-zarr/src/format_spec.rs:
##########
@@ -0,0 +1,135 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! `ExternalFormatSpec` impl for Zarr groups.
+//!
+//! Wraps [`ZarrChunkReader`] with the standard SedonaDB datasource API
+//! so users can read Zarr groups via `con.read_format(spec, uri)` or
+//! through `ListingTable`-style file discovery.
+
+use std::{collections::HashMap, sync::Arc};
+
+use arrow_array::RecordBatchReader;
+use arrow_schema::Schema;
+use async_trait::async_trait;
+use datafusion_common::{DataFusionError, Result};
+use sedona_datasource::spec::{ExternalFormatSpec, Object, OpenReaderArgs};
+use sedona_schema::datatypes::SedonaType;
+
+use crate::loader::ZarrChunkReader;
+
+/// `ExternalFormatSpec` implementation for Zarr groups.
+///
+/// Configurable via [`with_options`](ExternalFormatSpec::with_options):
+/// - `load_eager`: boolean. `false` (default) emits chunk-anchor URIs
+/// only. `true` currently errors — pixel-byte materialisation is
+/// pending the async `RS_EnsureLoaded` resolver.
+/// - `arrays`: JSON array of strings, e.g. `'["temperature","pressure"]'`.
+/// Names a subset of arrays in the group to read; defaults to every
+/// multi-dimensional array (1-D coord variables auto-skipped).
+#[derive(Debug, Clone, Default)]
+pub struct ZarrFormatSpec {
+ load_eager: bool,
+ arrays: Option<Vec<String>>,
+}
+
+impl ZarrFormatSpec {
+ pub fn new() -> Self {
+ Self::default()
+ }
+}
+
+#[async_trait]
+impl ExternalFormatSpec for ZarrFormatSpec {
+ async fn infer_schema(&self, _location: &Object) -> Result<Schema> {
+ // The Zarr loader always produces the canonical single-column
+ // Raster schema. No I/O needed to know that.
+ let field = SedonaType::Raster
+ .to_storage_field("raster", true)
+ .map_err(|e| DataFusionError::External(Box::new(e)))?;
+ Ok(Schema::new(vec![field]))
+ }
+
+ async fn open_reader(
+ &self,
+ args: &OpenReaderArgs,
+ ) -> Result<Box<dyn RecordBatchReader + Send>> {
+ if self.load_eager {
+ return Err(DataFusionError::Plan(
+ "ZarrFormatSpec: load_eager = true is not yet supported. \
+ Pixel-byte materialisation will be wired up when the async \
+ RS_EnsureLoaded resolver lands."
+ .into(),
+ ));
+ }
Review Comment:
Probably just leave out this option for now? Depending on what we do for ensuring loading, we may never pass this into the scan
##########
rust/sedona-raster-zarr/Cargo.toml:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+[package]
+name = "sedona-raster-zarr"
+version.workspace = true
+license.workspace = true
+keywords.workspace = true
+categories.workspace = true
+homepage.workspace = true
+repository.workspace = true
+description.workspace = true
+readme.workspace = true
+edition.workspace = true
+rust-version.workspace = true
+
+[lints.clippy]
+result_large_err = "allow"
+
+[dependencies]
+arrow-array = { workspace = true }
+arrow-schema = { workspace = true }
+async-trait = { workspace = true }
+datafusion = { workspace = true, features = ["sql"] }
+datafusion-catalog = { workspace = true }
+datafusion-common = { workspace = true }
+datafusion-expr = { workspace = true }
+futures = { workspace = true }
+log = { workspace = true }
+sedona-common = { workspace = true }
+sedona-datasource = { workspace = true }
+sedona-raster = { workspace = true }
+sedona-schema = { workspace = true }
+serde = { workspace = true }
+serde_json = { workspace = true }
+zarrs = { workspace = true, features = ["filesystem", "gzip", "zstd", "crc32c", "sharding", "transpose"] }
+zarrs_filesystem = { workspace = true }
+
+# `blosc` is gated off the Windows MinGW (gnu) toolchain only: c-blosc
+# (statically linked) bundles its own `pthread_create` / `pthread_cond_*`
+# symbols, which conflict with rtools45's `libpthread.a` during the MinGW
+# link of the R `sedonadb.dll`. The MSVC toolchain (Python wheels, etc.)
+# uses a different pthread implementation and is fine.
+[target.'cfg(not(all(target_os = "windows", target_env = "gnu")))'.dependencies]
+zarrs = { workspace = true, features = ["blosc"] }
Review Comment:
Do you still need this since R is not participating in the plugin system here?
##########
rust/sedona-raster-zarr/src/udtf.rs:
##########
@@ -0,0 +1,614 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! `sd_read_zarr` table function — exposes Zarr-backed raster groups
+//! as SQL-queryable tables.
+//!
+//! ```sql
+//! SELECT * FROM sd_read_zarr('file:///path/to/datacube.zarr');
+//! SELECT count(*) FROM sd_read_zarr(
+//! 'file:///path/to/datacube.zarr',
+//! '{"rows_per_batch": 256}'
+//! );
+//! ```
+//!
+//! Returns a single-column table `raster: Raster` with one row per chunk
+//! position in the Zarr group's chunk grid. All existing `RS_*` UDFs
+//! operate on the column unchanged.
+//!
+//! `load_eager` defaults to `false` — every row has empty `data` and a
+//! chunk-anchor URI in `outdb_uri`. Setting `load_eager = true`
+//! currently errors because no `RS_EnsureLoaded` resolver is registered
+//! for the `zarr` format yet; once the resolver lands, `true` will
+//! trigger the planner to inject the async UDF over the scan output.
+//!
+//! Registration happens via [`register`]; the `sedonadb-zarr` Python
+//! package calls this from its session-setup helper. The `sedona` crate
+//! itself does not register the UDTF — keeping zarr functionality out
+//! of the default bootstrap.
+
+use std::any::Any;
+use std::sync::Arc;
+
+use arrow_schema::{DataType, Schema, SchemaRef};
+use async_trait::async_trait;
+use datafusion::catalog::{Session, TableFunctionImpl, TableProvider};
+use datafusion::common::Result;
+use datafusion::datasource::TableType;
+use datafusion::execution::context::{SessionContext, TaskContext};
+use datafusion::physical_expr::EquivalenceProperties;
+use datafusion::physical_plan::execution_plan::{Boundedness, EmissionType};
+use datafusion::physical_plan::expressions::Column;
+use datafusion::physical_plan::projection::ProjectionExec;
+use datafusion::physical_plan::stream::RecordBatchStreamAdapter;
+use datafusion::physical_plan::{
+ DisplayAs, DisplayFormatType, ExecutionPlan, Partitioning, PhysicalExpr, PlanProperties,
+ SendableRecordBatchStream,
+};
+use datafusion::prelude::Expr;
+use datafusion_common::{plan_err, DataFusionError, ScalarValue};
+use sedona_schema::datatypes::SedonaType;
+use serde::{Deserialize, Serialize};
+
+use crate::loader::ZarrChunkReader;
+
+/// Register the `sd_read_zarr` UDTF on a `SessionContext`. Called from
+/// the `sedonadb-zarr` Python package's `register(con)` entry point.
+pub fn register(ctx: &SessionContext) {
+ ctx.register_udtf("sd_read_zarr", Arc::new(ZarrReadFunction::default()));
+}
Review Comment:
You probably want to avoid this line because the `SessionContext` is a heavy object that forces a heavy dependency (`datafusion`). Otherwise you can get away with the individual sub-crates as dependencies (compiles much faster).
##########
rust/sedona-raster-zarr/Cargo.toml:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+[package]
+name = "sedona-raster-zarr"
+version.workspace = true
+license.workspace = true
+keywords.workspace = true
+categories.workspace = true
+homepage.workspace = true
+repository.workspace = true
+description.workspace = true
+readme.workspace = true
+edition.workspace = true
+rust-version.workspace = true
+
+[lints.clippy]
+result_large_err = "allow"
+
+[dependencies]
+arrow-array = { workspace = true }
+arrow-schema = { workspace = true }
+async-trait = { workspace = true }
+datafusion = { workspace = true, features = ["sql"] }
Review Comment:
If you can avoid this one in `dependencies` (dev-dependencies is ok), the crate will compile faster and be smaller.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on PR #858:
URL: https://github.com/apache/sedona-db/pull/858#issuecomment-4522103346
@paleolimbot now I am happy with it review/approve/merge at your discretion.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3277660949
##########
rust/sedona/src/zarr_read.rs:
##########
@@ -0,0 +1,553 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! `sd_read_zarr` table function — exposes Zarr-backed raster groups
+//! as SQL-queryable tables.
+//!
+//! ```sql
+//! SELECT * FROM sd_read_zarr('file:///path/to/datacube.zarr');
+//! SELECT count(*) FROM sd_read_zarr(
+//! 'file:///path/to/datacube.zarr',
+//! '{"indb": true, "rows_per_batch": 256}'
+//! );
+//! ```
+//!
+//! Returns a single-column table `raster: Raster` with one row per chunk
+//! position in the Zarr group's chunk grid. All existing `RS_*` UDFs
+//! operate on the column unchanged.
+//!
+//! `indb` defaults to `true` so byte-reading kernels work end-to-end
+//! without depending on a registered OutDb resolver. The default will
+//! flip to `false` once the format-keyed dispatcher lands.
+
+use std::any::Any;
+use std::sync::Arc;
+
+use arrow_array::{Array, RecordBatch, StructArray};
+use arrow_schema::{DataType, Schema, SchemaRef};
+use async_trait::async_trait;
+use datafusion::catalog::{Session, TableFunctionImpl, TableProvider};
+use datafusion::common::Result;
+use datafusion::datasource::TableType;
+use datafusion::execution::context::TaskContext;
+use datafusion::physical_expr::EquivalenceProperties;
+use datafusion::physical_plan::execution_plan::{Boundedness, EmissionType};
+use datafusion::physical_plan::expressions::Column;
+use datafusion::physical_plan::projection::ProjectionExec;
+use datafusion::physical_plan::stream::RecordBatchStreamAdapter;
+use datafusion::physical_plan::{
+ DisplayAs, DisplayFormatType, ExecutionPlan, Partitioning, PhysicalExpr, PlanProperties,
+ SendableRecordBatchStream,
+};
+use datafusion::prelude::Expr;
+use datafusion_common::{plan_err, DataFusionError, ScalarValue};
+use sedona_raster_zarr::{group_to_indb_rasters, group_to_outdb_rasters};
+use sedona_schema::datatypes::SedonaType;
+use serde::{Deserialize, Serialize};
+
+/// Table function `sd_read_zarr(uri[, options_json])`.
+///
+/// Accepts one or two string arguments:
+/// - `uri` (required) — Zarr group URI (e.g. `file:///path/to/foo.zarr`).
+/// - `options_json` (optional) — JSON string with any of:
+/// - `indb`: `true` (default) materializes chunk bytes into the
+/// Arrow `data` column eagerly; `false` emits chunk-anchor URIs
+/// only and defers byte resolution to the OutDb loader.
+/// - `rows_per_batch`: chunks per `RecordBatch` (default 1024)
+/// - `num_partitions`: scan partitions (default 1; > 1 currently errors)
+/// - `arrays`: optional list of array names to read. Default reads
+/// every multi-dimensional array (1-D coord variables are
+/// auto-skipped); an explicit list reads exactly those arrays.
+#[derive(Debug, Default)]
+pub struct ZarrReadFunction {}
Review Comment:
ok I implemented against the ExternalFormatSpec
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3283706068
##########
python/sedonadb/tests/test_no_zarr_by_default.py:
##########
@@ -0,0 +1,43 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Architectural regression test: `sedonadb` alone has no Zarr.
+
+This locks in the plugin separation. A fresh sedonadb connection,
+created without importing `sedonadb_zarr`, must not know about the
+`sd_read_zarr` UDTF. If a future change accidentally re-bundles Zarr
+into the main package — for example by adding a convenience import or
+re-attaching the registration to `SedonaContext::new_from_context` —
+this test fails and forces a conscious decision.
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3276992353
##########
rust/sedona-raster-zarr/src/loader.rs:
##########
@@ -0,0 +1,674 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Zarr group → N-D raster `StructArray` entry points.
+//!
+//! Both `group_to_indb_rasters` and `group_to_outdb_rasters` produce the
+//! same row shape: one raster row per chunk position, with one band per
+//! array in the group. They differ only in how each row's pixel bytes
+//! are delivered:
+//!
+//! - **InDb** — every chunk is fetched eagerly and copied into the
+//! Arrow `data` column. Heavy for large datacubes; intended for
+//! snapshots.
+//! - **OutDb** — `data` is left empty; each band's `outdb_uri` carries a
+//! chunk anchor (`zarr://<store-uri>/<array-path>#chunk=i0,i1,...`).
+//! Byte resolution awaits the format-keyed dispatch work in a
+//! follow-up PR.
+
+use std::sync::Arc;
+
+use arrow_array::StructArray;
+use arrow_schema::ArrowError;
+use sedona_common::sedona_internal_datafusion_err;
+use sedona_raster::builder::RasterBuilder;
+use sedona_schema::raster::BandDataType;
+use zarrs::array::{Array, ArrayBytes};
+use zarrs::group::Group;
+use zarrs_filesystem::FilesystemStore;
+
+use crate::dtype::zarr_to_band_data_type;
+use crate::geozarr::GroupGeoMetadata;
+use crate::source_uri::{build_chunk_anchor, group_uri_to_filesystem_path};
+
+/// Open a Zarr group and eagerly fetch every chunk's bytes into the
+/// returned `StructArray`. Each row holds one chunk position's data
+/// across the selected arrays in the group.
+///
+/// `arrays`:
+/// - `None` — read every multi-dimensional array. 1-D arrays (typical
+/// coord variables in xarray-style datacubes) are auto-skipped.
+/// - `Some(names)` — read exactly the named arrays, in the order
+/// returned by the store's listing (which is then sorted by path). An
+/// unknown name errors. 1-D arrays are always rejected (a raster band
+/// needs ≥ 2 dimensions); naming one explicitly errors at parse time
+/// with a clear message.
+pub fn group_to_indb_rasters(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::InDb, arrays)
+}
+
+/// Open a Zarr group and emit one row per chunk position with chunk-anchor
+/// URIs in each band's `outdb_uri`. The `data` column is empty; bytes
+/// resolve on demand through whichever OutDb loader is registered for
+/// the `zarr` format. See [`group_to_indb_rasters`] for the meaning of
+/// `arrays`.
+pub fn group_to_outdb_rasters(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::OutDb, arrays)
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum Mode {
+ InDb,
+ OutDb,
+}
+
+/// Per-array metadata extracted once at group open and reused for every
+/// chunk position. Caching this avoids re-reading Zarr metadata for each
+/// of the (potentially thousands of) chunk rows.
+struct ArrayInfo {
+ /// Array path within the store, used to build chunk anchor URIs and
+ /// surface in band names.
+ path: String,
+ /// Open zarrs handle.
+ array: Array<FilesystemStore>,
+ /// SedonaDB BandDataType corresponding to this array's zarrs dtype.
+ data_type: BandDataType,
+ /// Dimension names in array order. Required to be `Some(_)` for every
+ /// dim; missing names error at validation time.
+ dim_names: Vec<String>,
+ /// Inner chunk grid shape, one entry per dimension. Used to enumerate
+ /// chunk positions and validated to match across arrays.
+ chunk_grid_shape: Vec<u64>,
+ /// Chunk shape (elements per chunk per dim). Same for every chunk
+ /// position — ragged final chunks are not emitted as separate short
+ /// rows.
+ chunk_shape: Vec<u64>,
+ /// Encoded fill value in native-endian byte representation, for the
+ /// `nodata` field. None when the array has no fill value declared.
+ nodata: Option<Vec<u8>>,
+}
+
+fn build_rasters(
+ group_uri: &str,
+ mode: Mode,
+ arrays_filter: Option<&[String]>,
+) -> Result<StructArray, ArrowError> {
+ let fs_path = group_uri_to_filesystem_path(group_uri)?;
+ let store = FilesystemStore::new(&fs_path).map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr filesystem store at {}: {e}",
+ fs_path.display()
+ )))
+ })?;
+ let storage: Arc<FilesystemStore> = Arc::new(store);
+
+ let group = Group::open(storage.clone(), "/").map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr group at {group_uri}: {e}"
+ )))
+ })?;
+
+ let geo = GroupGeoMetadata::from_attributes(group.attributes())?;
+
+ // CRS-without-transform is almost always a malformed-metadata bug —
+ // the user thinks they have full georef but downstream spatial joins
+ // will silently use the identity-pixel-coords default. Error loudly
+ // so they fix the metadata rather than getting empty result sets.
+ if geo.crs.is_some() && geo.transform.is_none() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} declares a CRS but no `spatial:transform` \
+ attribute; refusing to fall back to the identity transform because \
+ that would silently produce wrong results in spatial joins. Declare \
+ `spatial:transform` on the group or remove the CRS to read this as \
+ a non-georeferenced datacube."
+ )));
+ }
+
+ let arrays = group.child_arrays().map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to enumerate child arrays in {group_uri}: {e}"
+ )))
+ })?;
+ if arrays.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} has no child arrays"
+ )));
+ }
+
+ let arrays = select_arrays(arrays, arrays_filter, group_uri)?;
+ let array_infos = collect_array_infos(arrays)?;
+ validate_group_constraints(&array_infos)?;
+
+ // Spatial-dim resolution. Two configurations are accepted:
+ // - dim_names ends with ["y", "x"] (canonical for georeferenced
+ // 2-D and time-series rasters); the spatial extent is the chunk's
+ // last two dims.
+ // - `spatial:dims` attribute on the group explicitly names them.
+ // Anything else errors with a clear message — silently picking dims
+ // would produce wrong per-row transforms.
+ let spatial_dim_indices =
+ resolve_spatial_dim_indices(&array_infos[0].dim_names, geo.spatial_dims.as_deref())?;
+ let spatial_dims_names: Vec<&str> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].dim_names[i].as_str())
+ .collect();
+ let chunk_spatial_shape: Vec<i64> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].chunk_shape[i] as i64)
+ .collect();
+
+ let group_transform = match geo.transform {
+ Some(t) => t,
+ None => {
+ // Both `spatial:transform` and `proj:*` are absent (the
+ // CRS-only case errored above). Fall back to identity pixel
+ // coordinates and breadcrumb a warning so spatial-join
+ // surprises are debuggable.
+ log::warn!(
+ "Zarr group at {group_uri} has no `spatial:transform`; falling back \
+ to the identity pixel-coordinate transform [0, 1, 0, 0, 0, -1]. \
+ Spatial operations against this raster will use pixel coordinates."
+ );
+ [0.0, 1.0, 0.0, 0.0, 0.0, -1.0]
+ }
+ };
+
+ // chunk_grid_shape comes from untrusted Zarr metadata; bound-check the
+ // product so a hostile or malformed grid can't drive RasterBuilder
+ // capacity to overflow (and trigger an OOM preallocation).
+ let total_rows = array_infos[0]
+ .chunk_grid_shape
+ .iter()
+ .try_fold(1usize, |acc, &n| {
+ usize::try_from(n).ok().and_then(|n| acc.checked_mul(n))
+ })
+ .ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk grid shape {:?} overflows usize",
+ array_infos[0].chunk_grid_shape
+ ))
+ })?;
+ let mut builder = RasterBuilder::new(total_rows);
+
+ // Walk the chunk grid in row-major (C-order) order. The outer-most
+ // axis varies slowest, the innermost fastest — same convention used
+ // for byte strides in `BandRefImpl`.
+ let mut chunk_indices = vec![0u64; array_infos[0].chunk_grid_shape.len()];
+ loop {
+ let row_transform = compute_row_transform(
+ &group_transform,
+ &chunk_indices,
+ &array_infos[0].chunk_shape,
+ &spatial_dim_indices,
+ );
+ let crs_str = geo.crs.as_deref();
+ builder.start_raster_nd(
+ &row_transform,
+ &spatial_dims_names,
+ &chunk_spatial_shape,
+ crs_str,
+ )?;
+
+ for info in &array_infos {
+ let dim_names_ref: Vec<&str> = info.dim_names.iter().map(String::as_str).collect();
+ let nodata_ref = info.nodata.as_deref();
+ // Chunk anchor is provenance metadata; InDb vs OutDb is
+ // discriminated by `data.is_empty()`, not by these fields.
+ let anchor = build_chunk_anchor(group_uri, &info.path, &chunk_indices);
+ builder.start_band_nd(
+ Some(info.path.as_str()),
+ &dim_names_ref,
+ &info.chunk_shape,
+ info.data_type,
+ nodata_ref,
+ Some(anchor.as_str()),
+ Some("zarr"),
+ )?;
+ match mode {
+ Mode::InDb => {
+ let bytes = retrieve_chunk_bytes(&info.array, &chunk_indices)?;
+ builder.band_data_writer().append_value(&bytes);
+ }
+ Mode::OutDb => {
+ // Schema-OutDb: empty `data` column. Byte resolution
+ // routes through the OutDb loader when a downstream
+ // consumer calls contiguous_data / nd_buffer.
+ builder.band_data_writer().append_value([0u8; 0]);
+ }
+ }
+ builder.finish_band()?;
+ }
+ builder.finish_raster()?;
+
+ if !advance_chunk_indices(&mut chunk_indices, &array_infos[0].chunk_grid_shape) {
+ break;
+ }
+ }
+
+ builder.finish()
+}
+
+/// Collect per-array metadata from open zarrs `Array` handles.
+///
+/// Sorts arrays by path so band ordering across rows is deterministic
+/// (zarrs's underlying store listing order is implementation-defined —
Review Comment:
That wont work here. We are just ordering the arrays (ie the bands) lexically within the raster so that band(n) will refer to the same zarr variable in every raster record.
Its something that:
1) will tend to have low cardinality (n << 100 in almost all cases, typically 1-5)
2) does not have access to a datafusion session to query.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3276958000
##########
python/sedonadb/tests/test_funcs.py:
##########
@@ -24,3 +24,35 @@ def test_random_geometry(con):
# Ensure the output is reproducible
assert df.to_arrow_table() == df.to_arrow_table()
+
+
+def test_read_zarr(con, tmp_path):
+ # Skip cleanly if the optional `zarr` Python lib isn't installed —
+ # the binding is exercised by the Rust-side integration tests; this
+ # test only verifies the Python wrapper threads arguments through.
+ import pytest
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on PR #858:
URL: https://github.com/apache/sedona-db/pull/858#issuecomment-4514841810
Ah, I also see in CI the note about duplicate docs for `_lib` (`Consider documenting only one, renaming one, or marking one with `doc = false` in Cargo.toml.`). We should mark both internal Python crates as `doc = false` (and `publish = false` if they aren't already).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3290474388
##########
rust/sedona/Cargo.toml:
##########
@@ -50,6 +50,7 @@ s2geography = ["dep:sedona-s2geography", "dep:sedona-spatial-join-geography", "s
tempfile = { workspace = true }
tokio = { workspace = true, features = ["rt-multi-thread"] }
rstest = { workspace = true }
+sedona-raster = { workspace = true }
Review Comment:
removed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3284210353
##########
rust/sedona-raster-zarr/src/source_uri.rs:
##########
@@ -0,0 +1,285 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Zarr URI helpers — group locators and per-chunk OutDb anchors.
+//!
+//! Two URI shapes flow through the loader:
+//!
+//! 1. **Group URI** (the loader entry-point argument). Identifies a Zarr
+//! group on a backend, e.g. `file:///tmp/datacube.zarr` or a bare
+//! local path. Cloud schemes (`s3://`, `gs://`, `az://`, `https://`)
+//! will be accepted by a future revision; today only `file://` and
+//! bare-path are recognised.
+//! 2. **Chunk anchor URI** (written into a band's `outdb_uri`). Addresses
+//! one chunk in one array within a group:
+//! `<store-uri>#array=<array-path>&chunk=<i0>,<i1>,...`. The store URI
+//! is the original group URI verbatim (whatever scheme that uses);
+//! array path and chunk indices both live in the fragment so the
+//! store URI is unambiguous even when both contain `/` (e.g.
+//! `s3://bucket/foo.zarr/2024` + `subgroup/B01`). Inner-chunk indices
+//! always — sharding is a storage detail the resolver handles.
+//!
+//! The "this is a zarr anchor" signal lives in the band's
+//! `outdb_format = "zarr"` field, not in a URI scheme prefix — matches
+//! the GDAL OutDb convention and lets the format-keyed dispatcher
+//! route without parsing URI schemes.
+
+use arrow_schema::ArrowError;
+
+/// Parts of a chunk-anchor URI.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct ChunkAnchor {
+ /// Original store URI for the *group* (e.g. `file:///tmp/foo.zarr`,
+ /// `s3://bucket/foo.zarr/2024`). Same value across every chunk of every
+ /// array in a group.
+ pub store_uri: String,
+ /// Array's path within the store (e.g. `temperature`, `subgroup/B01`).
+ pub array_path: String,
+ /// Chunk's position in the array's inner chunk grid (one index per
+ /// array dimension).
+ pub chunk_indices: Vec<u64>,
+}
+
+/// Build a chunk anchor URI from its parts.
+///
+/// Format: `<store_uri>#array=<array_path>&chunk=<i0>,<i1>,...`. The
+/// store URI is the original group URI verbatim; array path and chunk
+/// indices live in the fragment so the store URI is unambiguous even
+/// when path components themselves contain `/`. "This is zarr" lives
+/// in the band's `outdb_format` field rather than in a URI scheme.
+pub fn build_chunk_anchor(store_uri: &str, array_path: &str, chunk_indices: &[u64]) -> String {
+ let indices = chunk_indices
+ .iter()
+ .map(|i| i.to_string())
+ .collect::<Vec<_>>()
+ .join(",");
+ let array = array_path.trim_start_matches('/');
+ format!("{store_uri}#array={array}&chunk={indices}")
+}
+
+/// Parse a chunk-anchor URI back into its parts.
+///
+/// Strict: rejects URIs that don't carry both `array=` and `chunk=`
+/// fragment parameters with valid values.
+pub fn parse_chunk_anchor(uri: &str) -> Result<ChunkAnchor, ArrowError> {
+ let (store_uri, fragment) = uri.split_once('#').ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk anchor URI missing `#array=...&chunk=...` fragment: {uri}"
+ ))
+ })?;
+
+ let mut array_path: Option<&str> = None;
+ let mut chunk_str: Option<&str> = None;
+ for pair in fragment.split('&') {
+ if let Some(v) = pair.strip_prefix("array=") {
+ array_path = Some(v);
+ } else if let Some(v) = pair.strip_prefix("chunk=") {
+ chunk_str = Some(v);
+ }
+ // Unknown fragment params are ignored — forward-compatible for
+ // future per-anchor extensions (e.g. version pins).
+ }
+
+ let array_path = array_path
+ .ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk anchor URI fragment missing `array=` parameter: {uri}"
+ ))
+ })?
+ .to_string();
+ let indices_str = chunk_str.ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk anchor URI fragment missing `chunk=` parameter: {uri}"
+ ))
+ })?;
+ if indices_str.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "chunk anchor URI has empty index list: {uri}"
+ )));
+ }
+ let chunk_indices = indices_str
+ .split(',')
+ .map(|s| {
+ s.parse::<u64>().map_err(|_| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk index `{s}` is not a non-negative integer in {uri}"
+ ))
+ })
+ })
+ .collect::<Result<Vec<_>, _>>()?;
+
+ Ok(ChunkAnchor {
+ store_uri: store_uri.to_string(),
+ array_path,
+ chunk_indices,
+ })
+}
+
+/// Normalize a user-supplied group URI into a local filesystem path.
+///
+/// Only `file://` and bare-path URIs are supported. Cloud schemes
+/// (`s3://`, `gs://`, `az://`, `https://`) error with a clear message.
+pub fn group_uri_to_filesystem_path(uri: &str) -> Result<std::path::PathBuf, ArrowError> {
Review Comment:
🏈 I think this is something I will have to worry about later!
##########
python/sedonadb/src/context.rs:
##########
@@ -200,4 +200,60 @@ impl InternalContext {
.to_string(),
))
}
+
+ /// Register a UDTF (table function) whose implementation is passed
+ /// through a `PyCapsule` containing an [`UdtfCapsule`].
+ ///
+ /// Plugin-handoff API: format-specific Python packages (e.g.
+ /// `sedonadb-zarr`) build their UDTF in their own PyO3 extension
+ /// and pass it across as an opaque capsule, sidestepping the
+ /// cross-extension `#[pyclass]` type-id mismatch.
+ ///
+ /// Two checks gate the dereference: the capsule's name must equal
+ /// [`UDTF_CAPSULE_NAME`] (a public string, so this is mostly an
+ /// ABI tag), and the payload's leading [`UDTF_CAPSULE_MAGIC`]
+ /// sentinel must match (much harder to forge from outside this
+ /// crate). The consumer clones the inner `Arc` to take its own
+ /// refcount; the capsule keeps a parallel refcount until Python
+ /// GC drops it.
+ ///
+ /// [`UdtfCapsule`]: crate::plugin::UdtfCapsule
+ /// [`UDTF_CAPSULE_NAME`]: crate::plugin::UDTF_CAPSULE_NAME
+ /// [`UDTF_CAPSULE_MAGIC`]: crate::plugin::UDTF_CAPSULE_MAGIC
+ pub fn register_udtf_capsule(
+ &self,
+ name: &str,
+ capsule: &Bound<'_, pyo3::types::PyCapsule>,
+ ) -> Result<(), PySedonaError> {
+ use crate::plugin::{UdtfCapsule, UDTF_CAPSULE_MAGIC, UDTF_CAPSULE_NAME};
+
+ let actual = capsule
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3284205420
##########
python/sedonadb-zarr/pyproject.toml:
##########
@@ -0,0 +1,49 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[build-system]
+requires = ["maturin>=1.8,<2.0"]
+build-backend = "maturin"
+
+[project]
+name = "sedonadb-zarr"
+readme = "README.md"
+requires-python = ">=3.9"
+classifiers = [
+ "Programming Language :: Rust",
+ "Programming Language :: Python :: Implementation :: CPython",
+ "Programming Language :: Python :: Implementation :: PyPy",
+]
+dynamic = ["version"]
+dependencies = [
+ "sedonadb",
+]
Review Comment:
done- testing now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3283687543
##########
rust/sedona-raster-zarr/src/loader.rs:
##########
@@ -0,0 +1,814 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Zarr group → N-D raster `StructArray` entry points.
+//!
+//! Both `group_to_indb_rasters` and `group_to_outdb_rasters` produce the
+//! same row shape: one raster row per chunk position, with one band per
+//! array in the group. They differ only in how each row's pixel bytes
+//! are delivered:
+//!
+//! - **InDb** — every chunk is fetched eagerly and copied into the
+//! Arrow `data` column. Heavy for large datacubes; intended for
+//! snapshots.
+//! - **OutDb** — `data` is left empty; each band's `outdb_uri` carries a
+//! chunk anchor (`zarr://<store-uri>/<array-path>#chunk=i0,i1,...`).
+//! Byte resolution awaits the format-keyed dispatch work in a
+//! follow-up PR.
+
+use std::sync::Arc;
+
+use arrow_array::{RecordBatch, RecordBatchReader};
+use arrow_schema::{ArrowError, Schema, SchemaRef};
+use sedona_common::sedona_internal_datafusion_err;
+use sedona_raster::builder::RasterBuilder;
+use sedona_schema::datatypes::SedonaType;
+use sedona_schema::raster::BandDataType;
+use zarrs::array::{Array, ArrayBytes};
+use zarrs::group::Group;
+use zarrs_filesystem::FilesystemStore;
+
+use crate::dtype::zarr_to_band_data_type;
+use crate::geozarr::GroupGeoMetadata;
+use crate::source_uri::{build_chunk_anchor, group_uri_to_filesystem_path};
+
+/// Streaming reader over the chunk grid of a Zarr group.
+///
+/// Each `next()` call emits one `RecordBatch` containing up to
+/// `batch_size` rows; one row per chunk position. The reader holds the
+/// open group, parsed metadata, and the current chunk-grid position —
+/// metadata parsing happens once in [`ZarrChunkReader::try_new`] and
+/// the per-row work is just transform arithmetic + anchor URI
+/// formatting.
+///
+/// Rows always emit OutDb-style: `data` is empty, `outdb_uri` carries
+/// a chunk anchor that the async OutDb resolver (registered separately)
+/// resolves to bytes on demand.
+pub struct ZarrChunkReader {
+ schema: SchemaRef,
+ group_uri: String,
+ array_infos: Vec<ArrayInfo>,
+ geo: GroupGeoMetadata,
+ group_transform: [f64; 6],
+ spatial_dim_indices: Vec<usize>,
+ /// Owned copy of `array_infos[0].dim_names[i]` for `i` in
+ /// `spatial_dim_indices`. Kept here so `next()` doesn't need to
+ /// rebuild the `&str` slice each row.
+ spatial_dims_names: Vec<String>,
+ chunk_spatial_shape: Vec<i64>,
+ /// Current position in the chunk grid (row-major, C-order).
+ chunk_indices: Vec<u64>,
+ /// Whether the grid has been exhausted.
+ exhausted: bool,
+ batch_size: usize,
+}
+
+impl std::fmt::Debug for ZarrChunkReader {
+ fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
+ f.debug_struct("ZarrChunkReader")
+ .field("group_uri", &self.group_uri)
+ .field("num_arrays", &self.array_infos.len())
+ .field("chunk_indices", &self.chunk_indices)
+ .field("exhausted", &self.exhausted)
+ .field("batch_size", &self.batch_size)
+ .finish()
+ }
+}
+
+impl ZarrChunkReader {
+ /// Open a Zarr group and prepare a streaming reader over its chunk
+ /// grid. Performs all I/O for metadata up front (open group, parse
+ /// attributes, list arrays, validate constraints) so per-batch work
+ /// is cheap.
+ ///
+ /// `arrays`:
+ /// - `None` — read every multi-dimensional array. 1-D arrays
+ /// (typical xarray coord variables) are auto-skipped.
+ /// - `Some(names)` — read exactly the named arrays. Unknown names
+ /// error. 1-D arrays are always rejected (a raster band needs
+ /// ≥ 2 dimensions); naming one explicitly errors with a clear
+ /// message.
+ ///
+ /// `batch_size` controls how many chunk rows are emitted per
+ /// `RecordBatch`. Must be ≥ 1; callers typically pass
+ /// `SessionConfig::batch_size` (defaults to 8192).
+ pub fn try_new(
+ group_uri: &str,
+ arrays: Option<&[String]>,
+ batch_size: usize,
+ ) -> Result<Self, ArrowError> {
+ let batch_size = batch_size.max(1);
+ let OpenedGroup {
+ array_infos,
+ geo,
+ group_transform,
+ spatial_dim_indices,
+ } = open_and_validate(group_uri, arrays)?;
+
+ let spatial_dims_names: Vec<String> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].dim_names[i].clone())
+ .collect();
+ let chunk_spatial_shape: Vec<i64> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].chunk_shape[i] as i64)
+ .collect();
+
+ let raster_field = SedonaType::Raster
+ .to_storage_field("raster", true)
+ .map_err(|e| ArrowError::ExternalError(Box::new(e)))?;
+ let schema = Arc::new(Schema::new(vec![raster_field]));
+
+ let chunk_indices = vec![0u64; array_infos[0].chunk_grid_shape.len()];
+
+ Ok(Self {
+ schema,
+ group_uri: group_uri.to_string(),
+ array_infos,
+ geo,
+ group_transform,
+ spatial_dim_indices,
+ spatial_dims_names,
+ chunk_spatial_shape,
+ chunk_indices,
+ exhausted: false,
+ batch_size,
+ })
+ }
+
+ /// Append one raster row at the current `chunk_indices` to the
+ /// in-progress builder. Does not advance the cursor.
+ fn emit_one_row(&self, builder: &mut RasterBuilder) -> Result<(), ArrowError> {
+ let row_transform = compute_row_transform(
+ &self.group_transform,
+ &self.chunk_indices,
+ &self.array_infos[0].chunk_shape,
+ &self.spatial_dim_indices,
+ );
+ let spatial_dims_ref: Vec<&str> =
+ self.spatial_dims_names.iter().map(String::as_str).collect();
+ let crs_str = self.geo.crs.as_deref();
+ builder.start_raster_nd(
+ &row_transform,
+ &spatial_dims_ref,
+ &self.chunk_spatial_shape,
+ crs_str,
+ )?;
+
+ for info in &self.array_infos {
+ let dim_names_ref: Vec<&str> = info.dim_names.iter().map(String::as_str).collect();
+ let nodata_ref = info.nodata.as_deref();
+ // Every band gets its chunk-anchor URI populated as
+ // provenance metadata. `data.is_empty()` is the InDb/OutDb
+ // discriminator; this reader always emits empty `data` and
+ // defers pixel-byte resolution to the OutDb resolver.
+ let anchor = build_chunk_anchor(&self.group_uri, &info.path, &self.chunk_indices);
+ builder.start_band_nd(
+ Some(info.path.as_str()),
+ &dim_names_ref,
+ &info.chunk_shape,
+ info.data_type,
+ nodata_ref,
+ Some(anchor.as_str()),
+ Some("zarr"),
+ )?;
+ builder.band_data_writer().append_value([0u8; 0]);
+ builder.finish_band()?;
+ }
+ builder.finish_raster()
+ }
+}
+
+impl Iterator for ZarrChunkReader {
+ type Item = Result<RecordBatch, ArrowError>;
+
+ fn next(&mut self) -> Option<Self::Item> {
+ if self.exhausted {
+ return None;
+ }
+ let mut builder = RasterBuilder::new(self.batch_size);
+ let mut rows_emitted = 0usize;
+ while rows_emitted < self.batch_size {
+ if let Err(e) = self.emit_one_row(&mut builder) {
+ self.exhausted = true;
+ return Some(Err(e));
+ }
+ rows_emitted += 1;
+ if !advance_chunk_indices(
+ &mut self.chunk_indices,
+ &self.array_infos[0].chunk_grid_shape,
+ ) {
+ self.exhausted = true;
+ break;
+ }
+ }
+ if rows_emitted == 0 {
+ return None;
+ }
+ let struct_arr = match builder.finish() {
+ Ok(arr) => arr,
+ Err(e) => return Some(Err(e)),
+ };
+ Some(RecordBatch::try_new(
+ self.schema.clone(),
+ vec![Arc::new(struct_arr)],
+ ))
+ }
+}
+
+impl RecordBatchReader for ZarrChunkReader {
+ fn schema(&self) -> SchemaRef {
+ self.schema.clone()
+ }
+}
+
+/// Per-array metadata extracted once at group open and reused for every
+/// chunk position. Caching this avoids re-reading Zarr metadata for each
+/// of the (potentially thousands of) chunk rows.
+struct ArrayInfo {
+ /// Array path within the store, used to build chunk anchor URIs and
+ /// surface in band names.
+ path: String,
+ /// SedonaDB BandDataType corresponding to this array's zarrs dtype.
+ data_type: BandDataType,
+ /// Dimension names in array order. Required to be `Some(_)` for every
+ /// dim; missing names error at validation time.
+ dim_names: Vec<String>,
+ /// Inner chunk grid shape, one entry per dimension. Used to enumerate
+ /// chunk positions and validated to match across arrays.
+ chunk_grid_shape: Vec<u64>,
+ /// Chunk shape (elements per chunk per dim). Same for every chunk
+ /// position — ragged final chunks are not emitted as separate short
+ /// rows.
+ chunk_shape: Vec<u64>,
+ /// Encoded fill value in native-endian byte representation, for the
+ /// `nodata` field. None when the array has no fill value declared.
+ nodata: Option<Vec<u8>>,
+}
+
+/// Bundle of group-level state returned by [`open_and_validate`].
+struct OpenedGroup {
+ array_infos: Vec<ArrayInfo>,
+ geo: GroupGeoMetadata,
+ group_transform: [f64; 6],
+ spatial_dim_indices: Vec<usize>,
+}
+
+/// Open the Zarr group, parse and validate group metadata, and return
+/// everything `ZarrChunkReader` needs to iterate without further I/O
+/// (apart from per-chunk byte fetches by future resolvers).
+fn open_and_validate(
+ group_uri: &str,
+ arrays_filter: Option<&[String]>,
+) -> Result<OpenedGroup, ArrowError> {
+ let fs_path = group_uri_to_filesystem_path(group_uri)?;
+ let store = FilesystemStore::new(&fs_path).map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr filesystem store at {}: {e}",
+ fs_path.display()
+ )))
+ })?;
+ let storage: Arc<FilesystemStore> = Arc::new(store);
+
+ let group = Group::open(storage.clone(), "/").map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr group at {group_uri}: {e}"
+ )))
+ })?;
+
+ let geo = GroupGeoMetadata::from_attributes(group.attributes())?;
+
+ // CRS-without-transform is almost always a malformed-metadata bug —
+ // the user thinks they have full georef but downstream spatial joins
+ // will silently use the identity-pixel-coords default. Error loudly
+ // so they fix the metadata rather than getting empty result sets.
+ if geo.crs.is_some() && geo.transform.is_none() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} declares a CRS but no `spatial:transform` \
+ attribute; refusing to fall back to the identity transform because \
+ that would silently produce wrong results in spatial joins. Declare \
+ `spatial:transform` on the group or remove the CRS to read this as \
+ a non-georeferenced datacube."
+ )));
+ }
+
+ let arrays = group.child_arrays().map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to enumerate child arrays in {group_uri}: {e}"
+ )))
+ })?;
+ if arrays.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} has no child arrays"
+ )));
+ }
+
+ let arrays = select_arrays(arrays, arrays_filter, group_uri)?;
+ let array_infos = collect_array_infos(arrays)?;
+ validate_group_constraints(&array_infos)?;
+
+ // Spatial-dim resolution. Two configurations are accepted:
+ // - dim_names ends with ["y", "x"] (canonical for georeferenced
+ // 2-D and time-series rasters); the spatial extent is the chunk's
+ // last two dims.
+ // - `spatial:dims` attribute on the group explicitly names them.
+ // Anything else errors with a clear message — silently picking dims
+ // would produce wrong per-row transforms.
+ let spatial_dim_indices =
+ resolve_spatial_dim_indices(&array_infos[0].dim_names, geo.spatial_dims.as_deref())?;
+
+ let group_transform = match geo.transform {
+ Some(t) => t,
+ None => {
+ // Both `spatial:transform` and `proj:*` are absent (the
+ // CRS-only case errored above). Fall back to identity pixel
+ // coordinates and breadcrumb a warning so spatial-join
+ // surprises are debuggable.
+ log::warn!(
+ "Zarr group at {group_uri} has no `spatial:transform`; falling back \
+ to the identity pixel-coordinate transform [0, 1, 0, 0, 0, -1]. \
+ Spatial operations against this raster will use pixel coordinates."
+ );
+ [0.0, 1.0, 0.0, 0.0, 0.0, -1.0]
+ }
+ };
+
+ // chunk_grid_shape comes from untrusted Zarr metadata; bound-check the
+ // product so a hostile or malformed grid can't make per-batch
+ // RasterBuilder capacity (or downstream consumers) overflow.
+ array_infos[0]
+ .chunk_grid_shape
+ .iter()
+ .try_fold(1usize, |acc, &n| {
+ usize::try_from(n).ok().and_then(|n| acc.checked_mul(n))
+ })
+ .ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "chunk grid shape {:?} overflows usize",
+ array_infos[0].chunk_grid_shape
+ ))
+ })?;
+
+ Ok(OpenedGroup {
+ array_infos,
+ geo,
+ group_transform,
+ spatial_dim_indices,
+ })
+}
+
+/// Collect per-array metadata from open zarrs `Array` handles.
+///
+/// Sorts arrays by path so band ordering across rows is deterministic
+/// (zarrs's underlying store listing order is implementation-defined —
+/// filesystem stores currently happen to enumerate alphabetically, but
+/// that's not part of the contract we want consumers to rely on).
+fn collect_array_infos(
+ mut arrays: Vec<Array<FilesystemStore>>,
+) -> Result<Vec<ArrayInfo>, ArrowError> {
+ arrays.sort_by(|a, b| a.path().as_str().cmp(b.path().as_str()));
+ let mut out = Vec::with_capacity(arrays.len());
+ for array in arrays {
+ let path = array.path().to_string();
+ let data_type = zarr_to_band_data_type(array.data_type())?;
+ let dim_names = resolve_dim_names(&array, &path)?;
+ let chunk_grid_shape = array.chunk_grid_shape().to_vec();
+ let chunk_shape = array
+ .chunk_shape(&vec![0u64; chunk_grid_shape.len()])
+ .map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "array {path}: failed to query chunk shape: {e}"
+ )))
+ })?
+ .iter()
+ .map(|n| n.get())
+ .collect();
+ let fill_bytes = array.fill_value().as_ne_bytes();
+ let nodata = if fill_bytes.is_empty() {
+ None
+ } else {
+ Some(fill_bytes.to_vec())
+ };
+ out.push(ArrayInfo {
+ path,
+ data_type,
+ dim_names,
+ chunk_grid_shape,
+ chunk_shape,
+ nodata,
+ });
+ }
+ Ok(out)
+}
+
+/// Apply the array-selection rules. 1-D arrays (typical xarray-style
+/// coord variables) are always dropped — a raster band requires at
+/// least 2 dimensions, so reading a 1-D array could never succeed.
+/// - Explicit filter: keep arrays whose path (with leading `/` stripped)
+/// matches one of the requested names. Unknown names error so users
+/// don't silently get an empty group from a typo; naming a 1-D array
+/// errors with a clear message rather than producing a confusing
+/// "no spatial axes" failure downstream.
+/// - No filter: read every multi-dimensional array.
+fn select_arrays(
+ arrays: Vec<Array<FilesystemStore>>,
+ filter: Option<&[String]>,
+ group_uri: &str,
+) -> Result<Vec<Array<FilesystemStore>>, ArrowError> {
+ if let Some(names) = filter {
+ let available: Vec<String> = arrays
+ .iter()
+ .map(|a| a.path().as_str().trim_start_matches('/').to_string())
+ .collect();
+ for requested in names {
+ let needle = requested.trim_start_matches('/');
+ match arrays
+ .iter()
+ .find(|a| a.path().as_str().trim_start_matches('/') == needle)
+ {
+ None => {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} has no array named {requested:?}; \
+ available arrays: {available:?}"
+ )));
+ }
+ Some(a) if a.shape().len() < 2 => {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "array {requested:?} has rank {} (shape {:?}); a raster band \
+ requires at least 2 dimensions and cannot be read.",
+ a.shape().len(),
+ a.shape()
+ )));
+ }
+ Some(_) => {}
+ }
+ }
+ let kept: Vec<_> = arrays
+ .into_iter()
+ .filter(|a| {
+ let path = a.path().as_str().trim_start_matches('/').to_string();
+ names.iter().any(|n| n.trim_start_matches('/') == path)
+ })
+ .collect();
+ return Ok(kept);
+ }
+
+ let kept: Vec<_> = arrays.into_iter().filter(|a| a.shape().len() > 1).collect();
+ if kept.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} contains only 1-D arrays (typical \
+ xarray-style coord variables); a raster band requires at least \
+ 2 dimensions, so this group has nothing readable as a raster"
+ )));
+ }
+ Ok(kept)
+}
+
+/// Resolve dimension names for an array, supporting both Zarr v3
+/// (first-class `dimension_names` field) and Zarr v2 with the xarray
+/// `_ARRAY_DIMENSIONS` attribute. Errors if neither carries a complete
+/// set of named dimensions matching the array's rank.
+fn resolve_dim_names<S: ?Sized>(array: &Array<S>, path: &str) -> Result<Vec<String>, ArrowError> {
+ let rank = array.shape().len();
+
+ if let Some(names) = array.dimension_names() {
+ return names
+ .iter()
+ .enumerate()
+ .map(|(i, n)| {
+ n.clone().ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "array {path}: dimension {i} has no name; every Zarr array \
+ dimension must be named",
+ ))
+ })
+ })
+ .collect();
+ }
+
+ // Zarr v2 fallback: xarray's `_ARRAY_DIMENSIONS` convention. v2 had no
+ // first-class dimension_names field and zarrs's v2->v3 converter
+ // preserves attributes verbatim without lifting this one.
+ if let Some(value) = array.attributes().get("_ARRAY_DIMENSIONS") {
+ let arr = value.as_array().ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "array {path}: _ARRAY_DIMENSIONS must be a JSON array of strings; got {value}"
+ ))
+ })?;
+ if arr.len() != rank {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "array {path}: _ARRAY_DIMENSIONS has {} entries but array has rank {rank}",
+ arr.len()
+ )));
+ }
+ return arr
+ .iter()
+ .enumerate()
+ .map(|(i, v)| {
+ v.as_str().map(str::to_string).ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "array {path}: _ARRAY_DIMENSIONS[{i}] must be a string; got {v}"
+ ))
+ })
+ })
+ .collect();
+ }
+
+ Err(ArrowError::InvalidArgumentError(format!(
+ "array {path}: no dimension names found. Zarr v3 arrays must set \
+ `dimension_names`; Zarr v2 arrays must set the `_ARRAY_DIMENSIONS` \
+ attribute (xarray convention)."
+ )))
+}
+
+/// Enforce group constraints. All arrays must agree on chunk grid shape,
+/// chunk shape, and dimension names. We do NOT enforce shared element
+/// shape (`array.shape()`) because users routinely group arrays with
+/// the same chunk grid but different totals (e.g. a coord variable with
+/// one fewer dim is rejected here anyway by the dim-name check).
+fn validate_group_constraints(infos: &[ArrayInfo]) -> Result<(), ArrowError> {
+ let first = &infos[0];
+ for other in &infos[1..] {
+ if other.chunk_grid_shape != first.chunk_grid_shape {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "arrays {} and {} have different chunk grid shapes ({:?} vs {:?}); \
+ every array in the group must share the same chunk grid. \
+ Pass `arrays = [...]` to read only compatible arrays.",
+ first.path, other.path, first.chunk_grid_shape, other.chunk_grid_shape
+ )));
+ }
+ if other.chunk_shape != first.chunk_shape {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "arrays {} and {} have different chunk shapes ({:?} vs {:?}); \
+ every array in the group must share the same chunk shape. \
+ Pass `arrays = [...]` to read only compatible arrays.",
+ first.path, other.path, first.chunk_shape, other.chunk_shape
+ )));
+ }
+ if other.dim_names != first.dim_names {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "arrays {} and {} have different dimension names ({:?} vs {:?}); \
+ every array in the group must declare identical dim_names. \
+ Pass `arrays = [...]` to read only compatible arrays.",
+ first.path, other.path, first.dim_names, other.dim_names
+ )));
+ }
+ }
+ Ok(())
+}
+
+/// Pick the `(y_index, x_index)` axes of an array's dim_names.
+///
+/// If `spatial_dims` is provided via the group's `spatial:dims` attribute,
+/// look up those names by position. Otherwise, default to the last two
+/// dims and require they be named `y` and `x` (in that order) — the
+/// canonical GeoZarr-2D convention. Anything else errors.
+fn resolve_spatial_dim_indices(
+ dim_names: &[String],
+ spatial_dims: Option<&[String]>,
+) -> Result<Vec<usize>, ArrowError> {
+ if let Some(spatial) = spatial_dims {
+ let mut idx = Vec::with_capacity(spatial.len());
+ for s in spatial {
+ let i = dim_names.iter().position(|n| n == s).ok_or_else(|| {
+ ArrowError::InvalidArgumentError(format!(
+ "spatial:dims declared name {s:?} not found in array dim_names {dim_names:?}",
+ ))
+ })?;
+ idx.push(i);
+ }
+ return Ok(idx);
+ }
+ let n = dim_names.len();
+ if n < 2 {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "at least 2 dimensions are required to resolve spatial axes; got {dim_names:?}",
+ )));
+ }
+ if dim_names[n - 2] != "y" || dim_names[n - 1] != "x" {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "the last two dim_names must be [\"y\", \"x\"] when `spatial:dims` is \
+ not declared; got {dim_names:?}",
+ )));
+ }
+ Ok(vec![n - 2, n - 1])
+}
+
+/// Per-chunk transform: translate the group's transform so the chunk's
+/// `[0, 0]` element maps to the chunk's spatial origin.
+fn compute_row_transform(
+ group_transform: &[f64; 6],
+ chunk_indices: &[u64],
+ chunk_shape: &[u64],
+ spatial_dim_indices: &[usize],
+) -> [f64; 6] {
+ // GDAL GeoTransform layout: [origin_x, scale_x, skew_x, origin_y, skew_y, scale_y].
+ // Translation along x = chunk_x_index × chunk_x_size in pixel-coordinate space,
+ // converted to world coordinates via the affine.
+ //
+ // spatial_dim_indices is validated upstream to be [y_index, x_index].
+ // Index 0 is the y axis, index 1 is the x axis.
+ let y_axis = spatial_dim_indices[0];
+ let x_axis = spatial_dim_indices[1];
+ let x_offset = (chunk_indices[x_axis] * chunk_shape[x_axis]) as f64;
+ let y_offset = (chunk_indices[y_axis] * chunk_shape[y_axis]) as f64;
+ let [ox, sx, kx, oy, ky, sy] = *group_transform;
+ [
+ ox + sx * x_offset + kx * y_offset,
+ sx,
+ kx,
+ oy + ky * x_offset + sy * y_offset,
+ ky,
+ sy,
+ ]
+}
+
+/// Advance `chunk_indices` row-major over `chunk_grid_shape`. Returns
+/// `true` while there are positions left, `false` when the grid is
+/// exhausted (and the indices wrap back to all-zero).
+fn advance_chunk_indices(chunk_indices: &mut [u64], chunk_grid_shape: &[u64]) -> bool {
+ for i in (0..chunk_indices.len()).rev() {
+ chunk_indices[i] += 1;
+ if chunk_indices[i] < chunk_grid_shape[i] {
+ return true;
+ }
+ chunk_indices[i] = 0;
+ }
+ false
+}
+
+/// Retrieve a single chunk's bytes as a fresh `Vec<u8>`.
+///
+/// Uses `ArrayBytes::Fixed`, so this errors for variable-length element
+/// types — those don't have a `BandDataType` counterpart anyway, so the
+/// dtype check in `collect_array_infos` rejects them upstream.
+///
+/// This is the only pixel-byte read primitive in the crate. The loader
+/// itself never calls it today — it always emits OutDb anchors — but
+/// the async `RS_EnsureLoaded` resolver (follow-up PR) will. Kept here
+/// rather than dropped because (a) it's tested directly below and (b)
+/// the resolver lives in this crate and wants a sync chunk-fetch
+/// helper as a starting point.
+#[allow(dead_code)]
Review Comment:
sorry this slipped by my eye. fixed
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3283689696
##########
rust/sedona-raster-zarr/src/udtf.rs:
##########
@@ -0,0 +1,614 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! `sd_read_zarr` table function — exposes Zarr-backed raster groups
+//! as SQL-queryable tables.
+//!
+//! ```sql
+//! SELECT * FROM sd_read_zarr('file:///path/to/datacube.zarr');
+//! SELECT count(*) FROM sd_read_zarr(
+//! 'file:///path/to/datacube.zarr',
+//! '{"rows_per_batch": 256}'
+//! );
+//! ```
+//!
+//! Returns a single-column table `raster: Raster` with one row per chunk
+//! position in the Zarr group's chunk grid. All existing `RS_*` UDFs
+//! operate on the column unchanged.
+//!
+//! `load_eager` defaults to `false` — every row has empty `data` and a
+//! chunk-anchor URI in `outdb_uri`. Setting `load_eager = true`
+//! currently errors because no `RS_EnsureLoaded` resolver is registered
+//! for the `zarr` format yet; once the resolver lands, `true` will
+//! trigger the planner to inject the async UDF over the scan output.
+//!
+//! Registration happens via [`register`]; the `sedonadb-zarr` Python
+//! package calls this from its session-setup helper. The `sedona` crate
+//! itself does not register the UDTF — keeping zarr functionality out
+//! of the default bootstrap.
+
+use std::any::Any;
+use std::sync::Arc;
+
+use arrow_schema::{DataType, Schema, SchemaRef};
+use async_trait::async_trait;
+use datafusion::catalog::{Session, TableFunctionImpl, TableProvider};
+use datafusion::common::Result;
+use datafusion::datasource::TableType;
+use datafusion::execution::context::{SessionContext, TaskContext};
+use datafusion::physical_expr::EquivalenceProperties;
+use datafusion::physical_plan::execution_plan::{Boundedness, EmissionType};
+use datafusion::physical_plan::expressions::Column;
+use datafusion::physical_plan::projection::ProjectionExec;
+use datafusion::physical_plan::stream::RecordBatchStreamAdapter;
+use datafusion::physical_plan::{
+ DisplayAs, DisplayFormatType, ExecutionPlan, Partitioning, PhysicalExpr, PlanProperties,
+ SendableRecordBatchStream,
+};
+use datafusion::prelude::Expr;
+use datafusion_common::{plan_err, DataFusionError, ScalarValue};
+use sedona_schema::datatypes::SedonaType;
+use serde::{Deserialize, Serialize};
+
+use crate::loader::ZarrChunkReader;
+
+/// Register the `sd_read_zarr` UDTF on a `SessionContext`. Called from
+/// the `sedonadb-zarr` Python package's `register(con)` entry point.
+pub fn register(ctx: &SessionContext) {
+ ctx.register_udtf("sd_read_zarr", Arc::new(ZarrReadFunction::default()));
+}
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3284199082
##########
python/sedonadb-zarr/Cargo.toml:
##########
@@ -0,0 +1,44 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+[package]
+name = "sedonadb-zarr"
+version = { workspace = true }
+publish = false
+edition = "2021"
+
+[lib]
+# rustc crate name. Distinct from `sedonadb`'s `_lib` to avoid the
+# rustdoc output-path collision on `cargo doc --workspace`. The
+# matching pymodule function in `src/lib.rs` is named `_zarr_lib`
+# so its `PyInit__zarr_lib` symbol doesn't collide with sedonadb's
+# `PyInit__lib` under cargo workspace feature unification.
+name = "_zarr_lib"
+crate-type = ["cdylib"]
+
+[dependencies]
+arrow-array = { workspace = true }
+datafusion = { workspace = true }
+pyo3 = { version = "0.25.1" }
+sedona-raster-zarr = { workspace = true }
+# The sedonadb Python extension; its rustc crate name is `_lib` per
+# the `[lib].name` override in `python/sedonadb/Cargo.toml`. We import
+# it in `src/lib.rs` via the leading `::_lib` syntax to disambiguate
+# from our own crate's `_lib` pymodule. Inlined here (not via
+# `workspace = true`) to avoid a self-referential workspace dep that
+# trips `cargo metadata` in cibuildwheel's isolated build env.
+sedonadb = { path = "../sedonadb", default-features = false }
Review Comment:
done. was able to keep the UDTF
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3284281134
##########
rust/sedona-raster-zarr/src/dtype.rs:
##########
@@ -0,0 +1,66 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Mapping between Zarr datatypes and SedonaDB's `BandDataType`.
+//!
+//! zarrs 0.23 models `DataType` as a wrapper around `Arc<dyn DataTypeTraits>`,
+//! so we discriminate via the type-erased `is::<T>()` check rather than
+//! pattern-matching an enum.
+
+use arrow_schema::ArrowError;
+use sedona_schema::raster::BandDataType;
+use zarrs::array::data_type::{
+ BoolDataType, Float32DataType, Float64DataType, Int16DataType, Int32DataType, Int64DataType,
+ Int8DataType, UInt16DataType, UInt32DataType, UInt64DataType, UInt8DataType,
+};
+use zarrs::array::DataType as ZarrDataType;
+
+/// Map a Zarr `DataType` to a SedonaDB `BandDataType`.
+///
+/// Bool maps to UInt8 losslessly (Zarr packs bools to one byte each, matching
+/// our representation). Variable-length, complex, and extended-precision
+/// types error with `NotYetImplemented` — they have no numeric counterpart
+/// in `BandDataType` today.
+pub fn zarr_to_band_data_type(dt: &ZarrDataType) -> Result<BandDataType, ArrowError> {
+ if dt.is::<BoolDataType>() {
+ Ok(BandDataType::UInt8)
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sedona-zarr plugin [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot merged PR #858:
URL: https://github.com/apache/sedona-db/pull/858
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3290433833
##########
python/sedonadb/src/datasource.rs:
##########
@@ -143,7 +153,28 @@ impl PyExternalFormat {
#[new]
fn new<'py>(py: Python<'py>, py_spec: PyObject) -> Result<Self, PySedonaError> {
let extension = py_spec.getattr(py, "extension")?.extract::<String>(py)?;
- Ok(Self { extension, py_spec })
+ let list_single_object = read_list_single_object(py, &py_spec)?;
+ Ok(Self {
+ extension,
+ list_single_object,
+ py_spec,
+ })
+ }
+}
+
+/// Read the `list_single_object` attribute on a Python spec, defaulting
+/// to `false` if the attribute is missing (older spec implementations
+/// that predate the directory-format path).
+fn read_list_single_object<'py>(
Review Comment:
we have the Zarr one now as well.
While pyogrio only supports single file formats, zarr (and maybe future file formats) support dirs so I think this is the right place to do this change.
or maybe im missing your point. Are you saying something about having a default behavior?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3270113571
##########
rust/sedona-raster-zarr/src/loader.rs:
##########
@@ -0,0 +1,513 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! Zarr group → N-D raster `StructArray` entry points.
+//!
+//! Both `group_to_indb_rasters` and `group_to_outdb_rasters` produce the
+//! same row shape: one raster row per chunk position, with one band per
+//! array in the group. They differ only in how each row's pixel bytes
+//! are delivered:
+//!
+//! - **InDb** — every chunk is fetched eagerly and copied into the
+//! Arrow `data` column. Heavy for large datacubes; intended for
+//! snapshots.
+//! - **OutDb** — `data` is left empty; each band's `outdb_uri` carries a
+//! chunk anchor (`zarr://<store-uri>/<array-path>#chunk=i0,i1,...`).
+//! Byte resolution awaits the format-keyed dispatch work in a
+//! follow-up PR.
+
+use std::sync::Arc;
+
+use arrow_array::StructArray;
+use arrow_schema::ArrowError;
+use sedona_common::sedona_internal_datafusion_err;
+use sedona_raster::builder::RasterBuilder;
+use sedona_schema::raster::BandDataType;
+use zarrs::array::{Array, ArrayBytes};
+use zarrs::group::Group;
+use zarrs_filesystem::FilesystemStore;
+
+use crate::dtype::zarr_to_band_data_type;
+use crate::geozarr::GroupGeoMetadata;
+use crate::source_uri::{build_chunk_anchor, group_uri_to_filesystem_path};
+
+/// Open a Zarr group and eagerly fetch every chunk's bytes into the
+/// returned `StructArray`. Each row holds one chunk position's data
+/// across every array in the group.
+pub fn group_to_indb_rasters(group_uri: &str) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::InDb)
+}
+
+/// Open a Zarr group and emit one row per chunk position with chunk-anchor
+/// URIs in each band's `outdb_uri`. The `data` column is empty; bytes
+/// resolve on demand through whichever OutDb loader is registered for
+/// the `zarr` format.
+pub fn group_to_outdb_rasters(group_uri: &str) -> Result<StructArray, ArrowError> {
+ build_rasters(group_uri, Mode::OutDb)
+}
+
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+enum Mode {
+ InDb,
+ OutDb,
+}
+
+/// Per-array metadata extracted once at group open and reused for every
+/// chunk position. Caching this avoids re-reading Zarr metadata for each
+/// of the (potentially thousands of) chunk rows.
+struct ArrayInfo {
+ /// Array path within the store, used to build chunk anchor URIs and
+ /// surface in band names.
+ path: String,
+ /// Open zarrs handle.
+ array: Array<FilesystemStore>,
+ /// SedonaDB BandDataType corresponding to this array's zarrs dtype.
+ data_type: BandDataType,
+ /// Dimension names in array order. Required to be `Some(_)` for every
+ /// dim; missing names error at validation time.
+ dim_names: Vec<String>,
+ /// Inner chunk grid shape, one entry per dimension. Used to enumerate
+ /// chunk positions and validated to match across arrays.
+ chunk_grid_shape: Vec<u64>,
+ /// Chunk shape (elements per chunk per dim). Same for every chunk
+ /// position in Phase 1 (no ragged final chunks emitted as separate
+ /// short rows).
+ chunk_shape: Vec<u64>,
+ /// Encoded fill value in native-endian byte representation, for the
+ /// `nodata` field. None when the array has no fill value declared.
+ nodata: Option<Vec<u8>>,
+}
+
+fn build_rasters(group_uri: &str, mode: Mode) -> Result<StructArray, ArrowError> {
+ let fs_path = group_uri_to_filesystem_path(group_uri)?;
+ let store = FilesystemStore::new(&fs_path).map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr filesystem store at {}: {e}",
+ fs_path.display()
+ )))
+ })?;
+ let storage: Arc<FilesystemStore> = Arc::new(store);
+
+ let group = Group::open(storage.clone(), "/").map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to open Zarr group at {group_uri}: {e}"
+ )))
+ })?;
+
+ let geo = GroupGeoMetadata::from_attributes(group.attributes())?;
+
+ let arrays = group.child_arrays().map_err(|e| {
+ ArrowError::ExternalError(Box::new(sedona_internal_datafusion_err!(
+ "failed to enumerate child arrays in {group_uri}: {e}"
+ )))
+ })?;
+ if arrays.is_empty() {
+ return Err(ArrowError::InvalidArgumentError(format!(
+ "Zarr group at {group_uri} has no child arrays"
+ )));
+ }
+
+ let array_infos = collect_array_infos(arrays)?;
+ validate_group_constraints(&array_infos)?;
+
+ // Spatial-dim resolution. Phase 1 supports two configurations:
+ // - dim_names ends with ["y", "x"] (canonical for georeferenced
+ // 2-D and time-series rasters); the spatial extent is the chunk's
+ // last two dims.
+ // - `spatial:dims` attribute on the group explicitly names them.
+ // Anything else errors with a clear message — silently picking dims
+ // would produce wrong per-row transforms.
+ let spatial_dim_indices =
+ resolve_spatial_dim_indices(&array_infos[0].dim_names, geo.spatial_dims.as_deref())?;
+ let spatial_dims_names: Vec<&str> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].dim_names[i].as_str())
+ .collect();
+ let chunk_spatial_shape: Vec<i64> = spatial_dim_indices
+ .iter()
+ .map(|&i| array_infos[0].chunk_shape[i] as i64)
+ .collect();
+
+ let group_transform = geo.transform.unwrap_or([0.0, 1.0, 0.0, 0.0, 0.0, -1.0]);
Review Comment:
added some validation and warning to help users who dont have full geozarr metadata.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3284213722
##########
rust/sedona-raster-zarr/Cargo.toml:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+[package]
+name = "sedona-raster-zarr"
+version.workspace = true
+license.workspace = true
+keywords.workspace = true
+categories.workspace = true
+homepage.workspace = true
+repository.workspace = true
+description.workspace = true
+readme.workspace = true
+edition.workspace = true
+rust-version.workspace = true
+
+[lints.clippy]
+result_large_err = "allow"
+
+[dependencies]
+arrow-array = { workspace = true }
+arrow-schema = { workspace = true }
+async-trait = { workspace = true }
+datafusion = { workspace = true, features = ["sql"] }
+datafusion-catalog = { workspace = true }
+datafusion-common = { workspace = true }
+datafusion-expr = { workspace = true }
+futures = { workspace = true }
+log = { workspace = true }
+sedona-common = { workspace = true }
+sedona-datasource = { workspace = true }
+sedona-raster = { workspace = true }
+sedona-schema = { workspace = true }
+serde = { workspace = true }
+serde_json = { workspace = true }
+zarrs = { workspace = true, features = ["filesystem", "gzip", "zstd", "crc32c", "sharding", "transpose"] }
+zarrs_filesystem = { workspace = true }
+
+# `blosc` is gated off the Windows MinGW (gnu) toolchain only: c-blosc
+# (statically linked) bundles its own `pthread_create` / `pthread_cond_*`
+# symbols, which conflict with rtools45's `libpthread.a` during the MinGW
+# link of the R `sedonadb.dll`. The MSVC toolchain (Python wheels, etc.)
+# uses a different pthread implementation and is fine.
+[target.'cfg(not(all(target_os = "windows", target_env = "gnu")))'.dependencies]
+zarrs = { workspace = true, features = ["blosc"] }
Review Comment:
I have r support for the plugin in my notes. Will eventually file against the repo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3284270540
##########
rust/sedona-datasource/src/provider.rs:
##########
@@ -110,3 +149,153 @@ impl ReadOptions<'_> for RecordBatchReaderTableOptions {
.await
}
}
+
+/// [`TableProvider`] that treats each input URI as one opaque object.
+///
+/// Built when [`ExternalFormatSpec::list_single_object`] is `true`. The
+/// listing layer is bypassed entirely: each URI is synthesised into a
+/// single-element [`PartitionedFile`] whose `object_meta.location` is
+/// the URL's path within the [`ObjectStoreUrl`]. The format's
+/// [`ExternalFileFormat::file_source`] then drives the same scan path
+/// used by [`ListingTable`] — projections, filter pushdown, and
+/// streaming behaviour are identical.
Review Comment:
it doesn't — the URL-table path goes through ListingTable, which has the same directory-vs-file mismatch the read_format path hit before. Same gap applies to pyogrio.
We can file a followup bug. It would be good to make this work but also we dont want to assume a zarr ends in `.zarr` (they often dont)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3277684409
##########
python/sedonadb/tests/test_funcs.py:
##########
@@ -24,3 +24,35 @@ def test_random_geometry(con):
# Ensure the output is reproducible
assert df.to_arrow_table() == df.to_arrow_table()
+
+
+def test_read_zarr(con, tmp_path):
+ # Skip cleanly if the optional `zarr` Python lib isn't installed —
+ # the binding is exercised by the Rust-side integration tests; this
+ # test only verifies the Python wrapper threads arguments through.
+ import pytest
+
+ zarr = pytest.importorskip("zarr")
+ np = pytest.importorskip("numpy")
+
+ # Build a 2x2 UInt8 array with two chunks, dim_names=["y","x"], inside
+ # a group at the temp path. Matches the minimal shape sd_read_zarr
+ # expects (2-D array with [y, x] suffix).
+ root = zarr.open_group(str(tmp_path), mode="w")
+ arr = root.create_array(
+ "temperature",
+ shape=(2, 2),
+ chunks=(1, 2),
+ dtype="uint8",
+ dimension_names=["y", "x"],
+ )
+ arr[:] = np.array([[10, 11], [20, 21]], dtype=np.uint8)
+
+ # Default-mode (InDb) read — every chunk materialized into the
+ # Arrow `data` column.
+ df = con.funcs.table.sd_read_zarr(f"file://{tmp_path}")
+ assert df.count() == 2
+
+ # Options thread through: rows_per_batch slices the output.
+ df = con.funcs.table.sd_read_zarr(f"file://{tmp_path}", rows_per_batch=1)
+ assert df.count() == 2
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3285704384
##########
rust/sedona/Cargo.toml:
##########
@@ -50,6 +50,7 @@ s2geography = ["dep:sedona-s2geography", "dep:sedona-spatial-join-geography", "s
tempfile = { workspace = true }
tokio = { workspace = true, features = ["rt-multi-thread"] }
rstest = { workspace = true }
+sedona-raster = { workspace = true }
Review Comment:
Is this still needed?
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3276956966
##########
python/sedonadb/tests/test_funcs.py:
##########
@@ -24,3 +24,35 @@ def test_random_geometry(con):
# Ensure the output is reproducible
assert df.to_arrow_table() == df.to_arrow_table()
+
+
+def test_read_zarr(con, tmp_path):
+ # Skip cleanly if the optional `zarr` Python lib isn't installed —
+ # the binding is exercised by the Rust-side integration tests; this
+ # test only verifies the Python wrapper threads arguments through.
+ import pytest
+
+ zarr = pytest.importorskip("zarr")
+ np = pytest.importorskip("numpy")
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3283689696
##########
rust/sedona-raster-zarr/src/udtf.rs:
##########
@@ -0,0 +1,614 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! `sd_read_zarr` table function — exposes Zarr-backed raster groups
+//! as SQL-queryable tables.
+//!
+//! ```sql
+//! SELECT * FROM sd_read_zarr('file:///path/to/datacube.zarr');
+//! SELECT count(*) FROM sd_read_zarr(
+//! 'file:///path/to/datacube.zarr',
+//! '{"rows_per_batch": 256}'
+//! );
+//! ```
+//!
+//! Returns a single-column table `raster: Raster` with one row per chunk
+//! position in the Zarr group's chunk grid. All existing `RS_*` UDFs
+//! operate on the column unchanged.
+//!
+//! `load_eager` defaults to `false` — every row has empty `data` and a
+//! chunk-anchor URI in `outdb_uri`. Setting `load_eager = true`
+//! currently errors because no `RS_EnsureLoaded` resolver is registered
+//! for the `zarr` format yet; once the resolver lands, `true` will
+//! trigger the planner to inject the async UDF over the scan output.
+//!
+//! Registration happens via [`register`]; the `sedonadb-zarr` Python
+//! package calls this from its session-setup helper. The `sedona` crate
+//! itself does not register the UDTF — keeping zarr functionality out
+//! of the default bootstrap.
+
+use std::any::Any;
+use std::sync::Arc;
+
+use arrow_schema::{DataType, Schema, SchemaRef};
+use async_trait::async_trait;
+use datafusion::catalog::{Session, TableFunctionImpl, TableProvider};
+use datafusion::common::Result;
+use datafusion::datasource::TableType;
+use datafusion::execution::context::{SessionContext, TaskContext};
+use datafusion::physical_expr::EquivalenceProperties;
+use datafusion::physical_plan::execution_plan::{Boundedness, EmissionType};
+use datafusion::physical_plan::expressions::Column;
+use datafusion::physical_plan::projection::ProjectionExec;
+use datafusion::physical_plan::stream::RecordBatchStreamAdapter;
+use datafusion::physical_plan::{
+ DisplayAs, DisplayFormatType, ExecutionPlan, Partitioning, PhysicalExpr, PlanProperties,
+ SendableRecordBatchStream,
+};
+use datafusion::prelude::Expr;
+use datafusion_common::{plan_err, DataFusionError, ScalarValue};
+use sedona_schema::datatypes::SedonaType;
+use serde::{Deserialize, Serialize};
+
+use crate::loader::ZarrChunkReader;
+
+/// Register the `sd_read_zarr` UDTF on a `SessionContext`. Called from
+/// the `sedonadb-zarr` Python package's `register(con)` entry point.
+pub fn register(ctx: &SessionContext) {
+ ctx.register_udtf("sd_read_zarr", Arc::new(ZarrReadFunction::default()));
+}
Review Comment:
done - this was dead code from an early approach I was testing anyway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3283703986
##########
python/sedonadb-zarr/python/sedonadb_zarr/__init__.py:
##########
@@ -0,0 +1,127 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+"""Zarr support for SedonaDB.
+
+Activate by calling :func:`register` on a SedonaDB connection. After
+registration, two surfaces work:
+
+1. ``con.sql("SELECT * FROM sd_read_zarr('s3://...')")`` — SQL UDTF.
+2. ``con.read_format(ZarrFormatSpec(), uri)`` — DataFrame API via
+ ``ExternalFormatSpec``.
+
+>>> import sedonadb
+>>> import sedonadb_zarr
+>>> con = sedonadb.connect()
+>>> sedonadb_zarr.register(con)
+>>> con.sql("SELECT count(*) FROM sd_read_zarr('file:///path/to/foo.zarr')").show() # doctest: +SKIP
+
+The plugin is opt-in: SedonaDB itself does not bundle Zarr support, so
+applications that don't import ``sedonadb_zarr`` pay no zarr build or
+runtime cost.
+"""
+
+import json
+from typing import Any, Mapping, Optional
+
+from sedonadb.datasource import ExternalFormatSpec
+
+from sedonadb_zarr._zarr_lib import PyZarrChunkReader, zarr_udtf_capsule
+
+
+def register(con) -> None:
+ """Attach Zarr SQL support to a SedonaDB connection.
+
+ After this call, ``con.sql("SELECT * FROM sd_read_zarr(...)")``
+ works. Idempotent — calling twice on the same connection re-
+ registers the UDTF without error.
+
+ Parameters
+ ----------
+ con
+ A ``sedonadb`` ``Context`` (the object returned by
+ ``sedonadb.connect()``). Internally, this function extracts
+ the underlying ``InternalContext`` PyO3 handle and registers
+ the UDTF on its DataFusion ``SessionContext`` via a
+ ``PyCapsule`` handoff — the only viable cross-extension
+ transport for the UDTF trait object.
+ """
+ internal_ctx = getattr(con, "_impl", None)
+ if internal_ctx is None:
+ raise TypeError(
+ "sedonadb_zarr.register: could not locate the InternalContext on "
+ f"{type(con).__name__}; expected attribute `_impl`."
+ )
+ capsule = zarr_udtf_capsule()
+ internal_ctx.register_udtf_capsule("sd_read_zarr", capsule)
+
+
+class ZarrFormatSpec(ExternalFormatSpec):
+ """`ExternalFormatSpec` for Zarr groups.
+
+ Use with ``con.read_format(spec, uri)``:
+
+ >>> con.read_format(ZarrFormatSpec(), "file:///path/to/foo.zarr") # doctest: +SKIP
+
+ Supported options (via :meth:`with_options`):
+
+ - ``load_eager`` (``bool``) — ``False`` (default) emits chunk-anchor
+ URIs only; ``True`` currently errors pending the async resolver.
+ - ``arrays`` (``list[str]`` or JSON-string) — explicit subset of
+ group arrays to read.
+ """
+
+ def __init__(self, options: Optional[Mapping[str, Any]] = None):
+ self._options: dict = dict(options) if options else {}
+
+ @property
+ def extension(self) -> str:
+ return ".zarr"
+
+ @property
+ def list_single_object(self) -> bool:
+ # A Zarr group is a directory, not a file. The DataFusion
+ # listing layer would enumerate its contents (zarr.json, chunk
+ # shards, ...), none of which carry the `.zarr` extension. The
+ # Rust `SingleObjectExternalTable` path skips listing and
+ # hands the URI straight to `open_reader`.
+ return True
+
+ def with_options(self, options: Mapping[str, Any]) -> "ZarrFormatSpec":
+ merged = {**self._options, **options}
+ return ZarrFormatSpec(merged)
+
+ def open_reader(self, args: Any) -> PyZarrChunkReader:
+ uri = args.src.to_url()
+ if uri is None:
+ raise ValueError(
+ "ZarrFormatSpec: could not resolve a URL from the source object"
+ )
+ if self._options.get("load_eager"):
+ raise NotImplementedError(
+ "ZarrFormatSpec: load_eager=True is not yet supported. "
+ "Pixel-byte materialisation lands with the async RS_EnsureLoaded "
+ "resolver."
+ )
Review Comment:
fair, done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3283710721
##########
rust/sedona-raster-zarr/src/format_spec.rs:
##########
@@ -0,0 +1,135 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! `ExternalFormatSpec` impl for Zarr groups.
+//!
+//! Wraps [`ZarrChunkReader`] with the standard SedonaDB datasource API
+//! so users can read Zarr groups via `con.read_format(spec, uri)` or
+//! through `ListingTable`-style file discovery.
+
+use std::{collections::HashMap, sync::Arc};
+
+use arrow_array::RecordBatchReader;
+use arrow_schema::Schema;
+use async_trait::async_trait;
+use datafusion_common::{DataFusionError, Result};
+use sedona_datasource::spec::{ExternalFormatSpec, Object, OpenReaderArgs};
+use sedona_schema::datatypes::SedonaType;
+
+use crate::loader::ZarrChunkReader;
+
+/// `ExternalFormatSpec` implementation for Zarr groups.
+///
+/// Configurable via [`with_options`](ExternalFormatSpec::with_options):
+/// - `load_eager`: boolean. `false` (default) emits chunk-anchor URIs
+/// only. `true` currently errors — pixel-byte materialisation is
+/// pending the async `RS_EnsureLoaded` resolver.
+/// - `arrays`: JSON array of strings, e.g. `'["temperature","pressure"]'`.
+/// Names a subset of arrays in the group to read; defaults to every
+/// multi-dimensional array (1-D coord variables auto-skipped).
+#[derive(Debug, Clone, Default)]
+pub struct ZarrFormatSpec {
+ load_eager: bool,
+ arrays: Option<Vec<String>>,
+}
+
+impl ZarrFormatSpec {
+ pub fn new() -> Self {
+ Self::default()
+ }
+}
+
+#[async_trait]
+impl ExternalFormatSpec for ZarrFormatSpec {
+ async fn infer_schema(&self, _location: &Object) -> Result<Schema> {
+ // The Zarr loader always produces the canonical single-column
+ // Raster schema. No I/O needed to know that.
+ let field = SedonaType::Raster
+ .to_storage_field("raster", true)
+ .map_err(|e| DataFusionError::External(Box::new(e)))?;
+ Ok(Schema::new(vec![field]))
+ }
+
+ async fn open_reader(
+ &self,
+ args: &OpenReaderArgs,
+ ) -> Result<Box<dyn RecordBatchReader + Send>> {
+ if self.load_eager {
+ return Err(DataFusionError::Plan(
+ "ZarrFormatSpec: load_eager = true is not yet supported. \
+ Pixel-byte materialisation will be wired up when the async \
+ RS_EnsureLoaded resolver lands."
+ .into(),
+ ));
+ }
Review Comment:
done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3283694466
##########
rust/sedona-raster-zarr/Cargo.toml:
##########
@@ -0,0 +1,62 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+[package]
+name = "sedona-raster-zarr"
+version.workspace = true
+license.workspace = true
+keywords.workspace = true
+categories.workspace = true
+homepage.workspace = true
+repository.workspace = true
+description.workspace = true
+readme.workspace = true
+edition.workspace = true
+rust-version.workspace = true
+
+[lints.clippy]
+result_large_err = "allow"
+
+[dependencies]
+arrow-array = { workspace = true }
+arrow-schema = { workspace = true }
+async-trait = { workspace = true }
+datafusion = { workspace = true, features = ["sql"] }
Review Comment:
done - easy once I got rid of SessionContext
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3277662805
##########
rust/sedona/Cargo.toml:
##########
@@ -50,6 +50,14 @@ s2geography = ["dep:sedona-s2geography", "dep:sedona-spatial-join-geography", "s
tempfile = { workspace = true }
tokio = { workspace = true, features = ["rt-multi-thread"] }
rstest = { workspace = true }
+sedona-raster = { workspace = true }
+zarrs = { workspace = true, features = ["filesystem", "gzip", "zstd", "crc32c", "sharding", "transpose"] }
+zarrs_filesystem = { workspace = true }
+
+# See sedona-raster-zarr/Cargo.toml — `blosc` is gated off Windows to avoid
+# a libwinpthreads/c-blosc symbol collision in the R `sedonadb.dll` link.
+[target.'cfg(not(target_os = "windows"))'.dev-dependencies]
+zarrs = { workspace = true, features = ["blosc"] }
Review Comment:
I have a pluggable approach. should work with the outdb loader as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3276769423
##########
rust/sedona-raster-zarr/src/geozarr.rs:
##########
@@ -0,0 +1,228 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+//! GeoZarr attribute parsing for CRS and affine transform.
+//!
+//! Reads the `proj:*` (CRS) and `spatial:*` (transform / spatial dim
+//! mapping) attribute conventions from a Zarr group's attributes, mapping
+//! them onto SedonaDB's per-raster `crs` and `transform` fields.
+//!
+//! Attributes live at the group level and are inherited by every array.
+//! Per-array overrides are rejected by the group-constraint validator
+//! (see `loader`).
+
+use arrow_schema::ArrowError;
+
+/// Per-group geo metadata distilled from `proj:*` / `spatial:*` attributes.
+#[derive(Debug, Clone, PartialEq)]
+pub struct GroupGeoMetadata {
+ /// CRS string in PROJ or WKT format (whichever the group declared).
+ /// `None` if no `proj:wkt2` / `proj:projjson` / `proj:epsg` attribute
+ /// is present on the group.
+ pub crs: Option<String>,
+ /// Affine transform in GDAL GeoTransform order:
+ /// `[origin_x, scale_x, skew_x, origin_y, skew_y, scale_y]`. `None`
+ /// when no `spatial:transform` attribute is present.
+ pub transform: Option<[f64; 6]>,
Review Comment:
yeah the ND refactor lays them out in the arrow as a single array. we can add methods to the trait that indexes into the array for this.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sd_read_zarr UDTF [sedona-db]
Posted by "james-willis (via GitHub)" <gi...@apache.org>.
james-willis commented on PR #858:
URL: https://github.com/apache/sedona-db/pull/858#issuecomment-4521298013
sorry dewey, I havent finished dealing with your last round of comments yet!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(raster-zarr): sedona-raster-zarr crate + sedona-zarr plugin [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #858:
URL: https://github.com/apache/sedona-db/pull/858#discussion_r3291855587
##########
python/sedonadb/python/sedonadb/context.py:
##########
@@ -333,6 +346,50 @@ def read_pyogrio(
self.options,
)
+ def read_format(
+ self,
+ spec: "ExternalFormatSpec",
+ table_paths: Union[str, Path, Iterable[str]],
+ check_extension: bool = False,
+ ) -> DataFrame:
+ """Read one or more paths using a Python-defined `ExternalFormatSpec`.
Review Comment:
This is great!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@sedona.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org