Only mark dictionaries as compacted when they are not nested

trinodb · Feb 22, 2022 · d10ec57 · d10ec57
1 parent 9f39f4e
commit d10ec57
Show file tree

Hide file tree

Showing 2 changed files with 16 additions and 1 deletion.
diff --git a/core/trino-spi/src/main/java/io/trino/spi/Page.java b/core/trino-spi/src/main/java/io/trino/spi/Page.java
@@ -251,7 +251,7 @@ private static List<DictionaryBlock> compactRelatedBlocks(List<DictionaryBlock>
 
             try {
                 Block compactDictionary = dictionaryBlock.getDictionary().copyPositions(dictionaryPositionsToCopy, 0, numberOfIndexes);
-                outputDictionaryBlocks.add(new DictionaryBlock(positionCount, compactDictionary, newIds, true, newDictionaryId));
+                outputDictionaryBlocks.add(new DictionaryBlock(positionCount, compactDictionary, newIds, !(compactDictionary instanceof DictionaryBlock), newDictionaryId));
             }
             catch (UnsupportedOperationException e) {
                 // ignore if copy positions is not supported for the dictionary

diff --git a/core/trino-spi/src/test/java/io/trino/spi/TestPage.java b/core/trino-spi/src/test/java/io/trino/spi/TestPage.java
@@ -30,6 +30,7 @@
 import static io.trino.spi.type.VarbinaryType.VARBINARY;
 import static org.assertj.core.api.Assertions.assertThatThrownBy;
 import static org.testng.Assert.assertEquals;
+import static org.testng.Assert.assertFalse;
 import static org.testng.Assert.assertNotEquals;
 import static org.testng.Assert.assertTrue;
 
@@ -115,6 +116,20 @@ public void testCompactDictionaryBlocks()
         assertEquals(((DictionaryBlock) page.getBlock(0)).getDictionarySourceId(), ((DictionaryBlock) page.getBlock(2)).getDictionarySourceId());
     }
 
+    @Test
+    public void testCompactNestedDictionary()
+    {
+        Slice[] expectedValues = createExpectedValues(10);
+        Block valuesBlock = createSlicesBlock(expectedValues);
+        DictionaryBlock nestedDictionary = new DictionaryBlock(valuesBlock, new int[] {0, 1, 2, 2, 4, 5});
+        DictionaryBlock dictionary = new DictionaryBlock(nestedDictionary, new int[] {2, 3, 2, 0});
+
+        Page page = new Page(dictionary);
+        page.compact();
+        // Page#compact does not unnest nested dictionaries
+        assertFalse(((DictionaryBlock) page.getBlock(0)).isCompact());
+    }
+
     @Test
     public void testGetPositions()
     {