Preconditions, Annotations, Exceptions, Nulls, and Optionals

[Charlie Summers <ch...@gomerits.com>]

Hey Pinot team,

My name is Charlie Summers and I'm a new contributor to the project. As
I've started diving into the code, I'm not seeing a consistent pattern for
how we handle unexpected behavior in the project.

In some places we're throwing exceptions. In some places we log an error
message then continue, returning a Null if a value is expected and having
calling code handle the Null. When handling Nulls, we sometimes
use @Nullable and @NotNull annotations and sometimes use
Preconditions.checkNotNull().

I was chatting a bit with Kishore about this in Slack - he didn't have a
strong preference for how to handle these things but was concerned about
the performance implications of over-using Annotations and Precondition
checks.

What are all of your feelings on the matter? Personally I'm a fan of
throwing Exceptions should state truly be exceptional instead of limping on
and then handling a Null later. Where there's a need for a
partially-functioning state or values may genuinely not be present, I'm a
fan of Optionals (about their performance:
https://stackoverflow.com/questions/34696884/performance-of-java-optional/34697019
).

What have you all found to be effective? And definitely let me know if
there is a more consistent standard that I'm missing because I've only just
started dabbling with the code base.

Cheers,
Charlie Summers

Re: Preconditions, Annotations, Exceptions, Nulls, and Optionals

[Mayank Shrivastava <ms...@linkedin.com.INVALID>]

Hi Charlie,
Thanks for taking the initiative on this one. My recommendation would be to document all such areas, and then open it up for consensus on the convention to follow. I think @Jackie Jiang<ma...@linkedin.com> has done similar cleanup in the past (specific to Nonnull vs Nullable<ma...@microsoft.com>), would be good to build up on that.

I do agree with Kishore though, we should not trade for performance (specifically in query execution), and keep that in mind when discussing.

Cheers,
Mayank
________________________________
From: Charlie Summers <ch...@gomerits.com>
Sent: Saturday, May 9, 2020 9:25 PM
To: dev@pinot.incubator.apache.org <de...@pinot.incubator.apache.org>
Subject: Preconditions, Annotations, Exceptions, Nulls, and Optionals

Hey Pinot team,

My name is Charlie Summers and I'm a new contributor to the project. As
I've started diving into the code, I'm not seeing a consistent pattern for
how we handle unexpected behavior in the project.

In some places we're throwing exceptions. In some places we log an error
message then continue, returning a Null if a value is expected and having
calling code handle the Null. When handling Nulls, we sometimes
use @Nullable and @NotNull annotations and sometimes use
Preconditions.checkNotNull().

I was chatting a bit with Kishore about this in Slack - he didn't have a
strong preference for how to handle these things but was concerned about
the performance implications of over-using Annotations and Precondition
checks.

What are all of your feelings on the matter? Personally I'm a fan of
throwing Exceptions should state truly be exceptional instead of limping on
and then handling a Null later. Where there's a need for a
partially-functioning state or values may genuinely not be present, I'm a
fan of Optionals (about their performance:
https://nam06.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstackoverflow.com%2Fquestions%2F34696884%2Fperformance-of-java-optional%2F34697019&amp;data=02%7C01%7Cmshrivas%40linkedin.com%7C10831b986b9647b5e19e08d7f49a3fce%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C637246815649018974&amp;sdata=bE%2FzJcow6K9VFmyzewNjmiuqP2REm8CgulR6PtTkV2g%3D&amp;reserved=0
).

What have you all found to be effective? And definitely let me know if
there is a more consistent standard that I'm missing because I've only just
started dabbling with the code base.

Cheers,
Charlie Summers