From 95f9ccbe83791eb7940001e1190e61d34f7975d2 Mon Sep 17 00:00:00 2001 From: d33bs Date: Sun, 29 Mar 2026 11:52:58 -0600 Subject: [PATCH 1/2] ome-arrow compat --- ARCHITECTURE.md | 4 +- README.md | 16 ++ ROADMAP.md | 2 +- scripts/create_sample_zarr.py | 3 + src/duckdb_zarr_extension.cpp | 1 + src/include/zarr_metadata.hpp | 1 + src/zarr_metadata.cpp | 228 +++++++++++++++++- test/README.md | 1 + test/data/simple_v3.zarr/fortran_v3/zarr.json | 2 +- test/data/simple_v3.zarr/mask_v3/zarr.json | 2 +- .../simple_v3.zarr/temperature_v3/zarr.json | 2 +- test/python_smoke.py | 12 + test/sql/duckdb_zarr.test | 50 ++++ 13 files changed, 306 insertions(+), 18 deletions(-) diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index 8ceb4e2..c4f050c 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -29,8 +29,8 @@ ## Current MVP - Implemented now: local Zarr v2 and v3 metadata discovery, Zarr v2 consolidated `.zmetadata` support, gzip/uncompressed numeric chunk decode, and DuckDB table functions (`zarr_groups`, `zarr_arrays`, `zarr_chunks`, `zarr_cells`, `zarr`) with projection-aware scans and chunk pruning. -- Partially implemented: the Store Adapter includes practical remote support for consolidated HTTP/S3-style Zarr v2 stores via DuckDB `httpfs`, and the Codec Pipeline now handles the current v3 subset of `bytes`, optional `gzip`, constrained `transpose`, and `sharding_indexed` with inner Blosc (`zstd`) for common OME-Zarr image and label data. -- Planned: the Arrow Materializer, broader codec chains beyond the current Blosc path, richer convention adapters, and the fuller target flow described below. +- Partially implemented: the Store Adapter includes practical remote support for consolidated HTTP/S3-style Zarr v2 stores via DuckDB `httpfs`, the Codec Pipeline now handles the current v3 subset of `bytes`, optional `gzip`, constrained `transpose`, and `sharding_indexed` with inner Blosc (`zstd`) for common OME-Zarr image and label data, and a thin `ome_arrow(...)` adapter exposes v3 arrays with dimension-aware relational columns. +- Planned: the Arrow Materializer, broader codec chains beyond the current Blosc path, richer convention adapters beyond the current `ome_arrow(...)` layer, and the fuller target flow described below. ## Flow diff --git a/README.md b/README.md index a35540f..d5891e4 100644 --- a/README.md +++ b/README.md @@ -12,6 +12,8 @@ Today’s extension provides metadata and relational scan table functions for lo - `zarr(path)` for a simple array overview - `zarr(path, array_path)` and `zarr(path, array_path, version_override)` as convenience aliases for `zarr_cells(...)` +- `ome_arrow(path)` for a Zarr v3 / OME-Arrow-friendly array overview +- `ome_arrow(path, array_path)` and `ome_arrow(path, array_path, version_override)` for Zarr v3 cell scans with metadata-derived dimension names such as `c`, `z`, `y`, and `x` - `zarr_groups(path)` and `zarr_groups(path, version_override)` - `zarr_arrays(path)` and `zarr_arrays(path, version_override)` - `zarr_chunks(path)` and `zarr_chunks(path, version_override)` @@ -29,6 +31,7 @@ The MVP currently supports: - Remote `http://`, `https://`, and `s3://` stores when DuckDB `httpfs` is available and the store is consolidated - OME-Zarr-style multiscales groups built on Zarr v2, including level arrays such as `0` - Real-world OME-Zarr v3 metadata discovery for stores such as image-label hierarchies and multiscales groups +- A lightweight OME-Arrow compatibility layer for Zarr v3 arrays through `ome_arrow(...)` - Group enumeration from `.zgroup` - Array enumeration from `.zarray` and `zarr.json` - Automatic v2/v3 detection for local stores, with explicit `version_override` support (`auto`, `v2`, `v3`) on the lower-level SQL functions @@ -54,6 +57,7 @@ The MVP does not yet support: - Non-consolidated remote store discovery - Remote hierarchical discovery for non-consolidated Zarr v3 group stores - Arrow materialization +- Writing formal OME-Arrow artifacts from DuckDB ## Quick Start @@ -136,9 +140,14 @@ SELECT * FROM zarr_cells('test/data/simple_v2.zarr', 'temperature'); SELECT * FROM zarr('test/data/simple_v2.zarr'); SELECT * FROM zarr_arrays('test/data/simple_v3.zarr', 'v3'); SELECT * FROM zarr('test/data/simple_v3.zarr', 'temperature_v3', 'v3'); +SELECT * FROM ome_arrow('test/data/simple_v3.zarr'); +SELECT * FROM ome_arrow('test/data/simple_v3.zarr', 'temperature_v3'); SELECT * FROM zarr('test/data/ome_example.ome.zarr', '0'); SELECT * FROM zarr('test/data/idr0062A/6001240_labels.zarr'); SELECT * FROM zarr('test/data/idr0062A/6001240_labels.zarr', '0') LIMIT 5; +SELECT * +FROM ome_arrow('test/data/idr0062A/6001240_labels.zarr', '0') +LIMIT 5; SELECT SUM(value) FROM zarr('test/data/idr0062A/6001240_labels.zarr', 'labels/0/0') WHERE dim_0 = 0 AND dim_1 = 0 AND dim_2 < 4 AND dim_3 < 4; @@ -156,6 +165,13 @@ Version detection notes: - lower-level functions accept an optional `version_override` argument: `auto`, `v2`, or `v3` - the convenience cell entrypoint also accepts an override as `zarr(path, array_path, version_override)` - explicit override is useful when a path is ambiguous or when you want failures to be version-specific +- `ome_arrow(...)` is intentionally Zarr v3-only because OME-Arrow compatibility in this repo is built on v3 `dimension_names` + +OME-Arrow adapter notes: + +- `ome_arrow(path)` returns a v3 array overview with `dimension_names` +- `ome_arrow(path, array_path)` uses those names as relational column names when scanning cells +- this is a compatibility layer for moving data into Arrow-friendly relational form, not a full OME-Arrow file writer ## Install Like An Extension diff --git a/ROADMAP.md b/ROADMAP.md index a997c86..2d3f443 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -28,5 +28,5 @@ ## [ ] Phase 5: Advanced - [ ] chunk_values -- [ ] convention adapters +- [~] convention adapters (`ome_arrow(...)` compatibility layer added for Zarr v3) - [ ] parallel decode diff --git a/scripts/create_sample_zarr.py b/scripts/create_sample_zarr.py index fa0e90e..78b71ee 100644 --- a/scripts/create_sample_zarr.py +++ b/scripts/create_sample_zarr.py @@ -208,6 +208,7 @@ def create_simple_v3_store() -> None: "zarr_format": 3, "node_type": "array", "shape": [3, 2], + "dimension_names": ["y", "x"], "data_type": "float64", "chunk_grid": {"name": "regular", "configuration": {"chunk_shape": [2, 2]}}, "chunk_key_encoding": {"name": "default", "configuration": {"separator": "/"}}, @@ -226,6 +227,7 @@ def create_simple_v3_store() -> None: "zarr_format": 3, "node_type": "array", "shape": [2, 3], + "dimension_names": ["y", "x"], "data_type": "bool", "chunk_grid": {"name": "regular", "configuration": {"chunk_shape": [2, 2]}}, "chunk_key_encoding": {"name": "v2", "configuration": {"separator": "."}}, @@ -240,6 +242,7 @@ def create_simple_v3_store() -> None: "zarr_format": 3, "node_type": "array", "shape": [2, 2], + "dimension_names": ["row", "col"], "data_type": "int32", "chunk_grid": {"name": "regular", "configuration": {"chunk_shape": [2, 2]}}, "chunk_key_encoding": {"name": "default", "configuration": {"separator": "/"}}, diff --git a/src/duckdb_zarr_extension.cpp b/src/duckdb_zarr_extension.cpp index a7e4cf7..d5e3419 100644 --- a/src/duckdb_zarr_extension.cpp +++ b/src/duckdb_zarr_extension.cpp @@ -16,6 +16,7 @@ static void LoadInternal(ExtensionLoader &loader) { loader.RegisterFunction(ZarrMetadata::GetCellsFunction()); loader.RegisterFunction(ZarrMetadata::GetZarrFunction()); loader.RegisterFunction(ZarrMetadata::GetZarrCellsAliasFunction()); + loader.RegisterFunction(ZarrMetadata::GetOmeArrowFunction()); } void DuckdbZarrExtension::Load(ExtensionLoader &loader) { diff --git a/src/include/zarr_metadata.hpp b/src/include/zarr_metadata.hpp index 9f0e8e0..cb6bb7f 100644 --- a/src/include/zarr_metadata.hpp +++ b/src/include/zarr_metadata.hpp @@ -13,6 +13,7 @@ class ZarrMetadata { static TableFunctionSet GetCellsFunction(); static TableFunctionSet GetZarrFunction(); static TableFunctionSet GetZarrCellsAliasFunction(); + static TableFunctionSet GetOmeArrowFunction(); }; } // namespace duckdb diff --git a/src/zarr_metadata.cpp b/src/zarr_metadata.cpp index b8ca2c9..cac1924 100644 --- a/src/zarr_metadata.cpp +++ b/src/zarr_metadata.cpp @@ -15,9 +15,11 @@ #include "zstd.h" #include +#include #include #include #include +#include namespace duckdb { @@ -70,6 +72,7 @@ struct ZarrArrayEntry { string inner_compressor; string index_codecs; string index_location; + vector dimension_names; }; struct ZarrChunkEntry { @@ -273,6 +276,27 @@ static vector RequireInt64Array(yyjson_val *obj, const char *key) { return result; } +static vector OptionalStringArray(yyjson_val *obj, const char *key) { + auto value = yyjson_obj_get(obj, key); + if (!value || yyjson_is_null(value)) { + return {}; + } + if (!yyjson_is_arr(value)) { + throw InvalidInputException("Zarr metadata key \"%s\" is not an array", key); + } + vector result; + result.reserve(unsafe_yyjson_get_len(value)); + yyjson_val *element; + yyjson_arr_iter iter = yyjson_arr_iter_with(value); + while ((element = yyjson_arr_iter_next(&iter))) { + if (!yyjson_is_str(element)) { + throw InvalidInputException("Zarr metadata key \"%s\" contains a non-string array element", key); + } + result.emplace_back(unsafe_yyjson_get_str(element), unsafe_yyjson_get_len(element)); + } + return result; +} + static yyjson_val *RequireObject(yyjson_val *obj, const char *key) { auto value = RequireObjectKey(obj, key); if (!yyjson_is_obj(value)) { @@ -322,6 +346,63 @@ static string NormalizeStorePath(FileSystem &fs, const string &path) { return fs.ExpandPath(path); } +static vector NormalizeDimensionNames(const vector &dimension_names, idx_t rank) { + if (dimension_names.empty()) { + return {}; + } + if (dimension_names.size() != rank) { + throw InvalidInputException("Zarr v3 dimension_names rank does not match array rank"); + } + vector result; + result.reserve(rank); + std::unordered_set seen; + for (idx_t i = 0; i < rank; i++) { + auto name = dimension_names[i]; + StringUtil::Trim(name); + name = StringUtil::Lower(name); + if (name.empty()) { + throw InvalidInputException("Zarr v3 dimension_names cannot contain empty names"); + } + for (auto &ch : name) { + if (!(std::isalnum(static_cast(ch)) || ch == '_')) { + ch = '_'; + } + } + if (std::isdigit(static_cast(name[0]))) { + name = "dim_" + name; + } + if (name.empty()) { + name = "dim_" + std::to_string(i); + } + if (!seen.insert(name).second) { + throw InvalidInputException("Zarr v3 dimension_names must be unique"); + } + result.push_back(std::move(name)); + } + return result; +} + +static vector GetOutputDimensionNames(const ZarrArrayEntry &array) { + if (!array.dimension_names.empty()) { + return array.dimension_names; + } + vector names; + names.reserve(NumericCast(array.rank)); + for (idx_t dim = 0; dim < NumericCast(array.rank); dim++) { + names.push_back("dim_" + std::to_string(dim)); + } + return names; +} + +static vector ToVarcharValues(const vector &values) { + vector result; + result.reserve(values.size()); + for (auto &value : values) { + result.emplace_back(Value(value)); + } + return result; +} + static ZarrVersionOverride ParseZarrVersionOverride(const Value &value, const string &function_name) { if (value.IsNull()) { return ZarrVersionOverride::AUTO; @@ -1599,6 +1680,7 @@ static ZarrArrayEntry ParseArrayMetadataObjectV3(yyjson_val *root, const string throw InvalidInputException("Expected a Zarr v3 array metadata document at %s", metadata_path); } auto shape = RequireInt64Array(root, "shape"); + auto dimension_names = NormalizeDimensionNames(OptionalStringArray(root, "dimension_names"), shape.size()); auto chunk_grid = RequireObject(root, "chunk_grid"); auto chunk_grid_name = GetExtensionName(chunk_grid, metadata_path, "chunk_grid"); if (chunk_grid_name != "regular") { @@ -1653,13 +1735,15 @@ static ZarrArrayEntry ParseArrayMetadataObjectV3(yyjson_val *root, const string bool little_endian = HostIsLittleEndian(); ParseV3ShardingCodec(first_codec, metadata_path, chunks, compressor, supports_cells, cells_error, inner_chunks, inner_compressor, index_codecs, index_location, little_endian); - return MakeArrayEntry(store_path, relative_path, zarr_format, NumericCast(shape.size()), - std::move(shape), std::move(chunks), - ParseV3DataType(root, metadata_path, little_endian), "C", std::move(compressor), - JsonToString(yyjson_obj_get(root, "fill_value")), std::move(chunk_key_encoding_name), - std::move(dimension_separator), metadata_path, supports_cells, std::move(cells_error), - true, {}, std::move(inner_chunks), std::move(inner_compressor), - std::move(index_codecs), std::move(index_location)); + auto entry = MakeArrayEntry(store_path, relative_path, zarr_format, NumericCast(shape.size()), + std::move(shape), std::move(chunks), + ParseV3DataType(root, metadata_path, little_endian), "C", + std::move(compressor), JsonToString(yyjson_obj_get(root, "fill_value")), + std::move(chunk_key_encoding_name), std::move(dimension_separator), metadata_path, + supports_cells, std::move(cells_error), true, {}, std::move(inner_chunks), + std::move(inner_compressor), std::move(index_codecs), std::move(index_location)); + entry.dimension_names = std::move(dimension_names); + return entry; } } bool saw_bytes = false; @@ -1756,11 +1840,14 @@ static ZarrArrayEntry ParseArrayMetadataObjectV3(yyjson_val *root, const string cells_error = "zarr_cells does not yet support the Zarr v3 codec pipeline for this array"; } - return MakeArrayEntry(store_path, relative_path, zarr_format, NumericCast(shape.size()), std::move(shape), - std::move(chunks), ParseV3DataType(root, metadata_path, little_endian), std::move(order), - std::move(compressor), JsonToString(yyjson_obj_get(root, "fill_value")), - std::move(chunk_key_encoding_name), std::move(dimension_separator), metadata_path, - supports_cells, std::move(cells_error), false, {}, {}, "", "", ""); + auto entry = MakeArrayEntry(store_path, relative_path, zarr_format, NumericCast(shape.size()), + std::move(shape), std::move(chunks), + ParseV3DataType(root, metadata_path, little_endian), std::move(order), + std::move(compressor), JsonToString(yyjson_obj_get(root, "fill_value")), + std::move(chunk_key_encoding_name), std::move(dimension_separator), metadata_path, + supports_cells, std::move(cells_error), false, {}, {}, "", "", ""); + entry.dimension_names = std::move(dimension_names); + return entry; } static string MetadataPathForKey(const string &store_path, const string &key) { @@ -2130,6 +2217,27 @@ static unique_ptr BindArraysInternal(ClientContext &context, const return make_uniq>(std::move(arrays)); } +static unique_ptr BindOmeArrowArraysInternal(ClientContext &context, const string &store_path, + vector &return_types, + vector &names) { + vector groups; + vector arrays; + vector chunks; + DiscoverStore(context, store_path, groups, arrays, chunks, false, ZarrVersionOverride::V3); + names = {"store_path", "array_path", "zarr_format", "rank", "shape", + "chunk_shape", "dtype", "dimension_names", "metadata_path"}; + return_types = {LogicalType::VARCHAR, + LogicalType::VARCHAR, + LogicalType::BIGINT, + LogicalType::BIGINT, + LogicalType::LIST(LogicalType::BIGINT), + LogicalType::LIST(LogicalType::BIGINT), + LogicalType::VARCHAR, + LogicalType::LIST(LogicalType::VARCHAR), + LogicalType::VARCHAR}; + return make_uniq>(std::move(arrays)); +} + static unique_ptr BindArrays(ClientContext &context, TableFunctionBindInput &input, vector &return_types, vector &names) { if (input.inputs.empty() || input.inputs[0].IsNull()) { @@ -2200,6 +2308,44 @@ static unique_ptr BindCellsInternal(ClientContext &context, const return make_uniq(cell_array, std::move(chunks)); } +static unique_ptr BindCellsWithDimensionNamesInternal(ClientContext &context, const string &store_path, + const string &array_path, + ZarrVersionOverride version_override, + vector &return_types, + vector &names) { + if (version_override == ZarrVersionOverride::V2) { + throw BinderException("ome_arrow only supports Zarr v3 stores"); + } + vector groups; + vector arrays; + vector chunks; + DiscoverStore(context, store_path, groups, arrays, chunks, false, ZarrVersionOverride::V3); + auto &array = FindArrayEntry(arrays, array_path); + if (!array.supports_cells) { + throw InvalidInputException("%s: %s", array.cells_error, array.array_path); + } + auto cell_array = array; + auto dtype = ParseNumericDType(array.dtype); + chunks = cell_array.is_sharding_indexed + ? GenerateCellChunkEntries(FileSystem::GetFileSystem(context), array.store_path, array, + HasMaterializedFillValue(array)) + : GenerateChunkEntries(FileSystem::GetFileSystem(context), cell_array.store_path, cell_array, + HasMaterializedFillValue(cell_array)); + if (cell_array.is_sharding_indexed) { + cell_array.storage_chunks = cell_array.chunks; + cell_array.chunks = cell_array.inner_chunks; + } + + names = GetOutputDimensionNames(cell_array); + return_types.reserve(cell_array.rank + 1); + for (idx_t dim = 0; dim < NumericCast(cell_array.rank); dim++) { + return_types.push_back(LogicalType::BIGINT); + } + names.push_back("value"); + return_types.push_back(dtype.logical_type); + return make_uniq(cell_array, std::move(chunks)); +} + static unique_ptr BindCells(ClientContext &context, TableFunctionBindInput &input, vector &return_types, vector &names) { if (input.inputs.size() < 2 || input.inputs[0].IsNull() || input.inputs[1].IsNull()) { @@ -2221,6 +2367,26 @@ static unique_ptr BindZarr(ClientContext &context, TableFunctionBi names); } +static unique_ptr BindOmeArrow(ClientContext &context, TableFunctionBindInput &input, + vector &return_types, vector &names) { + if (input.inputs.empty() || input.inputs[0].IsNull()) { + throw BinderException("ome_arrow requires a non-NULL store path"); + } + return BindOmeArrowArraysInternal(context, StringValue::Get(input.inputs[0]), return_types, names); +} + +static unique_ptr BindOmeArrowCells(ClientContext &context, TableFunctionBindInput &input, + vector &return_types, vector &names) { + if (input.inputs.size() < 2 || input.inputs[0].IsNull() || input.inputs[1].IsNull()) { + throw BinderException("ome_arrow requires a non-NULL store path and array path"); + } + auto version_override = + input.inputs.size() > 2 ? ParseZarrVersionOverride(input.inputs[2], "ome_arrow") : ZarrVersionOverride::V3; + return BindCellsWithDimensionNamesInternal(context, StringValue::Get(input.inputs[0]), + NormalizeArrayPath(StringValue::Get(input.inputs[1])), + version_override, return_types, names); +} + static void ScanGroups(ClientContext &, TableFunctionInput &data_p, DataChunk &output) { auto &state = data_p.global_state->Cast(); auto &entries = data_p.bind_data->Cast>().entries; @@ -2259,6 +2425,26 @@ static void ScanArrays(ClientContext &, TableFunctionInput &data_p, DataChunk &o output.SetCardinality(count); } +static void ScanOmeArrowArrays(ClientContext &, TableFunctionInput &data_p, DataChunk &output) { + auto &state = data_p.global_state->Cast(); + auto &entries = data_p.bind_data->Cast>().entries; + idx_t count = 0; + while (state.offset < entries.size() && count < STANDARD_VECTOR_SIZE) { + auto &entry = entries[state.offset++]; + output.SetValue(0, count, Value(entry.store_path)); + output.SetValue(1, count, Value(entry.array_path)); + output.SetValue(2, count, Value::BIGINT(entry.zarr_format)); + output.SetValue(3, count, Value::BIGINT(NumericCast(entry.shape.size()))); + output.SetValue(4, count, Value::LIST(LogicalType::BIGINT, ToBigIntValues(entry.shape))); + output.SetValue(5, count, Value::LIST(LogicalType::BIGINT, ToBigIntValues(entry.chunks))); + output.SetValue(6, count, Value(entry.dtype)); + output.SetValue(7, count, Value::LIST(LogicalType::VARCHAR, ToVarcharValues(GetOutputDimensionNames(entry)))); + output.SetValue(8, count, Value(entry.metadata_path)); + count++; + } + output.SetCardinality(count); +} + static void ScanChunks(ClientContext &, TableFunctionInput &data_p, DataChunk &output) { auto &state = data_p.global_state->Cast(); auto &entries = data_p.bind_data->Cast>().entries; @@ -2394,4 +2580,22 @@ TableFunctionSet ZarrMetadata::GetZarrCellsAliasFunction() { return set; } +TableFunctionSet ZarrMetadata::GetOmeArrowFunction() { + TableFunctionSet set("ome_arrow"); + set.AddFunction(TableFunction("ome_arrow", {LogicalType::VARCHAR}, ScanOmeArrowArrays, BindOmeArrow, ZarrInit)); + TableFunction base("ome_arrow", {LogicalType::VARCHAR, LogicalType::VARCHAR}, ScanCells, BindOmeArrowCells, + InitCells); + base.projection_pushdown = true; + base.filter_pushdown = true; + base.filter_prune = true; + set.AddFunction(base); + TableFunction with_override("ome_arrow", {LogicalType::VARCHAR, LogicalType::VARCHAR, LogicalType::VARCHAR}, + ScanCells, BindOmeArrowCells, InitCells); + with_override.projection_pushdown = true; + with_override.filter_pushdown = true; + with_override.filter_prune = true; + set.AddFunction(with_override); + return set; +} + } // namespace duckdb diff --git a/test/README.md b/test/README.md index 2590db3..49b26d4 100644 --- a/test/README.md +++ b/test/README.md @@ -8,6 +8,7 @@ The current suite covers the metadata and `zarr_cells` slices: - `zarr_arrays(path)` - `zarr_chunks(path)` - `zarr_cells(path, array_path)` +- `ome_arrow(path)` and `ome_arrow(path, array_path)` for the Zarr v3 compatibility layer - `test/python_smoke.py` for a Python-side smoke example using DuckDB 1.5.0 - real OME-Zarr v3 sharded/Blosc cell scans against `test/data/idr0062A/6001240_labels.zarr` diff --git a/test/data/simple_v3.zarr/fortran_v3/zarr.json b/test/data/simple_v3.zarr/fortran_v3/zarr.json index 07e5575..7c172e3 100644 --- a/test/data/simple_v3.zarr/fortran_v3/zarr.json +++ b/test/data/simple_v3.zarr/fortran_v3/zarr.json @@ -1 +1 @@ -{"zarr_format":3,"node_type":"array","shape":[2,2],"data_type":"int32","chunk_grid":{"name":"regular","configuration":{"chunk_shape":[2,2]}},"chunk_key_encoding":{"name":"default","configuration":{"separator":"/"}},"fill_value":0,"codecs":[{"name":"transpose","configuration":{"order":[1,0]}},{"name":"bytes","configuration":{"endian":"little"}}]} +{"zarr_format":3,"node_type":"array","shape":[2,2],"dimension_names":["row","col"],"data_type":"int32","chunk_grid":{"name":"regular","configuration":{"chunk_shape":[2,2]}},"chunk_key_encoding":{"name":"default","configuration":{"separator":"/"}},"fill_value":0,"codecs":[{"name":"transpose","configuration":{"order":[1,0]}},{"name":"bytes","configuration":{"endian":"little"}}]} diff --git a/test/data/simple_v3.zarr/mask_v3/zarr.json b/test/data/simple_v3.zarr/mask_v3/zarr.json index 2585735..194e091 100644 --- a/test/data/simple_v3.zarr/mask_v3/zarr.json +++ b/test/data/simple_v3.zarr/mask_v3/zarr.json @@ -1 +1 @@ -{"zarr_format":3,"node_type":"array","shape":[2,3],"data_type":"bool","chunk_grid":{"name":"regular","configuration":{"chunk_shape":[2,2]}},"chunk_key_encoding":{"name":"v2","configuration":{"separator":"."}},"fill_value":false,"codecs":[{"name":"bytes"}]} +{"zarr_format":3,"node_type":"array","shape":[2,3],"dimension_names":["y","x"],"data_type":"bool","chunk_grid":{"name":"regular","configuration":{"chunk_shape":[2,2]}},"chunk_key_encoding":{"name":"v2","configuration":{"separator":"."}},"fill_value":false,"codecs":[{"name":"bytes"}]} diff --git a/test/data/simple_v3.zarr/temperature_v3/zarr.json b/test/data/simple_v3.zarr/temperature_v3/zarr.json index 38a2b82..df15347 100644 --- a/test/data/simple_v3.zarr/temperature_v3/zarr.json +++ b/test/data/simple_v3.zarr/temperature_v3/zarr.json @@ -1 +1 @@ -{"zarr_format":3,"node_type":"array","shape":[3,2],"data_type":"float64","chunk_grid":{"name":"regular","configuration":{"chunk_shape":[2,2]}},"chunk_key_encoding":{"name":"default","configuration":{"separator":"/"}},"fill_value":null,"codecs":[{"name":"bytes","configuration":{"endian":"little"}},{"name":"gzip","configuration":{"level":1}}],"attributes":{"units":"celsius"}} +{"zarr_format":3,"node_type":"array","shape":[3,2],"dimension_names":["y","x"],"data_type":"float64","chunk_grid":{"name":"regular","configuration":{"chunk_shape":[2,2]}},"chunk_key_encoding":{"name":"default","configuration":{"separator":"/"}},"fill_value":null,"codecs":[{"name":"bytes","configuration":{"endian":"little"}},{"name":"gzip","configuration":{"level":1}}],"attributes":{"units":"celsius"}} diff --git a/test/python_smoke.py b/test/python_smoke.py index 68e53ec..ffc34b1 100644 --- a/test/python_smoke.py +++ b/test/python_smoke.py @@ -126,6 +126,17 @@ def main() -> None: print("\nV3 temperature cells:") print(v3_temperature) + ome_arrow_temperature = con.execute( + """ + SELECT * + FROM ome_arrow(?, 'temperature_v3') + ORDER BY y, x + """, + [FIXTURE_V3_PATH.as_posix()], + ).fetchdf() + print("\nOME-Arrow-compatible V3 temperature cells:") + print(ome_arrow_temperature) + if IDR_FIXTURE_PATH.exists(): idr_overview = con.execute( """ @@ -183,6 +194,7 @@ def main() -> None: assert v3_overview["array_path"].tolist() == ["fortran_v3", "mask_v3", "temperature_v3"] assert v3_overview["zarr_format"].tolist() == [3, 3, 3] assert np.isclose(v3_temperature["value"].to_numpy(dtype=np.float64).sum(), 21.0) + assert list(ome_arrow_temperature.columns) == ["y", "x", "value"] print("\nPython smoke test passed.") diff --git a/test/sql/duckdb_zarr.test b/test/sql/duckdb_zarr.test index 137c1b2..0cb8bd7 100644 --- a/test/sql/duckdb_zarr.test +++ b/test/sql/duckdb_zarr.test @@ -9,6 +9,11 @@ Catalog Error: Table Function with name zarr_groups does not exist! require duckdb_zarr +statement error +SELECT * FROM ome_arrow('test/data/simple_v2.zarr'); +---- +Invalid Input Error: Zarr store path does not contain recognizable v3 metadata: test/data/simple_v2.zarr + query III SELECT group_path, zarr_format, metadata_path LIKE '%simple_v2.zarr%' FROM zarr_groups('test/data/simple_v2.zarr') @@ -257,6 +262,15 @@ fortran_v3 2 [2, 2] [2, 2] default mask_v3 2 [2, 3] [2, 2] |b1 C v2 temperature_v3 2 [3, 2] [2, 2] Date: Sun, 29 Mar 2026 13:39:30 -0600 Subject: [PATCH 2/2] coderabbit review changes --- src/zarr_metadata.cpp | 60 ++++++++++++++++++++++++------------------- 1 file changed, 34 insertions(+), 26 deletions(-) diff --git a/src/zarr_metadata.cpp b/src/zarr_metadata.cpp index cac1924..f6f9776 100644 --- a/src/zarr_metadata.cpp +++ b/src/zarr_metadata.cpp @@ -276,10 +276,10 @@ static vector RequireInt64Array(yyjson_val *obj, const char *key) { return result; } -static vector OptionalStringArray(yyjson_val *obj, const char *key) { +static pair> OptionalStringArray(yyjson_val *obj, const char *key) { auto value = yyjson_obj_get(obj, key); if (!value || yyjson_is_null(value)) { - return {}; + return {false, {}}; } if (!yyjson_is_arr(value)) { throw InvalidInputException("Zarr metadata key \"%s\" is not an array", key); @@ -294,7 +294,7 @@ static vector OptionalStringArray(yyjson_val *obj, const char *key) { } result.emplace_back(unsafe_yyjson_get_str(element), unsafe_yyjson_get_len(element)); } - return result; + return {true, std::move(result)}; } static yyjson_val *RequireObject(yyjson_val *obj, const char *key) { @@ -346,18 +346,18 @@ static string NormalizeStorePath(FileSystem &fs, const string &path) { return fs.ExpandPath(path); } -static vector NormalizeDimensionNames(const vector &dimension_names, idx_t rank) { - if (dimension_names.empty()) { +static vector NormalizeDimensionNames(const pair> &dimension_names, idx_t rank) { + if (!dimension_names.first) { return {}; } - if (dimension_names.size() != rank) { + if (dimension_names.second.size() != rank) { throw InvalidInputException("Zarr v3 dimension_names rank does not match array rank"); } vector result; result.reserve(rank); std::unordered_set seen; for (idx_t i = 0; i < rank; i++) { - auto name = dimension_names[i]; + auto name = dimension_names.second[i]; StringUtil::Trim(name); name = StringUtil::Lower(name); if (name.empty()) { @@ -382,6 +382,15 @@ static vector NormalizeDimensionNames(const vector &dimension_na return result; } +static string UniqueValueColumnName(const vector &names) { + string candidate = "value"; + idx_t suffix = 1; + while (std::find(names.begin(), names.end(), candidate) != names.end()) { + candidate = "value_" + std::to_string(suffix++); + } + return candidate; +} + static vector GetOutputDimensionNames(const ZarrArrayEntry &array) { if (!array.dimension_names.empty()) { return array.dimension_names; @@ -1735,13 +1744,13 @@ static ZarrArrayEntry ParseArrayMetadataObjectV3(yyjson_val *root, const string bool little_endian = HostIsLittleEndian(); ParseV3ShardingCodec(first_codec, metadata_path, chunks, compressor, supports_cells, cells_error, inner_chunks, inner_compressor, index_codecs, index_location, little_endian); - auto entry = MakeArrayEntry(store_path, relative_path, zarr_format, NumericCast(shape.size()), - std::move(shape), std::move(chunks), - ParseV3DataType(root, metadata_path, little_endian), "C", - std::move(compressor), JsonToString(yyjson_obj_get(root, "fill_value")), - std::move(chunk_key_encoding_name), std::move(dimension_separator), metadata_path, - supports_cells, std::move(cells_error), true, {}, std::move(inner_chunks), - std::move(inner_compressor), std::move(index_codecs), std::move(index_location)); + auto entry = + MakeArrayEntry(store_path, relative_path, zarr_format, NumericCast(shape.size()), + std::move(shape), std::move(chunks), ParseV3DataType(root, metadata_path, little_endian), + "C", std::move(compressor), JsonToString(yyjson_obj_get(root, "fill_value")), + std::move(chunk_key_encoding_name), std::move(dimension_separator), metadata_path, + supports_cells, std::move(cells_error), true, {}, std::move(inner_chunks), + std::move(inner_compressor), std::move(index_codecs), std::move(index_location)); entry.dimension_names = std::move(dimension_names); return entry; } @@ -1840,12 +1849,12 @@ static ZarrArrayEntry ParseArrayMetadataObjectV3(yyjson_val *root, const string cells_error = "zarr_cells does not yet support the Zarr v3 codec pipeline for this array"; } - auto entry = MakeArrayEntry(store_path, relative_path, zarr_format, NumericCast(shape.size()), - std::move(shape), std::move(chunks), - ParseV3DataType(root, metadata_path, little_endian), std::move(order), - std::move(compressor), JsonToString(yyjson_obj_get(root, "fill_value")), - std::move(chunk_key_encoding_name), std::move(dimension_separator), metadata_path, - supports_cells, std::move(cells_error), false, {}, {}, "", "", ""); + auto entry = + MakeArrayEntry(store_path, relative_path, zarr_format, NumericCast(shape.size()), std::move(shape), + std::move(chunks), ParseV3DataType(root, metadata_path, little_endian), std::move(order), + std::move(compressor), JsonToString(yyjson_obj_get(root, "fill_value")), + std::move(chunk_key_encoding_name), std::move(dimension_separator), metadata_path, + supports_cells, std::move(cells_error), false, {}, {}, "", "", ""); entry.dimension_names = std::move(dimension_names); return entry; } @@ -2218,13 +2227,12 @@ static unique_ptr BindArraysInternal(ClientContext &context, const } static unique_ptr BindOmeArrowArraysInternal(ClientContext &context, const string &store_path, - vector &return_types, - vector &names) { + vector &return_types, vector &names) { vector groups; vector arrays; vector chunks; DiscoverStore(context, store_path, groups, arrays, chunks, false, ZarrVersionOverride::V3); - names = {"store_path", "array_path", "zarr_format", "rank", "shape", + names = {"store_path", "array_path", "zarr_format", "rank", "shape", "chunk_shape", "dtype", "dimension_names", "metadata_path"}; return_types = {LogicalType::VARCHAR, LogicalType::VARCHAR, @@ -2341,7 +2349,7 @@ static unique_ptr BindCellsWithDimensionNamesInternal(ClientContex for (idx_t dim = 0; dim < NumericCast(cell_array.rank); dim++) { return_types.push_back(LogicalType::BIGINT); } - names.push_back("value"); + names.push_back(UniqueValueColumnName(names)); return_types.push_back(dtype.logical_type); return make_uniq(cell_array, std::move(chunks)); } @@ -2383,8 +2391,8 @@ static unique_ptr BindOmeArrowCells(ClientContext &context, TableF auto version_override = input.inputs.size() > 2 ? ParseZarrVersionOverride(input.inputs[2], "ome_arrow") : ZarrVersionOverride::V3; return BindCellsWithDimensionNamesInternal(context, StringValue::Get(input.inputs[0]), - NormalizeArrayPath(StringValue::Get(input.inputs[1])), - version_override, return_types, names); + NormalizeArrayPath(StringValue::Get(input.inputs[1])), version_override, + return_types, names); } static void ScanGroups(ClientContext &, TableFunctionInput &data_p, DataChunk &output) {