Skip to content

Commit 73bbbd9

Browse files
natashasehgalfacebook-github-bot
authored andcommitted
refactor: CastExpr queries CastRulesRegistry before operator methods (#17012)
Summary: CastExpr::applyPeeled() now checks CastRulesRegistry::canCast() before falling back to the per-operator isSupportedFromType()/isSupportedToType() methods. This makes the registry the primary source of truth for cast validation, with operator methods as a fallback for types not yet fully migrated to the registry (e.g., JSON container types like ARRAY<X>→JSON that require recursive type checking). This is a no-op behavioral change: the registry answers match what operators would answer for all registered types. The fallback path ensures JSON container casts and any future types with complex validation continue to work. Differential Revision: D99232107
1 parent 45f6093 commit 73bbbd9

File tree

2 files changed

+17
-10
lines changed

2 files changed

+17
-10
lines changed

velox/expression/CastExpr.cpp

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
#include "velox/expression/ScopedVarSetter.h"
2727
#include "velox/external/tzdb/time_zone.h"
2828
#include "velox/functions/lib/RowsTranslationUtil.h"
29+
#include "velox/type/CastRegistry.h"
2930
#include "velox/type/Type.h"
3031
#include "velox/type/tz/TimeZoneMap.h"
3132
#include "velox/vector/ComplexVector.h"
@@ -782,15 +783,19 @@ void CastExpr::applyPeeled(
782783
const TypePtr& toType,
783784
VectorPtr& result) {
784785
auto castFromOperator = getCastOperator(fromType);
785-
if (castFromOperator && !castFromOperator->isSupportedToType(toType)) {
786-
VELOX_USER_FAIL(
787-
"Cannot cast {} to {}.", fromType->toString(), toType->toString());
788-
}
789-
790786
auto castToOperator = getCastOperator(toType);
791-
if (castToOperator && !castToOperator->isSupportedFromType(fromType)) {
792-
VELOX_USER_FAIL(
793-
"Cannot cast {} to {}.", fromType->toString(), toType->toString());
787+
788+
// Check CastRulesRegistry first — it has all registered cast rules.
789+
// If no rule exists (e.g., JSON container types), fall back to operator.
790+
if (!CastRulesRegistry::instance().canCast(fromType, toType)) {
791+
if (castFromOperator && !castFromOperator->isSupportedToType(toType)) {
792+
VELOX_USER_FAIL(
793+
"Cannot cast {} to {}.", fromType->toString(), toType->toString());
794+
}
795+
if (castToOperator && !castToOperator->isSupportedFromType(fromType)) {
796+
VELOX_USER_FAIL(
797+
"Cannot cast {} to {}.", fromType->toString(), toType->toString());
798+
}
794799
}
795800

796801
if (castFromOperator || castToOperator) {

velox/expression/CastExpr.h

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,13 @@ class CastOperator {
2929
virtual ~CastOperator() = default;
3030

3131
/// Determines whether the cast operator supports casting to the custom type
32-
/// from the other type.
32+
/// from the other type. CastRulesRegistry is checked first; this method is
33+
/// only called as a fallback when no registry rule exists.
3334
virtual bool isSupportedFromType(const TypePtr& other) const = 0;
3435

3536
/// Determines whether the cast operator supports casting from the custom type
36-
/// to the other type.
37+
/// to the other type. CastRulesRegistry is checked first; this method is
38+
/// only called as a fallback when no registry rule exists.
3739
virtual bool isSupportedToType(const TypePtr& other) const = 0;
3840

3941
/// Casts an input vector to the custom type. This function should not throw

0 commit comments

Comments
 (0)