apache / datafusion

Apache DataFusion SQL Query Engine
https://datafusion.apache.org/
Apache License 2.0
6.33k stars 1.2k forks source link

fix: serialize user-defined window functions to proto #13421

Closed jcsherin closed 6 days ago

jcsherin commented 1 week ago

Which issue does this PR close?

Closes #13401.

Rationale for this change

Same as #13401.

What changes are included in this PR?

  1. Serializes user-defined window functions to proto
  2. Adds codec methods to PhysicalExtensionCodec:

    pub trait PhysicalExtensionCodec: Debug + Send + Sync {
    
    fn try_decode_udwf(&self, name: &str, _buf: &[u8]) -> Result<Arc<WindowUDF>> {
        not_impl_err!("PhysicalExtensionCodec is not provided for window function {name}")
    }
    
    fn try_encode_udwf(&self, _node: &WindowUDF, _buf: &mut Vec<u8>) -> Result<()> {
        Ok(())
    }
    }

Are these changes tested?

Yes, roundtrip physical plan for,

  1. An existing user-defined window function and,
  2. A user-defined window function with a custom payload and implements PhysicalExtensionCodec.

Are there any user-facing changes?

No.

mwylde commented 1 week ago

Thanks! I can confirm this fixes our tests.

jcsherin commented 6 days ago

I just merged up from main to fix conflict.

alamb commented 6 days ago

Thanks again @jcsherin