Skip to content

Commit acf8a1b

Browse files
committed
more detailed docs on ScalarFnVTable
Signed-off-by: Connor Tsui <connor.tsui20@gmail.com>
1 parent c4b9949 commit acf8a1b

2 files changed

Lines changed: 27 additions & 6 deletions

File tree

vortex-array/src/scalar_fn/vtable.rs

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,15 @@ pub trait ScalarFnVTable: 'static + Sized + Clone + Send + Sync {
9393
}
9494

9595
/// Compute the return [`DType`] of the expression if evaluated over the given input types.
96+
///
97+
/// # Preconditions
98+
///
99+
/// The length of `args` must match the [`Arity`] of this function. Callers are responsible
100+
/// for validating this (e.g., [`Expression::try_new`] checks arity at construction time).
101+
/// Implementations may assume correct arity and will panic or return nonsensical results if
102+
/// violated.
103+
///
104+
/// [`Expression::try_new`]: crate::expr::Expression::try_new
96105
fn return_dtype(&self, options: &Self::Options, args: &[DType]) -> VortexResult<DType>;
97106

98107
/// Execute the expression over the input arguments.
@@ -218,12 +227,25 @@ pub trait ScalarFnVTable: 'static + Sized + Clone + Send + Sync {
218227
true
219228
}
220229

221-
/// Returns whether this expression itself is fallible. Conservatively default to *true*.
230+
/// Returns whether this expression is semantically fallible. Conservatively defaults to
231+
/// `true`.
232+
///
233+
/// An expression is semantically fallible if there exists a set of well-typed inputs that
234+
/// causes the expression to produce an error as part of its _defined behavior_. For example,
235+
/// `checked_add` is fallible because integer overflow is a domain error, and division is
236+
/// fallible because of division by zero.
237+
///
238+
/// This does **not** include execution errors that are incidental to the implementation, such
239+
/// as canonicalization failures, memory allocation errors, or encoding mismatches. Those can
240+
/// happen to any expression and are not what this method captures.
222241
///
223-
/// An expression is runtime fallible is there is an input set that causes the expression to
224-
/// panic or return an error, for example checked_add is fallible if there is overflow.
242+
/// This property is used by optimizations that speculatively evaluate an expression over values
243+
/// that may not appear in the actual input. For example, pushing a scalar function down to a
244+
/// dictionary's values array is only safe when the function is infallible or all values are
245+
/// referenced, since a fallible function might error on a value left unreferenced after
246+
/// slicing that would never be encountered during normal evaluation.
225247
///
226-
/// Note: this is only applicable to expressions that pass type-checking
248+
/// Note: this is only applicable to expressions that pass type-checking via
227249
/// [`ScalarFnVTable::return_dtype`].
228250
fn is_fallible(&self, options: &Self::Options) -> bool {
229251
_ = options;

vortex-tensor/src/scalar_fns/cosine_similarity.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -189,8 +189,7 @@ impl ScalarFnVTable for CosineSimilarity {
189189
}
190190

191191
fn is_fallible(&self, _options: &Self::Options) -> bool {
192-
// Canonicalization of the storage arrays can fail.
193-
true
192+
false
194193
}
195194
}
196195

0 commit comments

Comments
 (0)