Hi, Yunze:
If we don't change this behavior, we should pay special attention when
coding `pulsar-client`, because it is a point that is easy to
overlook. its impact may be more serious than "wrong " behavior
produced by the user using the current compareTo() method manually. I
don’t think this is a breaking change. On the contrary, it is a bug
that needs to be fixed. Because we cannot guarantee that everyone can
find the problem of compareTo() in time when writing code or reviewing
pr. The current implementation is Very anti-human.

Thanks,
bo

Yunze Xu <y...@streamnative.io.invalid> 于2022年12月8日周四 18:02写道:
>
> Actually, from the user side, this comparison would never happen.
> Users could never receive two MessageId objects with the same ledger
> id, entry id while the batch index fields are different. This
> comparison could only exist in the `pulsar-client` implementation.
>
> If users touch the case, the MessageId object must be created
> manually, which is a hack. The "wrong" behavior might be used. So my
> perspective is that we should not change this behavior.
>
> Thanks,
> Yunze
>
> On Thu, Dec 8, 2022 at 5:36 PM 丛搏 <bog...@apache.org> wrote:
> >
> > Hi, all:
> >
> > does anyone have any suggestions?
> >
> > Thanks,
> > bo
> >
> > 丛搏 <congbobo...@gmail.com> 于2022年11月21日周一 18:57写道:
> > >
> > > Hello, Pulsar community:
> > >
> > > now when `BatchMessageIdImpl` and `MessageIdImpl` with the same
> > > `ledgerId` and `EntryId`, one of it compare with the other, the
> > > `BatchMessageIdImpl` will always be greater than MessageIdImpl.
> > > see : 
> > > https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/BatchMessageIdImpl.java#L71-L74
> > >
> > > https://github.com/apache/pulsar/blob/master/pulsar-client/src/main/java/org/apache/pulsar/client/impl/MessageIdImpl.java#L219-L228
> > >
> > > but when we use it, we may think `MessageIdImpl` is bigger than
> > > `BatchMessageIdImpl` with the same `ledgerId` and `EntryId`. It causes
> > > a lot of bugs. I think we need to change this `compareTo()` method,
> > > although it is a public API, I think it is not a breaking change, it
> > > is a bug that needs to be fixed.
> > > eg. : https://github.com/apache/pulsar/pull/18486, need to add the
> > > separate logic for compareTo().
> > >
> > > Please leave your thoughts, thanks.
> > >
> > > Thanks,
> > > bo

Reply via email to