You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@sedona.apache.org by "CaptainAni187 (via GitHub)" <gi...@apache.org> on 2026/10/06 01:07:20 UTC
[PR] fix(rust/sedona): let CREATE DATABASE fall back past catalog lists that cannot create one [sedona-db]
CaptainAni187 opened a new pull request, #1410:
URL: https://github.com/apache/sedona-db/pull/1410
Closes #1407
`CREATE DATABASE` for a catalog nobody owns yet always went to `latest_foreign()`. If that catalog list could not create top-level catalogs, the statement failed, even when an older registration, or DataFusion's built-in list, could create it.
The new catalog is now offered to the foreign catalog lists newest first. `SedonaCatalogRegistry::foreign_newest_first()` is new; `latest_foreign()` is kept for `catalog_planner.rs`.
- A `NotImplemented` error from `create_object` (checked with `find_root()`) means "not supported here", and the next older registration is tried.
- If none of them can create it, the statement goes to DataFusion's built-in catalog list, as it does with no foreign catalogs registered.
- Any other error is returned as before.
The contract is documented on `SedonaCatalogList::create_object`. Statements on catalogs that already have an owner are unchanged.
Tests in `rust/sedona/src/exec.rs`, using two new flags on `TestCatalog`:
- `create_database_skips_catalog_lists_that_cannot_create_one`: the newer list refuses, and the older one creates the catalog.
- `create_database_falls_back_to_builtin_when_no_catalog_list_can`: the catalog ends up in the built-in list, and `CREATE SCHEMA` in it works.
- `create_database_does_not_fall_back_on_other_errors`: an ordinary error is returned, and nothing is created anywhere.
The first two fail on main. `cargo test -p sedona -p sedona-catalog` (with the geoarrow-data submodule), `cargo clippy -- -D warnings` and `cargo fmt --check` pass locally.
One open question: catalogs from `c/sedona-extension` can't use this yet, because `check_error` turns every non-zero callback code into an execution error. If you want those to take part, mapping a specific errno (for example `ENOTSUP`) to `NotImplemented` there would be a small follow-up. I left it out because it changes the C callback contract.
--
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] fix(rust/sedona): let CREATE DATABASE fall back past catalog lists that cannot create one [sedona-db]
Posted by "CaptainAni187 (via GitHub)" <gi...@apache.org>.
CaptainAni187 commented on code in PR #1410:
URL: https://github.com/apache/sedona-db/pull/1410#discussion_r4203413439
##########
rust/sedona/src/catalog.rs:
##########
@@ -81,10 +81,16 @@ impl SedonaCatalogRegistry {
Ok(None)
}
- /// Destination for new top-level catalogs.
+ /// The most recently registered foreign catalog list.
pub fn latest_foreign(&self) -> Option<Arc<dyn SedonaCatalogList>> {
self.foreign.read().last().cloned()
}
+
+ /// Foreign catalog lists in the order they are offered a new top-level
+ /// catalog: newest registration first.
+ pub fn foreign_newest_first(&self) -> Vec<Arc<dyn SedonaCatalogList>> {
+ self.foreign.read().iter().rev().cloned().collect()
+ }
Review Comment:
Done in 92fb382. A borrowed `&[...]` would have to keep the `RwLock` read guard alive across the `.await` in exec.rs, so the registry now stores an `Arc<[Arc<dyn SedonaCatalogList>]>` instead. `foreign_lists()` returns that slice, documented as oldest first (later registrations take precedence), and CREATE DATABASE iterates it with `.iter().rev()`. Taking the snapshot clones one `Arc`. `register_foreign` builds the new slice, and `foreign_catalog` uses the same snapshot instead of cloning the `Vec`.
--
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] fix(rust/sedona): let CREATE DATABASE fall back past catalog lists that cannot create one [sedona-db]
Posted by "CaptainAni187 (via GitHub)" <gi...@apache.org>.
CaptainAni187 commented on PR #1410:
URL: https://github.com/apache/sedona-db/pull/1410#issuecomment-6022237959
@paleolimbot Done in 4f1b0d0: an exported catalog list returns `ENOTSUP` when `create_object` fails with `NotImplemented`, and an imported one maps `ENOTSUP` back to `NotImplemented`, so C catalogs can decline `CREATE DATABASE` and the next registration gets it. The convention is documented on `create_object` in `sedona_extension.h`, with tests for both directions. `cargo test -p sedona-extension -p sedona -p sedona-catalog`, clippy and fmt pass locally.
--
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] fix(rust/sedona): let CREATE DATABASE fall back past catalog lists that cannot create one [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot merged PR #1410:
URL: https://github.com/apache/sedona-db/pull/1410
--
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] fix(rust/sedona): let CREATE DATABASE fall back past catalog lists that cannot create one [sedona-db]
Posted by "paleolimbot (via GitHub)" <gi...@apache.org>.
paleolimbot commented on code in PR #1410:
URL: https://github.com/apache/sedona-db/pull/1410#discussion_r4200584171
##########
rust/sedona/src/catalog.rs:
##########
@@ -81,10 +81,16 @@ impl SedonaCatalogRegistry {
Ok(None)
}
- /// Destination for new top-level catalogs.
+ /// The most recently registered foreign catalog list.
pub fn latest_foreign(&self) -> Option<Arc<dyn SedonaCatalogList>> {
self.foreign.read().last().cloned()
}
+
+ /// Foreign catalog lists in the order they are offered a new top-level
+ /// catalog: newest registration first.
+ pub fn foreign_newest_first(&self) -> Vec<Arc<dyn SedonaCatalogList>> {
+ self.foreign.read().iter().rev().cloned().collect()
+ }
Review Comment:
Instead of cloning the entire list, can you expose `&[Arc<dyn SedonaCatalogList>]`, document the order, and then iterate backwards over it in exec.rs?
--
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