[
https://issues.apache.org/jira/browse/CALCITE-7556?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ruiqi Dong updated CALCITE-7556:
--------------------------------
Priority: Minor (was: Major)
> NameMultimap.remove(key, value) leaves an empty key bucket behind
> -----------------------------------------------------------------
>
> Key: CALCITE-7556
> URL: https://issues.apache.org/jira/browse/CALCITE-7556
> Project: Calcite
> Issue Type: Bug
> Components: core
> Affects Versions: 1.41.0
> Reporter: Ruiqi Dong
> Priority: Minor
>
> *Summary*
> NameMultimap.remove(String, V) removes the value from the per-key list, but
> if that value was the last entry for the key, it does not remove the key
> itself from the backing NameMap. The object is left in an inconsistent state:
> * range(key, ...) is empty
> * containsKey(key, ...) is still true
> * map() still contains the key with an empty list
> Although remove is marked @Experimental, the surrounding public API still
> exposes clear consistency expectations between remove, containsKey, range,
> and map. The current behavior breaks those expectations.
>
> *Affected code*
> File: core/src/main/java/org/apache/calcite/util/NameMultimap.java
> {code:java}
> @Experimental
> public boolean remove(String key, V value) {
> final List<V> list = map().get(key);
> if (list == null) {
> return false;
> }
> return list.remove(value);
> } {code}
>
> *Reproducer*Add the following test to
> core/src/test/java/org/apache/calcite/util/UtilTest.java
> {code:java}
> @Test void testNameMultimapRemoveLastValueRemovesKey() {
> final NameMultimap<Integer> map = new NameMultimap<>();
> map.put("baz", 1);
> assertTrue(map.remove("baz", 1));
> assertThat(map.range("baz", true), hasSize(0));
> assertThat(map.containsKey("baz", true), is(false));
> assertThat(map.containsKey("BAZ", false), is(false));
> assertThat(map.map(), aMapWithSize(0));
> } {code}
> Run:
> {code:java}
> ./gradlew :core:test \
> --tests
> org.apache.calcite.util.UtilTest.testNameMultimapRemoveLastValueRemovesKey
> {code}
> Observed behavior:
> The key remains visible after the last value has been removed
> {code:java}
> Expected: is <false>
> but: was <true> {code}
> The failure is triggered by
> {code:java}
> assertThat(map.containsKey("baz", true), is(false)); {code}
> Expected behavior:
> After removing the last value associated with a key, the multimap should no
> longer report that key as present.
>
> This is a container state-consistency bug. The public query methods disagree
> about whether the key still exists, and the backing map retains an empty
> bucket that should have been deleted. The @Experimental annotation may lower
> compatibility expectations, but it does not make mutually inconsistent
> observable state acceptable.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
