nooberfsh / prusto

A presto/trino client library written in rust.
MIT License
37 stars 23 forks source link

Support more than 16 fields for Structs #36

Open bogdanstate opened 7 months ago

bogdanstate commented 7 months ago

The following struct compiles correctly:


#[derive(Presto)]
struct Foo {
  a1: i16,
  a2: i16,
  a3: i16,
  a4: i16,
  a5: i16,
  a6: i16,
  a7: i16,
  a8: i16,
  a9: i16,
  a10: i16,
  a11: i16,
  a12: i16,
  a13: i16,
  a14: i16,
  a15: i16,
  a16: i16,
}

If we add a17: i16 as an extra struct member we get the following error:


error[E0277]: the trait bound `(&'_a i16, &'_a i16, &'_a i16, &'_a i16, &'_a i16, &'_a i16, &'_a i16, &'_a i16, &'_a i16, &'_a i16, &'_a i16, &'_a i16, &'_a i16, &'_a i16, &'_a i16, &'_a i16, &'_a i16): Serialize` is not satisfied
  --> src/main.rs:13:10
   |
13 | #[derive(Presto)]
   |          ^^^^^^ the trait `Serialize` is not implemented for `(&i16, &i16, &i16, &i16, &i16, &i16, &i16, &i16, &i16, &i16, &i16, &i16, &i16, &i16, &i16, &i16, &i16)`
   |
   = help: the following other types implement trait `Serialize`:
             ()
             (T0,)
             (T0, T1)
             (T0, T1, T2)
             (T0, T1, T2, T3)
             (T0, T1, T2, T3, T4)
             (T0, T1, T2, T3, T4, T5)
             (T0, T1, T2, T3, T4, T5, T6)
           and 9 others
note: required by a bound in `prusto::Presto::ValueType`
  --> /Users/bogdanstate/.cargo/registry/src/index.crates.io-6f17d22bba15001f/prusto-0.5.1/src/types/mod.rs:69:25
   |
69 |     type ValueType<'a>: Serialize
   |                         ^^^^^^^^^ required by this bound in `Presto::ValueType`
   = note: this error originates in the derive macro `Presto` (in Nightly builds, run with -Z macro-backtrace for more info)

I assume this is due to a the way the derive(Presto) macro is implemented, i.e. by using a tuple as the underlying type.

nooberfsh commented 7 months ago

You are right and it is limited by the serde impls for tuples, I think there are some ways to solve this issue:

We have encountered this issue in our some internal tools and workaround this by splitting the struct into multiple smaller struct which have less than 16 fields.

Thanks for reporting this bug and I will find some time to fix this.

bogdanstate commented 7 months ago

Amazing, thanks so much for a great library!

nooberfsh commented 7 months ago

@bogdanstate I made a PR that make the Presto macro support up to 32 fields for Structs and have merged into the master branch, please check out and have a try.