search

trinodb / trino

Official repository of Trino, the distributed SQL query engine for big data, formerly known as PrestoSQL (https://trino.io)
https://trino.io
Apache License 2.0
10.46k stars 3.01k forks source link

Cleanup JSON serialization in event listener classes #3304

Open electrum opened 4 years ago

electrum commented 4 years ago

Many classes in io.prestosql.spi.eventlistener are serializable to JSON, a couple can also be deserialized from JSON, and others have no serialization. There are no tests for any of it.

I believe the current serialization was accidental. My current thinking is that, since we don't want to make guarantees around the JSON format, we should remove the annotations. All of the classes appear to be standard beans, and should be easily serializable or deserializable using Jackson's any other framework's bean mapping / introspection features.

martint commented 4 years ago

I believe the current serialization was accidental.

No, it's not accidental. The structure and names were chosen intentionally when we implemented the event listener at FB. Whether we want that to be a guaranteed format or not is a different question, but I think it's convenient for anyone that might want to serialize it to JSON, and as a way to minimize format changes if the internal structure of the classes changes and fields are renamed.