Skip to content

Commit 56a44fa

Browse files
cherylEnkiduwu-hui
cherylEnkidu
and
wu-hui
authored
Remove null / nan related operations (#15441)
Co-authored-by: wu-hui <[email protected]>
1 parent 680503d commit 56a44fa

File tree

4 files changed

+54
-140
lines changed

4 files changed

+54
-140
lines changed

‎Firestore/Swift/Source/ExpressionImplementation.swift‎

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -604,14 +604,6 @@ public extension Expression {
604604

605605
// --- Added Type Check Operations ---
606606

607-
func isNan() -> BooleanExpression {
608-
return BooleanExpression(functionName: "is_nan", args: [self])
609-
}
610-
611-
func isNil() -> BooleanExpression {
612-
return BooleanExpression(functionName: "is_null", args: [self])
613-
}
614-
615607
func exists() -> BooleanExpression {
616608
return BooleanExpression(functionName: "exists", args: [self])
617609
}
@@ -624,14 +616,6 @@ public extension Expression {
624616
return BooleanExpression(functionName: "is_absent", args: [self])
625617
}
626618

627-
func isNotNil() -> BooleanExpression {
628-
return BooleanExpression(functionName: "is_not_null", args: [self])
629-
}
630-
631-
func isNotNan() -> BooleanExpression {
632-
return BooleanExpression(functionName: "is_not_nan", args: [self])
633-
}
634-
635619
// --- Added String Operations ---
636620

637621
func join(delimiter: String) -> FunctionExpression {

‎Firestore/Swift/Source/SwiftAPI/Pipeline/Expressions/Expression.swift‎

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -635,26 +635,6 @@ public protocol Expression: Sendable {
635635
/// boolean expressions.
636636
func notEqualAny(_ arrayExpression: Expression) -> BooleanExpression
637637

638-
/// Creates an expression that checks if this expression evaluates to "NaN" (Not a Number).
639-
///
640-
/// ```swift
641-
/// // Check if the result of a calculation is NaN
642-
/// Field("value").divide(0).isNan()
643-
/// ```
644-
///
645-
/// - Returns: A new `BooleanExpression` representing the "isNaN" check.
646-
func isNan() -> BooleanExpression
647-
648-
/// Creates an expression that checks if this expression evaluates to "Nil".
649-
///
650-
/// ```swift
651-
/// // Check if the "optionalField" is null
652-
/// Field("optionalField").isNil()
653-
/// ```
654-
///
655-
/// - Returns: A new `BooleanExpression` representing the "isNil" check.
656-
func isNil() -> BooleanExpression
657-
658638
/// Creates an expression that checks if a field exists in the document.
659639
///
660640
/// ```swift
@@ -686,27 +666,6 @@ public protocol Expression: Sendable {
686666
/// - Returns: A new `BooleanExpression` representing the "isAbsent" check.
687667
func isAbsent() -> BooleanExpression
688668

689-
/// Creates an expression that checks if the result of this expression is not null.
690-
///
691-
/// ```swift
692-
/// // Check if the value of the "name" field is not null
693-
/// Field("name").isNotNil()
694-
/// ```
695-
///
696-
/// - Returns: A new `BooleanExpression` representing the "isNotNil" check.
697-
func isNotNil() -> BooleanExpression
698-
699-
/// Creates an expression that checks if the results of this expression is NOT "NaN" (Not a
700-
/// Number).
701-
///
702-
/// ```swift
703-
/// // Check if the result of a calculation is NOT NaN
704-
/// Field("value").divide(Field("count")).isNotNan() // Assuming count might be 0
705-
/// ```
706-
///
707-
/// - Returns: A new `BooleanExpr` representing the "isNotNaN" check.
708-
func isNotNan() -> BooleanExpression
709-
710669
// MARK: String Operations
711670

712671
/// Creates an expression that joins the elements of an array of strings with a given separator.

‎Firestore/Swift/Tests/Integration/PipelineTests.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2488,11 +2488,11 @@ class PipelineIntegrationTests: FSTIntegrationTestCase {
24882488
.limit(1)
24892489
.select(
24902490
[
2491-
Field("rating").isNil().as("ratingIsNull"),
2492-
Field("rating").isNan().as("ratingIsNaN"),
2491+
Field("rating").equal(Constant.nil).as("ratingIsNull"),
2492+
Field("rating").equal(Constant(Double.nan)).as("ratingIsNaN"),
24932493
Field("foo").isAbsent().as("isAbsent"),
2494-
Field("title").isNotNil().as("titleIsNotNull"),
2495-
Field("cost").isNotNan().as("costIsNotNan"),
2494+
Field("title").notEqual(Constant.nil).as("titleIsNotNull"),
2495+
Field("cost").notEqual(Constant(Double.nan)).as("costIsNotNan"),
24962496
Field("fooBarBaz").exists().as("fooBarBazExists"),
24972497
Field("title").exists().as("titleExists"),
24982498
]

‎Firestore/core/src/core/pipeline_util.cc‎

Lines changed: 50 additions & 79 deletions
Original file line numberDiff line numberDiff line change
@@ -554,87 +554,58 @@ std::shared_ptr<api::Expr> ToPipelineBooleanExpr(const Filter& filter) {
554554
const google_firestore_v1_Value& value = field_filter.value();
555555
FieldFilter::Operator op = field_filter.op();
556556

557-
if (model::IsNaNValue(value)) {
558-
auto is_nan_expr = std::make_shared<api::FunctionExpr>(
559-
"is_nan", std::vector<std::shared_ptr<api::Expr>>{api_field});
560-
if (op == FieldFilter::Operator::Equal) {
561-
return std::make_shared<api::FunctionExpr>(
562-
"and",
563-
std::vector<std::shared_ptr<api::Expr>>{exists_expr, is_nan_expr});
564-
} else { // Assuming NotEqual for IsNotNan
565-
auto is_not_nan_expr = std::make_shared<api::FunctionExpr>(
566-
"not", std::vector<std::shared_ptr<api::Expr>>{is_nan_expr});
567-
return std::make_shared<api::FunctionExpr>(
568-
"and", std::vector<std::shared_ptr<api::Expr>>{exists_expr,
569-
is_not_nan_expr});
557+
auto api_constant =
558+
std::make_shared<api::Constant>(model::DeepClone(value));
559+
std::shared_ptr<api::Expr> comparison_expr;
560+
std::string func_name;
561+
562+
switch (op) {
563+
case FieldFilter::Operator::LessThan:
564+
func_name = "lt";
565+
break;
566+
case FieldFilter::Operator::LessThanOrEqual:
567+
func_name = "lte";
568+
break;
569+
case FieldFilter::Operator::GreaterThan:
570+
func_name = "gt";
571+
break;
572+
case FieldFilter::Operator::GreaterThanOrEqual:
573+
func_name = "gte";
574+
break;
575+
case FieldFilter::Operator::Equal:
576+
func_name = "eq";
577+
break;
578+
case FieldFilter::Operator::NotEqual:
579+
func_name = "neq";
580+
break;
581+
case FieldFilter::Operator::ArrayContains:
582+
func_name = "array_contains";
583+
break;
584+
case FieldFilter::Operator::In:
585+
case FieldFilter::Operator::NotIn:
586+
case FieldFilter::Operator::ArrayContainsAny: {
587+
HARD_ASSERT(
588+
model::IsArray(value),
589+
"Value for IN, NOT_IN, ARRAY_CONTAINS_ANY must be an array.");
590+
591+
if (op == FieldFilter::Operator::In)
592+
func_name = "eq_any";
593+
else if (op == FieldFilter::Operator::NotIn)
594+
func_name = "not_eq_any";
595+
else if (op == FieldFilter::Operator::ArrayContainsAny)
596+
func_name = "array_contains_any";
597+
break;
570598
}
571-
} else if (model::IsNullValue(value)) {
572-
auto is_null_expr = std::make_shared<api::FunctionExpr>(
573-
"is_null", std::vector<std::shared_ptr<api::Expr>>{api_field});
574-
if (op == FieldFilter::Operator::Equal) {
575-
return std::make_shared<api::FunctionExpr>(
576-
"and",
577-
std::vector<std::shared_ptr<api::Expr>>{exists_expr, is_null_expr});
578-
} else { // Assuming NotEqual for IsNotNull
579-
auto is_not_null_expr = std::make_shared<api::FunctionExpr>(
580-
"not", std::vector<std::shared_ptr<api::Expr>>{is_null_expr});
581-
return std::make_shared<api::FunctionExpr>(
582-
"and", std::vector<std::shared_ptr<api::Expr>>{exists_expr,
583-
is_not_null_expr});
584-
}
585-
} else {
586-
auto api_constant =
587-
std::make_shared<api::Constant>(model::DeepClone(value));
588-
std::shared_ptr<api::Expr> comparison_expr;
589-
std::string func_name;
590-
591-
switch (op) {
592-
case FieldFilter::Operator::LessThan:
593-
func_name = "lt";
594-
break;
595-
case FieldFilter::Operator::LessThanOrEqual:
596-
func_name = "lte";
597-
break;
598-
case FieldFilter::Operator::GreaterThan:
599-
func_name = "gt";
600-
break;
601-
case FieldFilter::Operator::GreaterThanOrEqual:
602-
func_name = "gte";
603-
break;
604-
case FieldFilter::Operator::Equal:
605-
func_name = "eq";
606-
break;
607-
case FieldFilter::Operator::NotEqual:
608-
func_name = "neq";
609-
break;
610-
case FieldFilter::Operator::ArrayContains:
611-
func_name = "array_contains";
612-
break;
613-
case FieldFilter::Operator::In:
614-
case FieldFilter::Operator::NotIn:
615-
case FieldFilter::Operator::ArrayContainsAny: {
616-
HARD_ASSERT(
617-
model::IsArray(value),
618-
"Value for IN, NOT_IN, ARRAY_CONTAINS_ANY must be an array.");
619-
620-
if (op == FieldFilter::Operator::In)
621-
func_name = "eq_any";
622-
else if (op == FieldFilter::Operator::NotIn)
623-
func_name = "not_eq_any";
624-
else if (op == FieldFilter::Operator::ArrayContainsAny)
625-
func_name = "array_contains_any";
626-
break;
627-
}
628-
default:
629-
HARD_FAIL("Unexpected FieldFilter operator.");
630-
}
631-
comparison_expr = std::make_shared<api::FunctionExpr>(
632-
func_name,
633-
std::vector<std::shared_ptr<api::Expr>>{api_field, api_constant});
634-
return std::make_shared<api::FunctionExpr>(
635-
"and", std::vector<std::shared_ptr<api::Expr>>{exists_expr,
636-
comparison_expr});
599+
default:
600+
HARD_FAIL("Unexpected FieldFilter operator.");
637601
}
602+
comparison_expr = std::make_shared<api::FunctionExpr>(
603+
func_name,
604+
std::vector<std::shared_ptr<api::Expr>>{api_field, api_constant});
605+
return std::make_shared<api::FunctionExpr>(
606+
"and",
607+
std::vector<std::shared_ptr<api::Expr>>{exists_expr, comparison_expr});
608+
638609
} else if (filter.type() == FieldFilter::Type::kCompositeFilter) {
639610
const auto& composite_filter = static_cast<const CompositeFilter&>(filter);
640611
std::vector<std::shared_ptr<api::Expr>> sub_exprs;

0 commit comments

Comments
 (0)