Skip to content

Commit f338602

Browse files
nafrafoshadmi
nafraf
and
oshadmi
authored
MOD-10153: New APPLY function case() (#6332)
* WIP: Implement func_case * Add tests * Test nested case * Rename testNestedCase to testNestedCaseAndGroup * Add test for invalid apply expressions * Remove TODO comment * Fix flaky test * Test with document which doesn't have neither of the fields. * Add unit tests * Add tests for NULL and missing fields * Add more unit tests * refactor evalFuncCase and update test cases for func_case handling * Fix typo * Refactor testCaseWithLogicalOperators and testCaseWithMissingFields to improve product classification and result validation * Add test with deep nesting of case * One more simple test --------- Co-authored-by: oshadmi <omer.shadmi@redis.com>
1 parent 8e3a0c6 commit f338602

4 files changed

Lines changed: 716 additions & 0 deletions

File tree

‎src/aggregate/expr/expression.c‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,18 +20,50 @@ static void setReferenceValue(RSValue *dst, RSValue *src) {
2020
}
2121

2222
extern int func_exists(ExprEval *ctx, RSValue *argv, size_t argc, RSValue *result);
23+
extern int func_case(ExprEval *ctx, RSValue *argv, size_t argc, RSValue *result);
24+
25+
static int evalFuncCase(ExprEval *eval, const RSFunctionExpr *f, RSValue *result) {
26+
// Evaluate the condition
27+
RSValue condVal = RSVALUE_STATIC;
28+
int rc = evalInternal(eval, f->args->args[0], &condVal);
29+
if (rc != EXPR_EVAL_OK) {
30+
RSValue_Clear(&condVal);
31+
return rc;
32+
}
33+
34+
// Determine which branch to evaluate based on the condition
35+
int condition = RSValue_BoolTest(&condVal);
36+
RSValue_Clear(&condVal);
37+
38+
// Evaluate only the branch we need
39+
int branchIndex = condition ? 1 : 2;
40+
rc = evalInternal(eval, f->args->args[branchIndex], result);
41+
return rc;
42+
}
2343

2444
static int evalFunc(ExprEval *eval, const RSFunctionExpr *f, RSValue *result) {
2545
int rc = EXPR_EVAL_ERR;
2646

47+
// Special handling for func_case. The condition is evaluated to determine
48+
// which branch to take and only that branch is evaluated.
49+
// For other functions, we evaluate all arguments first.
50+
if (f->Call == func_case) {
51+
return evalFuncCase(eval, f, result);
52+
}
53+
2754
/** First, evaluate every argument */
2855
size_t nusedargs = 0;
2956
size_t nargs = f->args->len;
3057
RSValue args[nargs];
3158

59+
// Normal function evaluation
3260
for (size_t ii = 0; ii < nargs; ii++) {
3361
args[ii] = (RSValue)RSVALUE_STATIC;
3462
int internalRes = evalInternal(eval, f->args->args[ii], &args[ii]);
63+
64+
// Handle NULL values:
65+
// 1. For func_exists, always allow NULL values
66+
// 2. For all other functions, NULL values are errors
3567
if (internalRes == EXPR_EVAL_ERR ||
3668
(internalRes == EXPR_EVAL_NULL && f->Call != func_exists)) {
3769
goto cleanup;

‎src/aggregate/functions/string.c‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -276,6 +276,14 @@ int func_exists(ExprEval *ctx, RSValue *argv, size_t argc, RSValue *result) {
276276
return EXPR_EVAL_OK;
277277
}
278278

279+
int func_case(ExprEval *ctx, RSValue *argv, size_t argc, RSValue *result) {
280+
// This function is never directly called for CASE expressions
281+
// The actual implementation is in evalFuncCase in expression.c
282+
// This is just a placeholder for function registration
283+
284+
return EXPR_EVAL_OK;
285+
}
286+
279287
static int stringfunc_startswith(ExprEval *ctx, RSValue *argv, size_t argc, RSValue *result) {
280288
VALIDATE_ARG_ISSTRING("startswith", argv, 0);
281289
VALIDATE_ARG_ISSTRING("startswith", argv, 1);
@@ -340,6 +348,7 @@ void RegisterStringFunctions() {
340348
RSFunctionRegistry_RegisterFunction("to_number", func_to_number, RSValue_Number, 1, 1);
341349
RSFunctionRegistry_RegisterFunction("to_str", func_to_str, RSValue_String, 1, 1);
342350
RSFunctionRegistry_RegisterFunction("exists", func_exists, RSValue_Number, 1, 1);
351+
RSFunctionRegistry_RegisterFunction("case", func_case, RSValue_Undef, 3, 3);
343352
RSFunctionRegistry_RegisterFunction("startswith", stringfunc_startswith, RSValue_Number, 2, 2);
344353
RSFunctionRegistry_RegisterFunction("contains", stringfunc_contains, RSValue_Number, 2, 2);
345354
RSFunctionRegistry_RegisterFunction("strlen", stringfunc_strlen, RSValue_Number, 1, 1);

‎tests/cpptests/test_cpp_expr.cpp‎

Lines changed: 252 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -409,3 +409,255 @@ TEST_F(ExprTest, testPropertyFetch) {
409409
RLookupRow_Cleanup(&rr);
410410
RLookup_Cleanup(&lk);
411411
}
412+
413+
// Macro for testing expression evaluation with expected numeric result
414+
#define ASSERT_EXPR_EVAL_NUMBER(ctx_var, expected_value) \
415+
{ \
416+
ASSERT_TRUE(ctx_var) << ctx_var.error(); \
417+
ASSERT_EQ(EXPR_EVAL_OK, ctx_var.eval()); \
418+
auto res = RSValue_Dereference(&ctx_var.result()); \
419+
ASSERT_EQ(RSValue_Number, res->t); \
420+
ASSERT_EQ(expected_value, res->numval); \
421+
}
422+
423+
TEST_F(ExprTest, testEvalFuncCase) {
424+
TEvalCtx ctx;
425+
426+
// Basic case function tests - condition evaluates to true
427+
ctx.assign("case(1, 42, 99)");
428+
ASSERT_EXPR_EVAL_NUMBER(ctx, 42);
429+
430+
ctx.assign("case(0 < 1, 42, 99)");
431+
ASSERT_EXPR_EVAL_NUMBER(ctx, 42);
432+
433+
ctx.assign("case(!NULL, 100, 200)");
434+
ASSERT_EXPR_EVAL_NUMBER(ctx, 100);
435+
436+
// Basic case function tests - condition evaluates to false
437+
ctx.assign("case(0, 42, 99)");
438+
ASSERT_EXPR_EVAL_NUMBER(ctx, 99);
439+
440+
ctx.assign("case(1 > 2, 100, 200)");
441+
ASSERT_EXPR_EVAL_NUMBER(ctx, 200);
442+
443+
ctx.assign("case(NULL, 100, 200)");
444+
ASSERT_EXPR_EVAL_NUMBER(ctx, 200);
445+
446+
}
447+
448+
TEST_F(ExprTest, testEvalFuncCaseWithComparisons) {
449+
RLookup lk = {0};
450+
RLookup_Init(&lk, NULL);
451+
auto *kfoo = RLookup_GetKey(&lk, "foo", RLOOKUP_M_WRITE, RLOOKUP_F_NOFLAGS);
452+
auto *kbar = RLookup_GetKey(&lk, "bar", RLOOKUP_M_WRITE, RLOOKUP_F_NOFLAGS);
453+
RLookupRow rr = {0};
454+
RLookup_WriteOwnKey(kfoo, &rr, RS_NumVal(5));
455+
RLookup_WriteOwnKey(kbar, &rr, RS_NumVal(10));
456+
457+
TEvalCtx ctx("case(@foo < @bar, 1, 0)"); // 5 < 10 is true
458+
ASSERT_TRUE(ctx) << ctx.error();
459+
ctx.lookup = &lk;
460+
ctx.srcrow = &rr;
461+
462+
ASSERT_EQ(EXPR_EVAL_OK, ctx.bindLookupKeys());
463+
ASSERT_EXPR_EVAL_NUMBER(ctx, 1); // @foo < @bar is true, so should return 1
464+
465+
RLookupRow_Cleanup(&rr);
466+
RLookup_Cleanup(&lk);
467+
}
468+
469+
TEST_F(ExprTest, testEvalFuncCaseWithExists) {
470+
RLookup lk = {0};
471+
RLookup_Init(&lk, NULL);
472+
auto *kfoo = RLookup_GetKey(&lk, "foo", RLOOKUP_M_WRITE, RLOOKUP_F_NOFLAGS);
473+
RLookupRow rr = {0};
474+
RLookup_WriteOwnKey(kfoo, &rr, RS_NumVal(42));
475+
476+
TEvalCtx ctx("case(exists(@foo), 1, 0)"); // @foo exists
477+
ASSERT_TRUE(ctx) << ctx.error();
478+
ctx.lookup = &lk;
479+
ctx.srcrow = &rr;
480+
481+
ASSERT_EQ(EXPR_EVAL_OK, ctx.bindLookupKeys());
482+
ASSERT_EXPR_EVAL_NUMBER(ctx, 1); // @foo exists, so should return true branch (1)
483+
484+
// Test with negated exists - should return false branch
485+
TEvalCtx ctx1("case(!exists(@foo), 1, 0)"); // @foo exists, so !exists(@foo) is false
486+
ASSERT_TRUE(ctx1) << ctx1.error();
487+
ctx1.lookup = &lk;
488+
ctx1.srcrow = &rr;
489+
490+
ASSERT_EQ(EXPR_EVAL_OK, ctx1.bindLookupKeys());
491+
ASSERT_EXPR_EVAL_NUMBER(ctx1, 0); // !exists(@foo) is false, so should return false branch (0)
492+
493+
RLookupRow_Cleanup(&rr);
494+
RLookup_Cleanup(&lk);
495+
}
496+
497+
TEST_F(ExprTest, testEvalFuncCaseNested) {
498+
TEvalCtx ctx;
499+
500+
// Test nested case expressions
501+
ctx.assign("case(1, case(1, 'inner_true', 'inner_false'), 'outer_false')");
502+
ASSERT_TRUE(ctx) << ctx.error();
503+
ASSERT_EQ(EXPR_EVAL_OK, ctx.eval());
504+
auto res = RSValue_Dereference(&ctx.result());
505+
ASSERT_EQ(RSValue_String, res->t);
506+
ASSERT_STREQ("inner_true", res->strval.str);
507+
508+
ctx.assign("case(0, 'outer_true', case(1, 'nested_true', 'nested_false'))");
509+
ASSERT_TRUE(ctx) << ctx.error();
510+
ASSERT_EQ(EXPR_EVAL_OK, ctx.eval());
511+
res = RSValue_Dereference(&ctx.result());
512+
ASSERT_EQ(RSValue_String, res->t);
513+
ASSERT_STREQ("nested_true", res->strval.str);
514+
515+
ctx.assign("case(0, 'outer_true', case(0, 'nested_true', 'nested_false'))");
516+
ASSERT_TRUE(ctx) << ctx.error();
517+
ASSERT_EQ(EXPR_EVAL_OK, ctx.eval());
518+
res = RSValue_Dereference(&ctx.result());
519+
ASSERT_EQ(RSValue_String, res->t);
520+
ASSERT_STREQ("nested_false", res->strval.str);
521+
}
522+
523+
TEST_F(ExprTest, testEvalFuncCaseWithNullValues) {
524+
TEvalCtx ctx;
525+
526+
// Test case with NULL in different positions
527+
ctx.assign("case(NULL, 'true_branch', 'false_branch')");
528+
ASSERT_TRUE(ctx) << ctx.error();
529+
ASSERT_EQ(EXPR_EVAL_OK, ctx.eval());
530+
auto res = RSValue_Dereference(&ctx.result());
531+
ASSERT_EQ(RSValue_String, res->t);
532+
ASSERT_STREQ("false_branch", res->strval.str);
533+
534+
ctx.assign("case(1, NULL, 'false_branch')");
535+
ASSERT_TRUE(ctx) << ctx.error();
536+
ASSERT_EQ(EXPR_EVAL_OK, ctx.eval());
537+
res = RSValue_Dereference(&ctx.result());
538+
ASSERT_TRUE(RSValue_IsNull(res));
539+
540+
ctx.assign("case(0, 'true_branch', NULL)");
541+
ASSERT_TRUE(ctx) << ctx.error();
542+
ASSERT_EQ(EXPR_EVAL_OK, ctx.eval());
543+
res = RSValue_Dereference(&ctx.result());
544+
ASSERT_TRUE(RSValue_IsNull(res));
545+
}
546+
547+
TEST_F(ExprTest, testEvalFuncCaseErrorConditions) {
548+
TEvalCtx ctx;
549+
550+
// Test case with invalid number of arguments (should fail at parse time)
551+
ctx.assign("case()"); // Missing arguments
552+
ASSERT_FALSE(ctx) << "Should fail to parse case with only 2 arguments";
553+
554+
ctx.assign("case(1)"); // Missing second and third arguments
555+
ASSERT_FALSE(ctx) << "Should fail to parse case with only 2 arguments";
556+
557+
ctx.assign("case(1, 2)"); // Missing third argument
558+
ASSERT_FALSE(ctx) << "Should fail to parse case with only 2 arguments";
559+
560+
ctx.assign("case(1, 2, 3, 4)"); // Too many arguments
561+
ASSERT_FALSE(ctx) << "Should fail to parse case with 4 arguments";
562+
563+
// Test case with invalid function in condition
564+
ctx.assign("case(invalid_func(), 'true', 'false')");
565+
ASSERT_FALSE(ctx) << "Should fail to parse case with invalid function";
566+
}
567+
568+
TEST_F(ExprTest, testEvalFuncCaseShortCircuitEvaluation) {
569+
RLookup lk = {0};
570+
RLookup_Init(&lk, NULL);
571+
auto *kfoo = RLookup_GetKey(&lk, "foo", RLOOKUP_M_WRITE, RLOOKUP_F_NOFLAGS);
572+
RLookupRow rr = {0};
573+
RLookup_WriteOwnKey(kfoo, &rr, RS_NumVal(5));
574+
575+
TEvalCtx ctx("case(1, @foo + 10, @foo / 0)");
576+
ASSERT_TRUE(ctx) << ctx.error();
577+
ctx.lookup = &lk;
578+
ctx.srcrow = &rr;
579+
580+
// Test that only the selected branch is evaluated
581+
// When condition is true, only the true branch should be evaluated
582+
ASSERT_EQ(EXPR_EVAL_OK, ctx.bindLookupKeys());
583+
ASSERT_EXPR_EVAL_NUMBER(ctx, 15); // @foo + 10 = 5 + 10 = 15
584+
585+
RLookupRow_Cleanup(&rr);
586+
RLookup_Cleanup(&lk);
587+
}
588+
589+
TEST_F(ExprTest, testEvalFuncCaseWithDifferentTypes) {
590+
TEvalCtx ctx;
591+
592+
// Test case returning different types based on condition
593+
ctx.assign("case(1, 42, 'string_result')");
594+
ASSERT_TRUE(ctx) << ctx.error();
595+
ASSERT_EQ(EXPR_EVAL_OK, ctx.eval());
596+
auto res = RSValue_Dereference(&ctx.result());
597+
ASSERT_EQ(RSValue_Number, res->t);
598+
ASSERT_EQ(42, res->numval);
599+
600+
ctx.assign("case(0, 42, 'string_result')");
601+
ASSERT_TRUE(ctx) << ctx.error();
602+
ASSERT_EQ(EXPR_EVAL_OK, ctx.eval());
603+
res = RSValue_Dereference(&ctx.result());
604+
ASSERT_EQ(RSValue_String, res->t);
605+
ASSERT_STREQ("string_result", res->strval.str);
606+
607+
// Test with complex expressions returning different types
608+
ctx.assign("case(1, 3.14 * 2, 'pi_doubled')");
609+
ASSERT_EXPR_EVAL_NUMBER(ctx, 6.28);
610+
611+
// Test returning boolean values
612+
ctx.assign("case(1, 1==1, 2!=2)");
613+
ASSERT_TRUE(ctx) << ctx.error();
614+
ASSERT_EQ(EXPR_EVAL_OK, ctx.eval());
615+
res = RSValue_Dereference(&ctx.result());
616+
ASSERT_EQ(RSValue_Number, res->t);
617+
ASSERT_EQ(1, res->numval);
618+
619+
ctx.assign("case(0, 1==1, 2!=2)");
620+
ASSERT_EXPR_EVAL_NUMBER(ctx, 0);
621+
622+
// Error during evaluation due to missing key
623+
ctx.assign("case(1, exists(@missing), 0)");
624+
ASSERT_TRUE(ctx) << ctx.error();
625+
ASSERT_EQ(EXPR_EVAL_ERR, ctx.eval());
626+
627+
ctx.assign("case(0, 0, exists(@missing))");
628+
ASSERT_TRUE(ctx) << ctx.error();
629+
ASSERT_EQ(EXPR_EVAL_ERR, ctx.eval());
630+
}
631+
632+
TEST_F(ExprTest, testEvalFuncCaseNullComparison) {
633+
TEvalCtx ctx;
634+
635+
// Test case where condition uses comparison with NULL
636+
ctx.assign("case(NULL == NULL, 1, 0)");
637+
ASSERT_EXPR_EVAL_NUMBER(ctx, 1); // NULL == NULL should be true
638+
639+
ctx.assign("case(NULL != NULL, 1, 0)");
640+
ASSERT_EXPR_EVAL_NUMBER(ctx, 0); // NULL != NULL should be false
641+
}
642+
643+
TEST_F(ExprTest, testEvalFuncCaseWithDifferentTypeComparison) {
644+
TEvalCtx ctx;
645+
646+
// Test case where condition uses comparison with different types
647+
ctx.assign("case(1 == '1', 1, 0)");
648+
ASSERT_EXPR_EVAL_NUMBER(ctx, 1); // 1 == '1' should be true due to type coercion
649+
650+
ctx.assign("case(1 == '0', 1, 0)");
651+
ASSERT_EXPR_EVAL_NUMBER(ctx, 0); // 1 == '0' should be false
652+
653+
ctx.assign("case(1 == 'hello', 1, 0)");
654+
ASSERT_EXPR_EVAL_NUMBER(ctx, 0); // 1 == 'hello' should be false
655+
656+
ctx.assign("case(1 == NULL, 1, 0)");
657+
ASSERT_EXPR_EVAL_NUMBER(ctx, 0); // 1 == NULL should be false
658+
659+
ctx.assign("case(NULL == 'hello', 1, 0)");
660+
ASSERT_EXPR_EVAL_NUMBER(ctx, 0); // NULL == 'hello' should be false
661+
}
662+
663+
#undef ASSERT_EXPR_EVAL_NUMBER

0 commit comments

Comments
 (0)