Skip to content

Commit 2b2442c

Browse files
zhichenxu-metafacebook-github-bot
authored andcommitted
fix(optimizer): Handle CAST wrapping an RPC function call
Summary: The AIFunctionRewriteOptimizer rewrites meta.ai.embedding to CAST(fb_text_embedding(...) AS ARRAY<DOUBLE>) because fb_text_embedding returns ARRAY<REAL> but meta.ai.embedding is defined to return ARRAY<DOUBLE>. When the RpcFunctionOptimizer processes this plan, it creates an RPC node for fb_text_embedding, but the CAST of __rpc_result remains in the Project node. The VerifyProjectionLocality sanity check then fails because it expects any CallExpression in a REMOTE projection (like CAST) to be an external function (Python UDF), but the CAST from the AI rewrite is just a plain type conversion, not an external function. This causes queries with meta.ai.embedding and meta_ai_function_use_rpc_backend=true to fail with: Expect expression CAST(__rpc_result) to be an external function Fix: In RpcFunctionOptimizer.createRpcRewriter().rewriteCall(), handle the case where a CAST wraps an RPC function call. When the node is a CAST and its argument is an RPC function (e.g., fb_text_embedding), extract the inner RPC call and use the CAST target type (e.g., ARRAY<DOUBLE>) for the result variable instead of the inner call's type (e.g., ARRAY<REAL>). This eliminates the CAST from the plan, so the RPC node directly outputs the correct type, and VerifyProjectionLocality no longer sees a problematic CAST. The fix follows the same pattern as the existing TRY handling in the same method - it intercepts the CAST before the general RPC function check, processes the inner call, and returns the result variable directly. Fixes the issue where meta.ai.embedding was not usable with the RPC backend, forcing users to either use fb_text_embedding directly or disable the RPC backend and use the slower Python UDF path. Now meta.ai.embedding works with the RPC backend, enabling 20-50x speedup for embedding workloads. == RELEASE NOTES == General Changes * Fix a query failure that occurred when the result of a remote function (RPC) call was wrapped in a ``CAST`` expression. :pr:`28179` Reviewed By: sebastianopeluso Differential Revision: D112621072
1 parent a6c71c5 commit 2b2442c

1 file changed

Lines changed: 28 additions & 0 deletions

File tree

presto-main-base/src/main/java/com/facebook/presto/sql/planner/optimizations/RpcFunctionOptimizer.java

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,34 @@ && isLambdaOrBindWithLambda(node.getArguments().get(0))) {
279279
}
280280
}
281281

282+
// Handle CAST of RPC function, e.g., CAST(fb_text_embedding(...) AS ARRAY<DOUBLE>)
283+
// This happens when AIFunctionRewriteOptimizer wraps fb_text_embedding (which returns
284+
// ARRAY<REAL>) in a CAST to ARRAY<DOUBLE> to match meta.ai.embedding's return type.
285+
// We extract the inner RPC call and use the CAST target type for the result variable,
286+
// eliminating the CAST from the plan. This avoids the VerifyProjectionLocality check
287+
// which expects CASTs of RPC results to be external functions.
288+
if (node.getDisplayName().equalsIgnoreCase("CAST") && !node.getArguments().isEmpty()) {
289+
RowExpression arg = node.getArguments().get(0);
290+
if (arg instanceof CallExpression) {
291+
CallExpression innerCall = (CallExpression) arg;
292+
if (rpcFunctionNames.contains(
293+
innerCall.getDisplayName().toLowerCase(Locale.ENGLISH))) {
294+
// Rewrite children of the inner call first
295+
CallExpression rewritten = (CallExpression) treeRewriter.defaultRewrite(innerCall, context);
296+
297+
// Use the CAST target type (node.getType()) for the result variable,
298+
// not the inner call's type. This eliminates the need for the CAST.
299+
VariableReferenceExpression resultVar = variableAllocator.newVariable(
300+
"__rpc_result_",
301+
node.getType());
302+
303+
context.addCall(innerCall, rewritten, resultVar);
304+
305+
return resultVar;
306+
}
307+
}
308+
}
309+
282310
if (!rpcFunctionNames.contains(
283311
node.getDisplayName().toLowerCase(Locale.ENGLISH))) {
284312
return null;

0 commit comments

Comments
 (0)