Browse Source

ESQL: Add javadoc to some shared methods (#103337)

This adds some javadoc to some of the methods that are shared by huge
parts of ES|QL's "expression" tree. These methods have fairly
complicated contracts that play off of each other so this javadoc is
quite handy.
Nik Everett 1 year ago
parent
commit
f693d38169

+ 10 - 0
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/evaluator/mapper/EvaluatorMapper.java

@@ -26,6 +26,16 @@ import static org.elasticsearch.compute.data.BlockUtils.toJavaObject;
  * Expressions that have a mapping to an {@link ExpressionEvaluator}.
  */
 public interface EvaluatorMapper {
+    /**
+     * Build an {@link ExpressionEvaluator.Factory} for the tree of
+     * expressions rooted at this node. This is only guaranteed to return
+     * a sensible evaluator if this node has a valid type. If this node
+     * is a subclass of {@link Expression} then "valid type" means that
+     * {@link Expression#typeResolved} returns a non-error resolution.
+     * If {@linkplain Expression#typeResolved} returns an error then
+     * this method may throw. Or return an evaluator that produces
+     * garbage. Or return an evaluator that throws when run.
+     */
     ExpressionEvaluator.Factory toEvaluator(Function<Expression, ExpressionEvaluator.Factory> toEvaluator);
 
     /**

+ 38 - 0
x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/expression/Expression.java

@@ -99,6 +99,26 @@ public abstract class Expression extends Node<Expression> implements Resolvable
         return lazyChildrenResolved;
     }
 
+    /**
+     * Does the tree rooted at this expression have valid types at all nodes?
+     * <p>
+     *     For example, {@code SIN(1.2)} has a valid type and should return
+     *     {@link TypeResolution#TYPE_RESOLVED} to signal "this type is fine".
+     *     Another example, {@code SIN("cat")} has an invalid type in the
+     *     tree. The value passed to the {@code SIN} function is a string which
+     *     doesn't make any sense. So this method should return a "failure"
+     *     resolution which it can build by calling {@link TypeResolution#TypeResolution(String)}.
+     * </p>
+     * <p>
+     *     Take {@code SIN(1.2) + COS(ATAN("cat"))}, this tree should also
+     *     fail, specifically because {@code ATAN("cat")} is invalid. This should
+     *     fail even though {@code +} is perfectly valid when run on the results
+     *     of {@code SIN} and {@code COS}. And {@code COS} can operate on the results
+     *     of any valid call to {@code ATAN}. For this method to return a "valid"
+     *     result the <strong>whole</strong> tree rooted at this expression must
+     *     be valid.
+     * </p>
+     */
     public final TypeResolution typeResolved() {
         if (lazyTypeResolution == null) {
             lazyTypeResolution = resolveType();
@@ -106,6 +126,17 @@ public abstract class Expression extends Node<Expression> implements Resolvable
         return lazyTypeResolution;
     }
 
+    /**
+     * The implementation of {@link #typeResolved}, which is just a caching wrapper
+     * around this method. See it's javadoc for what this method should return.
+     * <p>
+     *     Implementations will rarely interact with the {@link TypeResolution}
+     *     class directly, instead usually calling the utility methods on {@link TypeResolutions}.
+     * </p>
+     * <p>
+     *     Implementations should fail if {@link #childrenResolved()} returns {@code false}.
+     * </p>
+     */
     protected TypeResolution resolveType() {
         return TypeResolution.TYPE_RESOLVED;
     }
@@ -142,6 +173,13 @@ public abstract class Expression extends Node<Expression> implements Resolvable
         return childrenResolved() && typeResolved().resolved();
     }
 
+    /**
+     * The {@link DataType} returned by executing the tree rooted at this
+     * expression. If {@link #typeResolved()} returns an error then the behavior
+     * of this method is undefined. It <strong>may</strong> return a valid
+     * type. Or it may throw an exception. Or it may return a totally nonsensical
+     * type.
+     */
     public abstract DataType dataType();
 
     @Override