浏览代码

ESQL: Add Block#doesHaveMultivaluedFields (#110242)

This adds a method to `Block` which returns `true` if it contains
multivalued fields. This is like `mayHaveMultivaluedFields` except it
won't make false positives - instead it'll always find a real
multivalued field.

We use this in LOOKUP right now, but we'll want it in apache arrow
support soon too.
Nik Everett 1 年之前
父节点
当前提交
ef52f8ccb8

+ 2 - 7
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/table/RowInTableLookup.java

@@ -45,13 +45,8 @@ public abstract sealed class RowInTableLookup implements Releasable permits Empt
                     "keys must have the same number of positions but [" + positions + "] != [" + keys[k].getPositionCount() + "]"
                 );
             }
-            if (keys[k].mayHaveMultivaluedFields()) {
-                for (int p = 0; p < keys[k].getPositionCount(); p++) {
-                    if (keys[k].getValueCount(p) > 1) {
-                        // TODO double check these errors over REST once we have LOOKUP
-                        throw new IllegalArgumentException("only single valued keys are supported");
-                    }
-                }
+            if (keys[k].doesHaveMultivaluedFields()) {
+                throw new IllegalArgumentException("only single valued keys are supported");
             }
         }
         if (positions == 0) {

+ 13 - 0
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/AbstractArrayBlock.java

@@ -46,6 +46,19 @@ abstract class AbstractArrayBlock extends AbstractNonThreadSafeRefCounted implem
         return firstValueIndexes != null;
     }
 
+    @Override
+    public boolean doesHaveMultivaluedFields() {
+        if (false == mayHaveMultivaluedFields()) {
+            return false;
+        }
+        for (int p = 0; p < getPositionCount(); p++) {
+            if (getValueCount(p) > 1) {
+                return true;
+            }
+        }
+        return false;
+    }
+
     @Override
     public final MvOrdering mvOrdering() {
         return mvOrdering;

+ 5 - 0
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/AbstractVectorBlock.java

@@ -46,6 +46,11 @@ abstract class AbstractVectorBlock extends AbstractNonThreadSafeRefCounted imple
         return false;
     }
 
+    @Override
+    public boolean doesHaveMultivaluedFields() {
+        return false;
+    }
+
     @Override
     public final MvOrdering mvOrdering() {
         return MvOrdering.DEDUPLICATED_AND_SORTED_ASCENDING;

+ 12 - 1
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/Block.java

@@ -116,10 +116,21 @@ public interface Block extends Accountable, BlockLoader.Block, NamedWriteable, R
 
     /**
      * Can this block have multivalued fields? Blocks that return {@code false}
-     * will never return more than one from {@link #getValueCount}.
+     * will never return more than one from {@link #getValueCount}. This may
+     * return {@code true} for Blocks that do not have multivalued fields, but
+     * it will always answer quickly.
      */
     boolean mayHaveMultivaluedFields();
 
+    /**
+     * Does this block have multivalued fields? Unlike {@link #mayHaveMultivaluedFields}
+     * this will never return a false positive. In other words, if this returns
+     * {@code true} then there <strong>are</strong> positions for which {@link #getValueCount}
+     * will return more than 1. This will answer quickly if it can but may have
+     * to check all positions.
+     */
+    boolean doesHaveMultivaluedFields();
+
     /**
      * Creates a new block that only exposes the positions provided.
      * @param positions the positions to retain

+ 8 - 0
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/CompositeBlock.java

@@ -132,6 +132,14 @@ public final class CompositeBlock extends AbstractNonThreadSafeRefCounted implem
         return Arrays.stream(blocks).anyMatch(Block::mayHaveMultivaluedFields);
     }
 
+    @Override
+    public boolean doesHaveMultivaluedFields() {
+        if (false == Arrays.stream(blocks).anyMatch(Block::mayHaveMultivaluedFields)) {
+            return false;
+        }
+        return Arrays.stream(blocks).anyMatch(Block::doesHaveMultivaluedFields);
+    }
+
     @Override
     public CompositeBlock filter(int... positions) {
         CompositeBlock result = null;

+ 5 - 0
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ConstantNullBlock.java

@@ -68,6 +68,11 @@ final class ConstantNullBlock extends AbstractNonThreadSafeRefCounted
         return false;
     }
 
+    @Override
+    public boolean doesHaveMultivaluedFields() {
+        return false;
+    }
+
     @Override
     public ElementType elementType() {
         return ElementType.NULL;

+ 5 - 0
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/OrdinalBytesRefBlock.java

@@ -191,6 +191,11 @@ public final class OrdinalBytesRefBlock extends AbstractNonThreadSafeRefCounted
         return ordinals.mayHaveMultivaluedFields();
     }
 
+    @Override
+    public boolean doesHaveMultivaluedFields() {
+        return ordinals.mayHaveMultivaluedFields();
+    }
+
     @Override
     public MvOrdering mvOrdering() {
         return ordinals.mvOrdering();

+ 1 - 0
x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BasicBlockTests.java

@@ -184,6 +184,7 @@ public class BasicBlockTests extends ESTestCase {
             assertThat(block.mayHaveNulls(), is(false));
             assertThat(block.areAllValuesNull(), is(false));
             assertThat(block.mayHaveMultivaluedFields(), is(false));
+            assertThat(block.doesHaveMultivaluedFields(), is(false));
 
             initialBlock = block.asVector().asBlock();
         }

+ 3 - 0
x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BlockMultiValuedTests.java

@@ -66,6 +66,7 @@ public class BlockMultiValuedTests extends ESTestCase {
             }
 
             assertThat(b.block().mayHaveMultivaluedFields(), equalTo(b.values().stream().anyMatch(l -> l != null && l.size() > 1)));
+            assertThat(b.block().doesHaveMultivaluedFields(), equalTo(b.values().stream().anyMatch(l -> l != null && l.size() > 1)));
         } finally {
             b.block().close();
         }
@@ -151,6 +152,8 @@ public class BlockMultiValuedTests extends ESTestCase {
                 filtered.close();
             }
             assertThat(b.block().mayHaveMultivaluedFields(), equalTo(b.values().stream().anyMatch(l -> l != null && l.size() > 1)));
+            assertThat(b.block().doesHaveMultivaluedFields(), equalTo(b.values().stream().anyMatch(l -> l != null && l.size() > 1)));
+
         } finally {
             b.block().close();
         }