Skip to content

Commit 9a30e48

Browse files
authored
fix: DH-18567: Filter locations before removal allowed checks. (#6607)
1 parent aec9cbf commit 9a30e48

File tree

3 files changed

+396
-15
lines changed

3 files changed

+396
-15
lines changed

engine/table/src/main/java/io/deephaven/engine/table/impl/SourceTable.java

+18-14
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import javax.annotation.OverridingMethodsMustInvokeSuper;
2929
import java.util.ArrayList;
3030
import java.util.Collection;
31+
import java.util.stream.Stream;
3132

3233
/**
3334
* Basic uncoalesced table that only adds keys.
@@ -153,7 +154,6 @@ private void initializeAvailableLocations() {
153154
try (final TableLocationSubscriptionBuffer.LocationUpdate locationUpdate =
154155
locationBuffer.processPending()) {
155156
if (locationUpdate != null) {
156-
maybeRemoveLocations(locationUpdate.getPendingRemovedLocationKeys());
157157
maybeAddLocations(locationUpdate.getPendingAddedLocationKeys());
158158
}
159159
}
@@ -188,14 +188,26 @@ private void maybeAddLocations(@NotNull final Collection<LiveSupplier<ImmutableT
188188
.forEach(lk -> columnSourceManager.addLocation(locationProvider.getTableLocation(lk.get())));
189189
}
190190

191-
private void maybeRemoveLocations(@NotNull final Collection<LiveSupplier<ImmutableTableLocationKey>> removedKeys) {
191+
private void maybeRemoveLocations(@NotNull final Collection<LiveSupplier<ImmutableTableLocationKey>> removedKeys,
192+
final boolean removedAllowed) {
192193
if (removedKeys.isEmpty()) {
193194
return;
194195
}
195196

196-
filterLocationKeys(removedKeys).stream()
197+
final Collection<LiveSupplier<ImmutableTableLocationKey>> filteredSuppliers = filterLocationKeys(removedKeys);
198+
if (filteredSuppliers.isEmpty()) {
199+
return;
200+
}
201+
202+
if (removedAllowed) {
203+
filteredSuppliers.stream().map(LiveSupplier::get).forEach(columnSourceManager::removeLocationKey);
204+
return;
205+
}
206+
207+
final ImmutableTableLocationKey[] keys = filteredSuppliers.stream()
197208
.map(LiveSupplier::get)
198-
.forEach(columnSourceManager::removeLocationKey);
209+
.toArray(ImmutableTableLocationKey[]::new);
210+
throw new TableLocationRemovedException("Source table does not support removed locations", keys);
199211
}
200212

201213
private void initializeLocationSizes() {
@@ -238,16 +250,8 @@ protected void instrumentedRefresh() {
238250
try (final TableLocationSubscriptionBuffer.LocationUpdate locationUpdate =
239251
locationBuffer.processPending()) {
240252
if (locationUpdate != null) {
241-
if (!locationProvider.getUpdateMode().removeAllowed()
242-
&& !locationUpdate.getPendingRemovedLocationKeys().isEmpty()) {
243-
// This TLP doesn't support removed locations, we need to throw an exception.
244-
final ImmutableTableLocationKey[] keys = locationUpdate.getPendingRemovedLocationKeys().stream()
245-
.map(LiveSupplier::get).toArray(ImmutableTableLocationKey[]::new);
246-
throw new TableLocationRemovedException(
247-
"Source table does not support removed locations", keys);
248-
}
249-
250-
maybeRemoveLocations(locationUpdate.getPendingRemovedLocationKeys());
253+
maybeRemoveLocations(locationUpdate.getPendingRemovedLocationKeys(),
254+
locationProvider.getUpdateMode().removeAllowed());
251255
maybeAddLocations(locationUpdate.getPendingAddedLocationKeys());
252256
}
253257
}

engine/table/src/test/java/io/deephaven/engine/table/impl/TestPartitionAwareSourceTable.java

-1
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,6 @@ private void allowLivenessRelease() {
225225
{
226226
allowing(locationProvider).supportsSubscriptions();
227227
allowing(locationProvider).unsubscribe(with(any(TableLocationProvider.Listener.class)));
228-
will(returnValue(true));
229228
for (int li = 0; li < tableLocations.length; ++li) {
230229
final TableLocation tableLocation = tableLocations[li];
231230
allowing(tableLocation).supportsSubscriptions();

0 commit comments

Comments
 (0)