Skip to content

Commit 2cb72fe

Browse files
committed
fix memory error in findnearst option
1 parent 5272a73 commit 2cb72fe

File tree

6 files changed

+59
-68
lines changed

6 files changed

+59
-68
lines changed

Firestore/Source/API/FIRPipelineBridge.mm

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@
7373
using firebase::firestore::api::Union;
7474
using firebase::firestore::api::Unnest;
7575
using firebase::firestore::api::Where;
76+
using firebase::firestore::model::DeepClone;
7677
using firebase::firestore::model::FieldPath;
78+
using firebase::firestore::nanopb::MakeSharedMessage;
7779
using firebase::firestore::nanopb::SharedMessage;
7880
using firebase::firestore::util::ComparisonResult;
7981
using firebase::firestore::util::MakeCallback;
@@ -560,7 +562,7 @@ @implementation FIRFindNearestStageBridge {
560562
FIRVectorValue *_vectorValue;
561563
NSString *_distanceMeasure;
562564
NSNumber *_limit;
563-
NSString *_Nullable _distanceField;
565+
FIRExprBridge *_Nullable _distanceField;
564566
Boolean isUserDataRead;
565567
std::shared_ptr<FindNearestStage> cpp_find_nearest;
566568
}
@@ -569,7 +571,7 @@ - (id)initWithField:(FIRFieldBridge *)field
569571
vectorValue:(FIRVectorValue *)vectorValue
570572
distanceMeasure:(NSString *)distanceMeasure
571573
limit:(NSNumber *_Nullable)limit
572-
distanceField:(NSString *_Nullable)distanceField {
574+
distanceField:(FIRExprBridge *_Nullable)distanceField {
573575
self = [super init];
574576
if (self) {
575577
_field = field;
@@ -584,21 +586,16 @@ - (id)initWithField:(FIRFieldBridge *)field
584586

585587
- (std::shared_ptr<api::Stage>)cppStageWithReader:(FSTUserDataReader *)reader {
586588
if (!isUserDataRead) {
587-
std::unordered_map<std::string,
588-
nanopb::SharedMessage<firebase::firestore::google_firestore_v1_Value>>
589-
optional_value;
589+
std::unordered_map<std::string, firebase::firestore::google_firestore_v1_Value> optional_value;
590590
if (_limit) {
591-
optional_value.emplace(
592-
std::make_pair(std::string("limit"),
593-
nanopb::SharedMessage<firebase::firestore::google_firestore_v1_Value>(
594-
[reader parsedQueryValue:_limit])));
591+
optional_value.emplace(std::make_pair(
592+
std::string("limit"), *DeepClone(*[reader parsedQueryValue:_limit]).release()));
595593
}
596594

597595
if (_distanceField) {
596+
std::shared_ptr<Expr> cpp_distance_field = [_distanceField cppExprWithReader:reader];
598597
optional_value.emplace(
599-
std::make_pair(std::string("distance_field"),
600-
nanopb::SharedMessage<firebase::firestore::google_firestore_v1_Value>(
601-
[reader parsedQueryValue:_distanceField])));
598+
std::make_pair(std::string("distance_field"), cpp_distance_field->to_proto()));
602599
}
603600

604601
FindNearestStage::DistanceMeasure::Measure measure_enum;

Firestore/Source/Public/FirebaseFirestore/FIRPipelineBridge.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -160,7 +160,7 @@ NS_SWIFT_NAME(FindNearestStageBridge)
160160
vectorValue:(FIRVectorValue *)vectorValue
161161
distanceMeasure:(NSString *)distanceMeasure
162162
limit:(NSNumber *_Nullable)limit
163-
distanceField:(NSString *_Nullable)distanceField;
163+
distanceField:(FIRExprBridge *_Nullable)distanceField;
164164
@end
165165

166166
NS_SWIFT_SENDABLE

Firestore/Swift/Source/SwiftAPI/Stages.swift

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,7 +262,7 @@ class FindNearest: Stage {
262262
vectorValue: VectorValue(vectorValue),
263263
distanceMeasure: distanceMeasure.kind.rawValue,
264264
limit: limit as NSNumber?,
265-
distanceField: distanceField
265+
distanceField: distanceField.map { Field($0).toBridge() } ?? nil
266266
)
267267
}
268268
}

Firestore/Swift/Tests/Integration/PipelineTests.swift

Lines changed: 43 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -1508,57 +1508,57 @@ class PipelineIntegrationTests: FSTIntegrationTestCase {
15081508

15091509
TestHelper.compare(pipelineSnapshot: snapshot, expected: expectedResults, enforceOrder: false)
15101510
}
1511-
1512-
func testFindNearest() async throws {
1513-
let collRef = collectionRef(withDocuments: bookDocs)
1514-
let db = collRef.firestore
1515-
1516-
let measures: [DistanceMeasure] = [.euclidean, .dotProduct, .cosine]
1517-
let expectedResults: [[String: Sendable]] = [
1518-
["title": "The Hitchhiker's Guide to the Galaxy"],
1519-
["title": "One Hundred Years of Solitude"],
1520-
["title": "The Handmaid's Tale"],
1521-
]
1522-
1523-
for measure in measures {
1524-
let pipeline = db.pipeline()
1525-
.collection(collRef.path)
1526-
.findNearest(
1527-
field: Field("embedding"),
1528-
vectorValue: [10, 1, 3, 1, 2, 1, 1, 1, 1, 1],
1529-
distanceMeasure: measure, limit: 3
1530-
)
1531-
.select("title")
1532-
let snapshot = try await pipeline.execute()
1533-
TestHelper.compare(pipelineSnapshot: snapshot, expected: expectedResults, enforceOrder: true)
1534-
}
1535-
}
15361511

1537-
func testFindNearestWithDistance() async throws {
1538-
let collRef = collectionRef(withDocuments: bookDocs)
1539-
let db = collRef.firestore
1512+
func testFindNearest() async throws {
1513+
let collRef = collectionRef(withDocuments: bookDocs)
1514+
let db = collRef.firestore
15401515

1541-
let expectedResults: [[String: Sendable]] = [
1542-
[
1543-
"title": "The Hitchhiker's Guide to the Galaxy",
1544-
"computedDistance": 1.0,
1545-
],
1546-
[
1547-
"title": "One Hundred Years of Solitude",
1548-
"computedDistance": 12.041594578792296,
1549-
],
1550-
]
1516+
let measures: [DistanceMeasure] = [.euclidean, .dotProduct, .cosine]
1517+
let expectedResults: [[String: Sendable]] = [
1518+
["title": "The Hitchhiker's Guide to the Galaxy"],
1519+
["title": "One Hundred Years of Solitude"],
1520+
["title": "The Handmaid's Tale"],
1521+
]
15511522

1523+
for measure in measures {
15521524
let pipeline = db.pipeline()
15531525
.collection(collRef.path)
15541526
.findNearest(
15551527
field: Field("embedding"),
1556-
vectorValue: [10, 1, 2, 1, 1, 1, 1, 1, 1, 1],
1557-
distanceMeasure: .euclidean, limit: 2,
1558-
distanceField: "computedDistance"
1528+
vectorValue: [10, 1, 3, 1, 2, 1, 1, 1, 1, 1],
1529+
distanceMeasure: measure, limit: 3
15591530
)
1560-
.select("title", "computedDistance")
1531+
.select("title")
15611532
let snapshot = try await pipeline.execute()
1562-
TestHelper.compare(pipelineSnapshot: snapshot, expected: expectedResults, enforceOrder: false)
1533+
TestHelper.compare(pipelineSnapshot: snapshot, expected: expectedResults, enforceOrder: true)
15631534
}
1535+
}
1536+
1537+
func testFindNearestWithDistance() async throws {
1538+
let collRef = collectionRef(withDocuments: bookDocs)
1539+
let db = collRef.firestore
1540+
1541+
let expectedResults: [[String: Sendable]] = [
1542+
[
1543+
"title": "The Hitchhiker's Guide to the Galaxy",
1544+
"computedDistance": 1.0,
1545+
],
1546+
[
1547+
"title": "One Hundred Years of Solitude",
1548+
"computedDistance": 12.041594578792296,
1549+
],
1550+
]
1551+
1552+
let pipeline = db.pipeline()
1553+
.collection(collRef.path)
1554+
.findNearest(
1555+
field: Field("embedding"),
1556+
vectorValue: [10, 1, 2, 1, 1, 1, 1, 1, 1, 1],
1557+
distanceMeasure: .euclidean, limit: 2,
1558+
distanceField: "computedDistance"
1559+
)
1560+
.select("title", "computedDistance")
1561+
let snapshot = try await pipeline.execute()
1562+
TestHelper.compare(pipelineSnapshot: snapshot, expected: expectedResults, enforceOrder: false)
1563+
}
15641564
}

Firestore/core/src/api/stages.cc

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -198,11 +198,9 @@ google_firestore_v1_Pipeline_Stage FindNearestStage::to_proto() const {
198198

199199
nanopb::SetRepeatedField(
200200
&result.options, &result.options_count, options_,
201-
[](const std::pair<std::string,
202-
nanopb::SharedMessage<google_firestore_v1_Value>>&
203-
entry) {
201+
[](const std::pair<std::string, google_firestore_v1_Value>& entry) {
204202
return _google_firestore_v1_Pipeline_Stage_OptionsEntry{
205-
nanopb::MakeBytesArray(entry.first), *DeepClone(*entry.second).release()};
203+
nanopb::MakeBytesArray(entry.first), entry.second};
206204
});
207205

208206
return result;

Firestore/core/src/api/stages.h

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -150,13 +150,11 @@ class FindNearestStage : public Stage {
150150
std::shared_ptr<Expr> property,
151151
nanopb::SharedMessage<google_firestore_v1_Value> vector,
152152
DistanceMeasure distance_measure,
153-
std::unordered_map<std::string,
154-
nanopb::SharedMessage<google_firestore_v1_Value>>
155-
options)
153+
std::unordered_map<std::string, google_firestore_v1_Value> options)
156154
: property_(std::move(property)),
157155
vector_(std::move(vector)),
158156
distance_measure_(distance_measure),
159-
options_(options) {
157+
options_(std::move(options)) {
160158
}
161159

162160
~FindNearestStage() override = default;
@@ -167,9 +165,7 @@ class FindNearestStage : public Stage {
167165
std::shared_ptr<Expr> property_;
168166
nanopb::SharedMessage<google_firestore_v1_Value> vector_;
169167
DistanceMeasure distance_measure_;
170-
std::unordered_map<std::string,
171-
nanopb::SharedMessage<google_firestore_v1_Value>>
172-
options_;
168+
std::unordered_map<std::string, google_firestore_v1_Value> options_;
173169
};
174170

175171
class LimitStage : public Stage {

0 commit comments

Comments
 (0)