bit2swaz commented on issue #10679: URL: https://github.com/apache/arrow-rs/issues/10679#issuecomment-5334169556
@alamb @Jefffrey friendly ping on this one. id like to fix it but the two options change the API differently so i figured its best to ask before proceeding a lil recap of the fork: > A: make `with_metadata` (and `with_name`) `unsafe` > B: detect foreign schemas and return `Err` instead of casting one thing thats changed since i filed this: both methods already return `Result` on current main ([with_metadata at ffi.rs#L198](https://github.com/apache/arrow-rs/blob/cb20cc4bfd0fd29bf31d4c4c5c783174d9fcbff6/arrow-schema/src/ffi.rs#L198)). so option B adds no new signature and no breakage, it just returns an `Err` on the case that is currently UB, the [cast at ffi.rs#L246](https://github.com/apache/arrow-rs/blob/cb20cc4bfd0fd29bf31d4c4c5c783174d9fcbff6/arrow-schema/src/ffi.rs#L246). the check for B is cheap: a schema arrow-rs built always has `release == Some(release_schema)` (set in `try_new`, [ffi.rs#L155](https://github.com/apache/arrow-rs/blob/cb20cc4bfd0fd29bf31d4c4c5c783174d9fcbff6/arrow-schema/src/ffi.rs#L155)), and a foreign one carries the producer's release, so that tells them apart with no new field and thats why im biased towards B. whats your guys' take on this tho? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
