Actually I'm refactoring the MessageId related code [1], whose current
implementations are very messy from my perspective. My solution to
this issue is adding two compare methods, one of them is the "wrong"
implementation and used in `MessageId#compareTo` to avoid the breaking
change. See the `legacyCompare` and `compare` methods.
```java
// The legacy compare method, which treats the non-batched message id
as preceding the batched message id.
// However, this behavior is wrong because a non-batched message id
represents an entry, while a batched message
// represents a single message in the entry, which should precedes the
message id.
// Keep this implementation just for backward compatibility when users
compare two message ids.
static int legacyCompare(MessageIdDataInterface lhs,
MessageIdDataInterface rhs) { /* ... */ }
static int compare(MessageIdDataInterface lhs, MessageIdDataInterface
rhs) { /* ... */ }
```
[1] https://github.com/BewareMyPower/pulsar/pull/11/files
Thanks,
Yunze
On Thu, Dec 8, 2022 at 7:22 PM 丛搏 <bog...@apache.org> wrote:
>
> 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
