Copilot commented on code in PR #7621:
URL: https://github.com/apache/ignite-3/pull/7621#discussion_r2822212824
##########
modules/sql-engine/src/test/java/org/apache/ignite/internal/sql/engine/exec/SqlOutdatedPlanTest.java:
##########
@@ -188,41 +186,38 @@ private static CatalogCommand makeAddColumnCommand(String
columnName) {
private static class PrepareServiceSpy {
private final AtomicInteger callsCounter = new AtomicInteger();
- private final AtomicReference<ReentrantLock> prepareBlockHolder = new
AtomicReference<>();
+ private final AtomicReference<Semaphore> prepareBlockHolder = new
AtomicReference<>();
PrepareServiceSpy(TestNode gatewayNode) {
((PrepareServiceWithPrepareCallback) gatewayNode.prepareService())
.setPrepareCallback(() -> {
callsCounter.incrementAndGet();
- Lock lock = prepareBlockHolder.get();
+ Semaphore semaphore = prepareBlockHolder.get();
Review Comment:
There's a potential NullPointerException in the callback if it's invoked
before resetAndBlockNextCall() is called for the first time. The
prepareBlockHolder is initialized as an empty AtomicReference, so
semaphore.acquire() at line 199 would throw NPE if semaphore is null.
While the current test code appears to avoid this by calling
resetAndBlockNextCall() before executeQueryAsync(), this creates a fragile
design. Consider initializing prepareBlockHolder with a default Semaphore(0) in
the constructor, or adding a null check in the callback.
```suggestion
if (semaphore == null) {
// No semaphore has been installed yet; nothing
to block on.
return;
}
```
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
