crossoverJie commented on code in PR #20911:
URL: https://github.com/apache/pulsar/pull/20911#discussion_r1329759484
##########
pulsar-sql/presto-pulsar/src/main/java/org/apache/pulsar/sql/presto/PulsarSplitManager.java:
##########
@@ -328,6 +328,9 @@ Collection<PulsarSplit> getSplitsForTopic(String
topicNamePersistenceEncoding,
splits.add(pulsarSplit);
}
return splits;
+ } catch (ManagedLedgerException exception){
Review Comment:
Having examined the source code, I know that when `ML` is initialized and
sending messages, it will write data to `ledgers`. However, `ledgers` are
always empty for a new topic, which results in a `NoSuchElementException`.
Therefore I think throwing this exception is normal, but for users using
`pulsar-sql`, they should not get this exception. Returning an empty list to
indicate that there is currently no data would be more reasonable.
```java
Caused by: java.util.NoSuchElementException
at
java.base/java.util.concurrent.ConcurrentSkipListMap.firstKey(ConcurrentSkipListMap.java:1859)
at
org.apache.bookkeeper.mledger.impl.ManagedLedgerImpl.getFirstPosition(ManagedLedgerImpl.java:3695)
at
org.apache.bookkeeper.mledger.impl.ReadOnlyCursorImpl.<init>(ReadOnlyCursorImpl.java:38)
at
org.apache.bookkeeper.mledger.impl.ReadOnlyManagedLedgerImpl.createReadOnlyCursor(ReadOnlyManagedLedgerImpl.java:138)
at
org.apache.bookkeeper.mledger.impl.ManagedLedgerFactoryImpl$4.openReadOnlyManagedLedgerComplete(ManagedLedgerFactoryImpl.java:498)
at
org.apache.bookkeeper.mledger.impl.ManagedLedgerFactoryImpl.lambda$asyncOpenReadOnlyManagedLedger$9(ManagedLedgerFactoryImpl.java:442)
at
java.base/java.util.concurrent.CompletableFuture$UniRun.tryFire(CompletableFuture.java:787)
... 16 more
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
