bit2swaz commented on PR #10764: URL: https://github.com/apache/arrow-rs/pull/10764#issuecomment-5361935337
agreed, ill revert that commit then. a schema with `format = ""` drops fine but its a half built C schema that could get exported to a foreign consumer, not worth it > Leave the behavior the same the original on an empty schema was `Box::from_raw(null)`, the UB we're fixing so "the same" doesnt make sense to me. did you mean keep the early `Err` on null `private_data` or drop the empty handling entirely and let the `unsafe` contract + docs cover it? your note about documenting "dont use on an empty schema" reads like the second, but i figured id just be sure > lead with `self` must be a Schema created in arrow-rs will do one correction: re setting metadata doesnt leak. the old bytes are in `private_data.metadata: Option<Vec<u8>>` and get dropped on reassign. the leaker is `with_name` which overwrites `self.name` without freeing the old `CString`: https://github.com/apache/arrow-rs/blob/dcc801a9a99130245cbfb4f8aa6690ee0b17f876/arrow-schema/src/ffi.rs#L179-L188 note it there instead, or leave for later? -- 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]
