Skip to content

Commit 43866cb

Browse files
jnthntatumcopybara-github
authored andcommitted
Add min_field_depth_for_optimization option to SelectOptimizationOptions.
PiperOrigin-RevId: 1000374223
1 parent 921dde9 commit 43866cb

3 files changed

Lines changed: 66 additions & 5 deletions

File tree

‎extensions/select_optimization.cc‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -400,8 +400,9 @@ absl::StatusOr<std::vector<SelectQualifier>> SelectInstructionsFromCall(
400400

401401
class RewriterImpl : public AstRewriterBase {
402402
public:
403-
RewriterImpl(const Ast& ast, PlannerContext& planner_context)
404-
: ast_(ast), planner_context_(planner_context) {}
403+
RewriterImpl(const Ast& ast, PlannerContext& planner_context,
404+
const SelectOptimizationOptions& options)
405+
: ast_(ast), planner_context_(planner_context), options_(options) {}
405406

406407
void PreVisitExpr(const Expr& expr) override { path_.push_back(&expr); }
407408

@@ -474,6 +475,10 @@ class RewriterImpl : public AstRewriterBase {
474475
}
475476

476477
SelectPath path = GetSelectPath(&expr);
478+
if (path.select_instructions.size() <
479+
options_.min_field_depth_for_optimization) {
480+
return false;
481+
}
477482

478483
// generate the new cel.attribute call.
479484
absl::string_view fn = path.test_only ? kCelHasField : kCelAttribute;
@@ -556,6 +561,7 @@ class RewriterImpl : public AstRewriterBase {
556561

557562
const Ast& ast_;
558563
PlannerContext& planner_context_;
564+
SelectOptimizationOptions options_;
559565
// ids of potentially optimizeable expr nodes.
560566
absl::flat_hash_map<const Expr*, QualifierInstruction> candidates_;
561567
std::vector<const Expr*> path_;
@@ -858,7 +864,7 @@ google::api::expr::runtime::FlatExprBuilder* GetFlatExprBuilder(
858864

859865
absl::Status SelectOptimizationAstUpdater::UpdateAst(PlannerContext& context,
860866
Ast& ast) const {
861-
RewriterImpl rewriter(ast, context);
867+
RewriterImpl rewriter(ast, context, options_);
862868
AstRewrite(ast.mutable_root_expr(), rewriter);
863869
return rewriter.GetProgressStatus();
864870
}
@@ -880,7 +886,7 @@ absl::Status EnableSelectOptimization(
880886
}
881887

882888
flat_expr_builder->AddAstTransform(
883-
std::make_unique<SelectOptimizationAstUpdater>());
889+
std::make_unique<SelectOptimizationAstUpdater>(options));
884890
// Add overloads for select optimization signature.
885891
// These are never bound, only used to prevent the builder from failing on
886892
// the overloads check.

‎extensions/select_optimization.h‎

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,15 @@ struct SelectOptimizationOptions {
3535
// unimplemented for a given StructType. This option is exposed for testing or
3636
// to more closely match behavior of unoptimized expressions.
3737
bool force_fallback_implementation = false;
38+
39+
// The minimum depth of a select chain to be considered for optimization.
40+
// For example, if this is set to 2, then `a.b.c` will be optimized, but
41+
// `a.b` will not be.
42+
//
43+
// This is applied in the pre-pass phase that rewrites the AST. If the
44+
// optimization is already applied by a compiler / static optimizer, this
45+
// option has no effect.
46+
int min_field_depth_for_optimization = 0;
3847
};
3948

4049
// Enable select optimization on the given RuntimeBuilder, replacing long
@@ -77,10 +86,15 @@ absl::Status EnableSelectOptimization(
7786
class SelectOptimizationAstUpdater
7887
: public google::api::expr::runtime::AstTransform {
7988
public:
80-
SelectOptimizationAstUpdater() = default;
89+
explicit SelectOptimizationAstUpdater(
90+
const SelectOptimizationOptions& options = {})
91+
: options_(options) {}
8192

8293
absl::Status UpdateAst(google::api::expr::runtime::PlannerContext& context,
8394
cel::Ast& ast) const override;
95+
96+
private:
97+
SelectOptimizationOptions options_;
8498
};
8599

86100
google::api::expr::runtime::ProgramOptimizerFactory

‎extensions/select_optimization_test.cc‎

Lines changed: 41 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,47 @@ TEST_F(SelectOptimizationTest, AstTransformComplexSelect) {
528528
ElementsAre(SelectFieldEntry(1, "child"), SelectFieldEntry(1, "child")));
529529
}
530530

531+
TEST_F(SelectOptimizationTest, AstTransformMinFieldDepth) {
532+
ASSERT_OK_AND_ASSIGN(
533+
std::unique_ptr<cel::Ast> ast,
534+
CompileForTestCase(
535+
"((false)? a.child.child : b.child).child.payload.single_int64"));
536+
537+
SelectOptimizationOptions options;
538+
options.min_field_depth_for_optimization = 2;
539+
SelectOptimizationAstUpdater updater(options);
540+
EXPECT_THAT(updater.UpdateAst(context_, *ast), IsOk());
541+
542+
const CallExpr& attr_call = ast->root_expr().call_expr();
543+
EXPECT_EQ(attr_call.function(), "cel.@attribute");
544+
545+
ASSERT_THAT(attr_call.args(), SizeIs(2));
546+
547+
EXPECT_THAT(
548+
attr_call.args()[1].list_expr().elements(),
549+
ElementsAre(SelectFieldEntry(1, "child"), SelectFieldEntry(2, "payload"),
550+
SelectFieldEntry(2, "single_int64")));
551+
552+
const Expr& operand = attr_call.args()[0];
553+
554+
EXPECT_EQ(operand.call_expr().function(), cel::builtin::kTernary);
555+
ASSERT_THAT(operand.call_expr().args(), SizeIs(3));
556+
557+
const Expr& true_branch = operand.call_expr().args()[1];
558+
559+
EXPECT_EQ(true_branch.call_expr().function(), "cel.@attribute");
560+
ASSERT_THAT(true_branch.call_expr().args(), SizeIs(2));
561+
562+
EXPECT_THAT(
563+
true_branch.call_expr().args()[1].list_expr().elements(),
564+
ElementsAre(SelectFieldEntry(1, "child"), SelectFieldEntry(1, "child")));
565+
566+
const Expr& false_branch = operand.call_expr().args()[2];
567+
EXPECT_TRUE(false_branch.has_select_expr());
568+
EXPECT_EQ(false_branch.select_expr().field(), "child");
569+
EXPECT_EQ(false_branch.select_expr().operand().ident_expr().name(), "b");
570+
}
571+
531572
TEST_F(SelectOptimizationTest, AstTransformMapIndexTraversal) {
532573
// nested_test_all_types.payload.map_string_message['$not_a_field'].bb
533574
ASSERT_OK_AND_ASSIGN(std::unique_ptr<cel::Ast> ast,

0 commit comments

Comments
 (0)