RowWithGetters, FieldValueGetter being Serializable?

[Alex Van Boxel <al...@vanboxel.be>]

Hi,

I noticed that RowWithGetters and FieldValueGetter are both serializable
(all in package org.apache.beam.sdk.values). I do have my doubt if they
should be.

Certainly RowWithGetters would be problematic:

   - it references the underlying object that could be anything and it's *not
   guaranteed* Serializable. In my case I'm referring a Protobuf message
   that is not.
   - the FieldValueGetter should also not be Serializable as they are
   generated by the factory. I'm implementing Getters that also needs
   FieldDescriptors to access the underlying dynamic Protobuf fields,
   FieldDescriptors are also not serializable.

The *only* class that should be serializable is RowWithStorage as the
current implementation will convert any type of Row to this as soon as a
serialization step needs to happen.

Thoughts?! If you all agree, I'll create a ticket and fix this, as this is
a bit blocking my implementation of Protobuf (as it wont pass SpotBug
complaining on non serializable fields in FieldValueGetter).

 _/
_/ Alex Van Boxel

Re: RowWithGetters, FieldValueGetter being Serializable?

[Reuven Lax <re...@google.com>]

My first thought is that Row shouldn't need to be serializable at all.
However it's possible that we have a dependency in some code on it being
serializable. Worth testing it out to see what breaks.

On Wed, Aug 28, 2019 at 2:55 AM Alex Van Boxel <al...@vanboxel.be> wrote:

> Hi,
>
> I noticed that RowWithGetters and FieldValueGetter are both serializable
> (all in package org.apache.beam.sdk.values). I do have my doubt if they
> should be.
>
> Certainly RowWithGetters would be problematic:
>
>    - it references the underlying object that could be anything and it's *not
>    guaranteed* Serializable. In my case I'm referring a Protobuf message
>    that is not.
>    - the FieldValueGetter should also not be Serializable as they are
>    generated by the factory. I'm implementing Getters that also needs
>    FieldDescriptors to access the underlying dynamic Protobuf fields,
>    FieldDescriptors are also not serializable.
>
> The *only* class that should be serializable is RowWithStorage as the
> current implementation will convert any type of Row to this as soon as a
> serialization step needs to happen.
>
> Thoughts?! If you all agree, I'll create a ticket and fix this, as this is
> a bit blocking my implementation of Protobuf (as it wont pass SpotBug
> complaining on non serializable fields in FieldValueGetter).
>
>  _/
> _/ Alex Van Boxel
>