Skip to content

Commit 87db8cd

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

2 files changed

Lines changed: 26 additions & 6 deletions

File tree

vortex-array/src/scalar_fn/vtable.rs

Lines changed: 25 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,24 @@ 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
243+
/// values that may not appear in the actual input (e.g., unreferenced dictionary values). A
244+
/// fallible expression cannot be safely pushed into such contexts because it might error on a
245+
/// value that would never be encountered during normal evaluation.
225246
///
226-
/// Note: this is only applicable to expressions that pass type-checking
247+
/// Note: this is only applicable to expressions that pass type-checking via
227248
/// [`ScalarFnVTable::return_dtype`].
228249
fn is_fallible(&self, options: &Self::Options) -> bool {
229250
_ = 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)