It doesn’t matter if PIP is open. can directly describe it clearly in https://lists.apache.org/thread/mbrpjsgrgwrlkdpvkk738jxnlk7rf4qk and modify it. But either newly opened PIP or in https://lists.apache.org/thread/mbrpjsgrgwrlkdpvkk738jxnlk7rf4q, my point is that we need to fix it, not compatible with it.
Thanks, Bo Yunze Xu <y...@streamnative.io.invalid> 于2022年12月16日周五 08:21写道： > > Let's see another example that is considered as a "bug", not a breaking > change. > > https://lists.apache.org/thread/88t1xxf68j092k09srdwyzj1tk4ml5n9 > > > I think that this is fixing a bug, if the topic does not exist we should > > return "not found". > > The "bug" and "breaking change" are not in contrast. What a user > thinks is a bug might be a breaking change. Would you like to open a > PIP for that? From my perspective, both modifying or not is okay. > > Thanks, > Yunze > > On Wed, Dec 14, 2022 at 11:27 PM 丛搏 <bog...@apache.org> wrote: > > > > I still feel better to change compareTo directly. > > > > 1. Although using PulsarApiMessageId.campare() can reduce the > > probability of developers using errors, it cannot be completely > > avoided. > > > > 2. While a direct change would change the default behavior, I consider > > it a bug, not a breaking change. We can explain it in the new version > > release blog. Maybe some users use it, but they didn’t find the > > problem, and we changed it correctly . I don't think any user will be > > able to use the current compareTo() correctly. Because the current > > implementation is unexpected. When the user finds out that this > > problem exists, he will not use this method. > > > > Thanks, > > Bo > > > > Yunze Xu <y...@streamnative.io.invalid> 于2022年12月8日周四 20:43写道： > > > > > > 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
