milenkovicm commented on code in PR #1678:
URL:
https://github.com/apache/datafusion-python/pull/1678#discussion_r3796279741
##########
crates/core/src/codec.rs:
##########
@@ -223,32 +224,129 @@ fn strip_wire_header<'a>(
Ok(Some(&buf[py_minor_idx + 1..]))
}
+/// Run `f` against each codec in `chain`, returning the first `Ok`.
+///
+/// A codec signals "not mine" by returning an error, so the chain
+/// keeps trying until a codec succeeds. When every codec fails and the
+/// chain has more than one entry, the errors are aggregated into a
+/// single message — returning only the last error would surface the
+/// terminal `Default*ExtensionCodec` "not provided" message and mask
+/// the more specific diagnostic from an installed codec (e.g. a
+/// corrupt-token error from the codec that owns the payload family).
+fn chain_try<C: ?Sized, R>(chain: &[Arc<C>], what: &str, f: impl Fn(&C) ->
Result<R>) -> Result<R> {
+ let mut errors: Vec<datafusion::error::DataFusionError> = Vec::new();
+ for codec in chain {
Review Comment:
this is not safe, you can get things decoded by accident. please dont ask
how i know and how much time i spent debuging it
##########
crates/core/src/codec.rs:
##########
@@ -223,32 +224,129 @@ fn strip_wire_header<'a>(
Ok(Some(&buf[py_minor_idx + 1..]))
}
+/// Run `f` against each codec in `chain`, returning the first `Ok`.
+///
+/// A codec signals "not mine" by returning an error, so the chain
+/// keeps trying until a codec succeeds. When every codec fails and the
+/// chain has more than one entry, the errors are aggregated into a
+/// single message — returning only the last error would surface the
+/// terminal `Default*ExtensionCodec` "not provided" message and mask
+/// the more specific diagnostic from an installed codec (e.g. a
+/// corrupt-token error from the codec that owns the payload family).
+fn chain_try<C: ?Sized, R>(chain: &[Arc<C>], what: &str, f: impl Fn(&C) ->
Result<R>) -> Result<R> {
+ let mut errors: Vec<datafusion::error::DataFusionError> = Vec::new();
+ for codec in chain {
+ match f(codec) {
+ Ok(value) => return Ok(value),
+ Err(err) => errors.push(err),
+ }
+ }
+ Err(aggregate_chain_errors(what, errors))
+}
+
+/// Collapse per-codec failures into one error. A single failure is
+/// returned as-is so the one-codec (default-only) chain behaves
+/// exactly like the pre-chain implementation.
+fn aggregate_chain_errors(
+ what: &str,
+ mut errors: Vec<datafusion::error::DataFusionError>,
+) -> datafusion::error::DataFusionError {
+ match errors.len() {
+ 0 => datafusion::error::DataFusionError::Internal(format!(
+ "Empty extension codec chain while handling {what}"
+ )),
+ 1 => errors.swap_remove(0),
+ _ => {
+ let joined = errors
+ .iter()
+ .map(|err| err.to_string())
+ .collect::<Vec<_>>()
+ .join("; ");
+ datafusion::error::DataFusionError::Execution(format!(
+ "None of the {} composed extension codecs handled {what}:
{joined}",
+ errors.len()
+ ))
+ }
+ }
+}
+
+/// Encode variant of [`chain_try`] for methods that write into a
+/// caller-provided buffer.
+///
+/// Each codec encodes into a scratch buffer so a failed attempt cannot
+/// leave partial bytes behind. `Ok` with bytes written commits those
+/// bytes and ends the chain. `Ok` with an empty buffer is treated as
+/// "no opinion" — the standard `Default*ExtensionCodec` behavior of
+/// encoding a UDF by name writes nothing — so later codecs still get a
+/// chance to emit a richer payload. If no codec writes bytes but at
+/// least one returned `Ok`, the overall result is `Ok` with nothing
+/// written (encode by name).
+fn chain_encode<C: ?Sized>(
Review Comment:
this might work, but still can be tricky
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
