[realppl 7] realppl integration with remote/local and unit tests #14853

wu-hui · 2025-05-13T20:03:19Z

No description provided.

gemini-code-assist · 2025-05-13T20:03:23Z

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

google-oss-bot · 2025-05-13T20:04:15Z

	1 Warning
⚠️	Did you forget to add a changelog entry? (Add #no-changelog to the PR description to silence this warning.)

Generated by 🚫 Danger

wu-hui · 2025-07-24T15:54:17Z

/gemini summary

gemini-code-assist · 2025-07-24T15:56:05Z

Summary of Changes

This pull request introduces a new query execution model based on "realtime pipelines" to enhance Firestore's query capabilities. It refactors core components to support a unified QueryOrPipeline type, allowing both traditional queries and new pipeline definitions to be processed consistently across the local store, sync engine, and remote communication layers. This lays the groundwork for more advanced query features and improves the flexibility of query processing.

Highlights

Core Query Model Enhancement: Introduced RealtimePipeline to represent a sequence of evaluable stages, and QueryOrPipeline/TargetOrPipeline as polymorphic types to encapsulate either traditional Query/Target objects or the new RealtimePipeline objects. This provides a unified approach for handling various query types.
Extensive System Refactoring: Modified numerous components across the client API, local persistence, sync engine, and remote communication layers to adopt and process the new QueryOrPipeline and TargetOrPipeline types, ensuring consistent behavior throughout the system.
Query-to-Pipeline Conversion: Implemented core logic to convert existing Query objects into a series of RealtimePipeline stages, ensuring backward compatibility and laying the groundwork for future pipeline-based optimizations and advanced query features.
Advanced Query Feature Support: Enhanced LimitStage within the pipeline model to correctly support limitToLast queries by handling negative limits during evaluation, expanding the expressive power of queries.
Pipeline Canonicalization & Testing: Added new unit tests (canonify_eq_test.cc, pipeline_util_test.cc) to validate the canonical representation, equality, and hashing of RealtimePipeline and its utility classes, which are crucial for consistent behavior in data structures like hash maps.
Local Pipeline Evaluation: Enabled the local documents view to evaluate RealtimePipeline objects directly against the local cache, including handling overlays and different pipeline source types, which is vital for offline query execution.
Serialization Updates: Updated the serialization logic to correctly encode and decode RealtimePipeline objects and their constituent stages and expressions, ensuring that pipeline definitions can be persisted and transmitted.

Changelog

Firestore/Example/Firestore.xcodeproj/project.pbxproj
- Added canonify_eq_test.cc to the Xcode project.
- Reordered some existing test file entries (timestamp_test.cc, comparison_test.cc).
Firestore/Example/Tests/API/FIRQuerySnapshotTests.mm
- Updated ViewSnapshot constructor calls to use core::QueryOrPipeline.
Firestore/Example/Tests/API/FSTAPIHelpers.mm
- Updated ViewSnapshot constructor calls to use core::QueryOrPipeline.
Firestore/Example/Tests/SpecTests/FSTMockDatastore.mm
- Updated query.target() to query.target_or_pipeline().
Firestore/Example/Tests/SpecTests/FSTSpecTests.mm
- Updated TargetData construction and access to use core::TargetOrPipeline.
Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm
- Updated QueryListener::Create calls to use core::QueryOrPipeline.
Firestore/Source/API/FIRQuery.mm
- Updated ListenToQuery to accept core::QueryOrPipeline.
Firestore/core/src/api/api_fwd.h
- Added forward declaration for class RealtimePipeline.
Firestore/core/src/api/document_reference.cc
- Updated ListenToQuery to accept core::QueryOrPipeline.
Firestore/core/src/api/expressions.cc
- Added Constant::value() getter.
Firestore/core/src/api/expressions.h
- Added const google_firestore_v1_Value& Constant::value() const;.
Firestore/core/src/api/ordering.h
- Added const std::shared_ptr<Expr> expr_shared() const;.
- Added Ordering WithReversedDirection() const;.
Firestore/core/src/api/query_core.cc
- Updated ListenToQuery to accept core::QueryOrPipeline.
Firestore/core/src/api/query_snapshot.cc
- Updated QuerySnapshot to use snapshot_.query_or_pipeline().Comparator().
Firestore/core/src/api/realtime_pipeline.cc
- Refactored RealtimePipeline constructor to use std::unique_ptr<remote::Serializer>.
- Added copy constructor and assignment operator for RealtimePipeline.
- Initialized rewritten_stages_ in the constructor using core::RewriteStages.
- Removed SetRewrittentStages method.
- Updated evaluate_context() to return EvaluateContext(serializer_.get()) and be const.
Firestore/core/src/api/realtime_pipeline.h
- Updated RealtimePipeline constructor signature.
- Added copy constructor and assignment operator declarations.
- Removed SetRewrittentStages declaration.
- Updated evaluate_context() declaration to be const.
Firestore/core/src/api/stages.cc
- Made DocumentsSource inherit from EvaluableStage and implemented its Evaluate method.
- Updated DocumentsSource to use std::set<std::string> for documents_.
- Updated LimitStage::Evaluate to handle negative limits (for limitToLast).
Firestore/core/src/api/stages.h
- Updated DocumentsSource class definition and inheritance.
- Added path() to CollectionSource and collection_id() to CollectionGroupSource.
- Added expr() to Where and limit() to LimitStage.
Firestore/core/src/core/core_fwd.h
- Added forward declaration for class TargetOrPipeline.
Firestore/core/src/core/event_manager.cc
- Updated AddQueryListener, RemoveQueryListener, OnError, OnViewSnapshots to use QueryOrPipeline.
- Added HARD_FAIL("Unimplemented") for pipeline cases in AddQueryListener and RemoveQueryListener.
Firestore/core/src/core/event_manager.h
- Updated method signatures for Listen, ListenToRemoteStore, StopListening, StopListeningToRemoteStoreOnly, and OnError to use QueryOrPipeline.
- Changed queries_ map to std::unordered_map<core::QueryOrPipeline, QueryListenersInfo>.
Firestore/core/src/core/firestore_client.cc
- Updated ListenToQuery and GetDocumentsFromLocalCache to use QueryOrPipeline.
Firestore/core/src/core/firestore_client.h
- Updated ListenToQuery method signature to use QueryOrPipeline.
Firestore/core/src/core/pipeline_run.cc
- Removed redundant pipeline.SetRewrittentStages call, as rewritten_stages_ is now initialized in the RealtimePipeline constructor.
Firestore/core/src/core/pipeline_util.cc
- Introduced QueryOrPipeline and TargetOrPipeline classes as absl::variant wrappers.
- Implemented operator==, Hash(), CanonicalId(), ToString() for both QueryOrPipeline and TargetOrPipeline.
- Implemented MatchesAllDocuments(), has_limit(), Matches(), Comparator() for QueryOrPipeline.
- Added GetLastEffectiveSortOrderings, CanonifyConstant, CanonifyExpr, CanonifySortOrderings, CanonifyStage, CanonifyPipeline helpers.
- Added PipelineFlavor, PipelineSourceType enums and associated helper functions.
- Implemented ToPipelineStages(const Query& query) to convert traditional queries to pipeline stages.
- Added ReverseOrderings and WhereConditionsFromCursor helpers.
Firestore/core/src/core/pipeline_util.h
- Defined QueryOrPipeline and TargetOrPipeline classes.
- Declared canonicalization, hashing, and comparison methods for new types.
- Declared pipeline analysis enums and helper functions.
- Declared ToPipelineStages function.
Firestore/core/src/core/query_listener.cc
- Updated QueryListener::Create and constructor to accept QueryOrPipeline.
- Updated OnViewSnapshot and RaiseInitialEvent to use snapshot.query_or_pipeline().
Firestore/core/src/core/query_listener.h
- Updated QueryListener class to use QueryOrPipeline for its query_ member and related methods.
Firestore/core/src/core/sync_engine.cc
- Updated Listen, ListenToRemoteStore, StopListening, StopListeningToRemoteStoreOnly to use QueryOrPipeline.
- Updated AllocateTarget to take TargetOrPipeline.
- Updated InitializeViewAndComputeSnapshot to take QueryOrPipeline.
- Updated QueryView class to hold QueryOrPipeline.
- Updated query_views_by_query_ and queries_by_target_ maps to use QueryOrPipeline.
Firestore/core/src/core/sync_engine.h
- Updated QueryEventSource and SyncEngine method signatures to use QueryOrPipeline and TargetOrPipeline.
- Updated QueryView and internal map types to use QueryOrPipeline.
Firestore/core/src/core/sync_engine_callback.h
- Updated OnError method signature to use QueryOrPipeline.
Firestore/core/src/core/view.cc
- Updated View constructor to take QueryOrPipeline.
- Added GetLimit, GetLimitType, GetLimitEdges helper functions to handle limits for both Query and RealtimePipeline.
- Updated ComputeDocumentChanges to use new limit helpers and query_or_pipeline.Matches().
Firestore/core/src/core/view.h
- Updated View constructor and query_ member to use QueryOrPipeline.
- Added declarations for limit helper functions.
Firestore/core/src/core/view_snapshot.cc
- Updated constructor and FromInitialDocuments to take QueryOrPipeline.
- Renamed query() getter to query_or_pipeline().
Firestore/core/src/core/view_snapshot.h
- Updated constructor and FromInitialDocuments to use QueryOrPipeline.
- Renamed query() getter to query_or_pipeline().
Firestore/core/src/local/leveldb_migrations.cc
- Updated RewriteTargetsCanonicalIds to use target_data.target_or_pipeline().CanonicalId().
Firestore/core/src/local/leveldb_remote_document_cache.cc
- Updated GetDocumentsMatchingQuery to use QueryOrPipeline and handle pipeline-specific collection path extraction.
Firestore/core/src/local/leveldb_remote_document_cache.h
- Updated GetDocumentsMatchingQuery method signatures to use QueryOrPipeline.
Firestore/core/src/local/leveldb_target_cache.cc
- Updated AddTarget, RemoveTarget, GetTarget to use TargetOrPipeline.
Firestore/core/src/local/leveldb_target_cache.h
- Updated GetTarget method signature to use TargetOrPipeline.
Firestore/core/src/local/local_documents_view.cc
- Updated GetDocumentsMatchingQuery to take QueryOrPipeline.
- Introduced GetDocumentsMatchingPipeline and RetrieveMatchingLocalDocuments helpers for local pipeline evaluation.
- Introduced GetOverlaysForPipeline to retrieve overlays for pipeline sources.
Firestore/core/src/local/local_documents_view.h
- Updated GetDocumentsMatchingQuery method signatures to use QueryOrPipeline.
- Added declarations for GetDocumentsMatchingPipeline, RetrieveMatchingLocalDocuments, and GetOverlaysForPipeline.
Firestore/core/src/local/local_serializer.cc
- Updated EncodeTargetData and DecodeTargetData to handle TargetOrPipeline and pipeline_query protobuf fields.
Firestore/core/src/local/local_store.cc
- Updated AllocateTarget, GetTargetData, ExecuteQuery to use TargetOrPipeline or QueryOrPipeline.
- Updated target_id_by_target_ map to use TargetOrPipeline.
Firestore/core/src/local/local_store.h
- Updated AllocateTarget, GetTargetData, ExecuteQuery method signatures to use TargetOrPipeline or QueryOrPipeline.
- Updated target_id_by_target_ map to use TargetOrPipeline.
Firestore/core/src/local/memory_remote_document_cache.cc
- Updated GetDocumentsMatchingQuery to use QueryOrPipeline and handle pipeline-specific collection path extraction.
Firestore/core/src/local/memory_remote_document_cache.h
- Updated GetDocumentsMatchingQuery method signatures to use QueryOrPipeline.
Firestore/core/src/local/memory_target_cache.cc
- Updated AddTarget, RemoveTarget, GetTarget to use TargetOrPipeline.
Firestore/core/src/local/memory_target_cache.h
- Updated GetTarget method signature to use TargetOrPipeline.
- Updated targets_ map to use TargetOrPipeline.
Firestore/core/src/local/query_engine.cc
- Updated all relevant methods (GetDocumentsMatchingQuery, CreateCacheIndexes, PerformQueryUsingIndex, etc.) to use QueryOrPipeline.
- Explicitly skipped pipeline indexing in CreateCacheIndexes and PerformQueryUsingIndex.
- Simplified NeedsRefill logic for pipelines.
Firestore/core/src/local/query_engine.h
- Updated all relevant method signatures to use QueryOrPipeline.
Firestore/core/src/local/remote_document_cache.h
- Updated GetDocumentsMatchingQuery method signatures to use QueryOrPipeline.
Firestore/core/src/local/target_cache.h
- Updated GetTarget method signature to use TargetOrPipeline.
Firestore/core/src/local/target_data.cc
- Updated TargetData constructor and target() getter to use TargetOrPipeline.
- Updated target_ member to TargetOrPipeline.
Firestore/core/src/local/target_data.h
- Updated TargetData class to encapsulate TargetOrPipeline.
Firestore/core/src/remote/remote_event.cc
- Added IsSingleDocumentTarget helper.
- Updated HandleExistenceFilter and CreateRemoteEvent to use TargetOrPipeline and IsSingleDocumentTarget.
Firestore/core/src/remote/remote_store.cc
- Updated RaiseWatchSnapshot to use target_data.target_or_pipeline().
Firestore/core/src/remote/serializer.cc
- Updated EncodeTarget to handle TargetOrPipeline and encode pipeline_query.
- Added EncodeRealtimePipeline to serialize RealtimePipeline objects.
- Added DecodePipelineTarget to deserialize pipeline_query.
- Added DecodeStage, DecodeExpression, DecodeFunctionExpression, DecodeOrdering helpers for deserializing pipeline components.
Firestore/core/src/remote/serializer.h
- Updated EncodeTarget method signature.
- Added declarations for EncodeRealtimePipeline, DecodePipelineTarget, and various Decode helpers for pipeline components.
Firestore/core/test/unit/core/event_manager_test.cc
- Updated test mocks and calls to use QueryOrPipeline.
Firestore/core/test/unit/core/pipeline/canonify_eq_test.cc
- New file: Contains unit tests for canonicalization and equality of RealtimePipeline and QueryOrPipeline objects.
Firestore/core/test/unit/core/pipeline/complex_test.cc
- Minor update to remove commented out code.
Firestore/core/test/unit/core/pipeline/utils.cc
- Updated TestSerializer to return std::unique_ptr<remote::Serializer>.
Firestore/core/test/unit/core/pipeline/utils.h
- Updated TestSerializer signature.
Firestore/core/test/unit/core/pipeline_util_test.cc
- New file: Contains unit tests for QueryOrPipeline and TargetOrPipeline equality, hashing, and their usage in unordered maps.
Firestore/core/test/unit/core/query_listener_test.cc
- Updated test mocks and calls to use QueryOrPipeline.
Firestore/core/test/unit/core/view_snapshot_test.cc
- Updated test calls to use QueryOrPipeline.
Firestore/core/test/unit/core/view_test.cc
- Updated test calls to use QueryOrPipeline.
Firestore/core/test/unit/local/counting_query_engine.cc
- Updated to use QueryOrPipeline.
Firestore/core/test/unit/local/counting_query_engine.h
- Updated to use QueryOrPipeline.
Firestore/core/test/unit/local/leveldb_local_store_test.cc
- Updated test suite instantiation to include use_pipeline parameter.
Firestore/core/test/unit/local/leveldb_migrations_test.cc
- Updated to use TargetOrPipeline.
Firestore/core/test/unit/local/leveldb_query_engine_test.cc
- Updated test suite instantiation to include use_pipeline parameter.
Firestore/core/test/unit/local/leveldb_target_cache_test.cc
- Updated to use TargetOrPipeline.
Firestore/core/test/unit/local/local_serializer_test.cc
- Added tests for TargetData with pipelines.
- Updated existing tests to use TargetOrPipeline.
Firestore/core/test/unit/local/local_store_test.cc
- Updated to use QueryOrPipeline and TargetOrPipeline.
- Added should_use_pipeline_ flag for tests.
Firestore/core/test/unit/local/local_store_test.h
- Updated to use QueryOrPipeline and TargetOrPipeline.
- Added should_use_pipeline_ flag and ConvertQueryToPipeline helper for tests.
Firestore/core/test/unit/local/lru_garbage_collector_test.cc
- Updated to use TargetOrPipeline.
Firestore/core/test/unit/local/memory_local_store_test.cc
- Updated test suite instantiation to include use_pipeline parameter.
Firestore/core/test/unit/local/memory_query_engine_test.cc
- Updated test suite instantiation to include use_pipeline parameter.
Firestore/core/test/unit/local/memory_target_cache.cc
- Updated to use TargetOrPipeline.
Firestore/core/test/unit/local/memory_target_cache.h
- Updated to use TargetOrPipeline.
Firestore/core/test/unit/local/query_engine_test.cc
- Updated to use QueryOrPipeline.
- Added should_use_pipeline_ flag for tests.
Firestore/core/test/unit/local/query_engine_test.h
- Updated to use QueryOrPipeline.
- Added should_use_pipeline_ flag and ConvertQueryToPipeline helper for tests.
Firestore/core/test/unit/local/remote_document_cache_test.cc
- Updated to use QueryOrPipeline.
Firestore/core/test/unit/local/target_cache_test.cc
- Updated to use TargetOrPipeline.
Firestore/core/test/unit/remote/fake_target_metadata_provider.cc
- Updated to use TargetOrPipeline.
Firestore/core/test/unit/remote/remote_event_test.cc
- Updated to use TargetOrPipeline.
Firestore/core/test/unit/remote/serializer_test.cc
- Updated to use TargetOrPipeline.
- Added tests for pipeline serialization.

Activity

The pull request was opened on 2025-05-13.
A bot comment from Gemini Code Assist was added on 2025-05-13, providing usage instructions.
Another bot comment from Danger was added on 2025-05-13, indicating a warning about a missing changelog entry.
The author wu-hui requested a summary from Gemini on 2025-07-24.

ehsannas · 2025-09-24T20:04:12Z

/gemini review

gemini-code-assist

Code Review

This is a large and significant refactoring to introduce a pipeline-based query execution model. The core of the change is the introduction of QueryOrPipeline and TargetOrPipeline variant types, which are then plumbed through the entire query execution stack. The use of absl::variant is a good choice. The testing strategy of parameterizing existing test suites to run with both Query and RealtimePipeline is excellent and provides confidence in this large change. I've found a couple of issues, one of which is critical, but overall this is a well-executed refactoring.

gemini-code-assist · 2025-09-24T20:06:52Z

Firestore/core/src/core/pipeline_util.cc

+      } else if (bound.inclusive() && sub_end == orderings.size() - 1) {
+        conditions.push_back(std::make_shared<api::FunctionExpr>(
+            func_inclusive_name,
+            std::vector<std::shared_ptr<api::Expr>>{
+                orderings[index].expr_shared(), cursors[index]}));
+      } else {


The logic for applying an inclusive cursor bound appears to be incorrect. The condition sub_end == orderings.size() - 1 causes the inclusive operator (lte/gte) to be applied to the second-to-last component of the OR condition, while the final component (when sub_end == orderings.size()) will incorrectly use the exclusive operator. This will lead to incorrect results for queries with inclusive cursors.

The condition should likely be sub_end == orderings.size() to apply the inclusive bound to the final term of the entire disjunction.

Suggested change

} else if (bound.inclusive() && sub_end == orderings.size() - 1) {

conditions.push_back(std::make_shared<api::FunctionExpr>(

func_inclusive_name,

std::vector<std::shared_ptr<api::Expr>>{

orderings[index].expr_shared(), cursors[index]}));

} else {

} else if (bound.inclusive() && sub_end == orderings.size()) {

conditions.push_back(std::make_shared<api::FunctionExpr>(

func_inclusive_name,

std::vector<std::shared_ptr<api::Expr>>{

orderings[index].expr_shared(), cursors[index]}));

gemini-code-assist · 2025-09-24T20:06:53Z

Firestore/core/src/remote/serializer.cc

+      if (current_args[i].which_value_type ==
+          google_firestore_v1_Value_string_value_tag) {
+        document_paths.push_back(DecodeString(current_args[i].string_value));
+      } else {
+        context->Fail(StringFormat(
+            "Invalid argument type for 'documents' stage at index %zu: "
+            "expected string_value, got %d",
+            i, current_args[i].which_value_type));
+        return nullptr;
+      }


There seems to be a mismatch in the serialization and deserialization of the documents pipeline stage. DocumentsSource::to_proto() encodes document paths using reference_value and google_firestore_v1_Value_reference_value_tag. However, this decoding logic expects string_value and google_firestore_v1_Value_string_value_tag.

This will cause deserialization of pipelines containing a documents stage to fail.

Suggested change

if (current_args[i].which_value_type ==

google_firestore_v1_Value_string_value_tag) {

document_paths.push_back(DecodeString(current_args[i].string_value));

} else {

context->Fail(StringFormat(

"Invalid argument type for 'documents' stage at index %zu: "

"expected string_value, got %d",

i, current_args[i].which_value_type));

return nullptr;

}

if (current_args[i].which_value_type ==

google_firestore_v1_Value_reference_value_tag) {

document_paths.push_back(DecodeString(current_args[i].reference_value));

} else {

context->Fail(StringFormat(

"Invalid argument type for 'documents' stage at index %zu: "

"expected reference_value, got %d",

i, current_args[i].which_value_type));

return nullptr;

}

Firestore/core/src/api/stages.cc

gemini-code-assist · 2025-09-24T20:06:53Z

Firestore/core/src/local/leveldb_target_cache.cc

+    auto kk = index_iterator->key();
+    (void)kk;


This appears to be leftover debugging code and should be removed.

ehsannas · 2025-09-24T20:11:13Z

Firestore/core/src/api/realtime_pipeline.cc

+    serializer_ =
+        std::make_unique<remote::Serializer>(other.serializer_->database_id());


should serializer_ be a shared_ptr instead to make this easier?

Curious why this is a unique_ptr. RealtimePipeline::operator= might not be called often, but in the AddStage function below, using a shared_ptr could improve efficiency.

ehsannas · 2025-09-24T20:17:50Z

Firestore/core/src/core/pipeline_run.cc

-  if (pipeline.rewritten_stages().empty()) {
-    pipeline.SetRewrittentStages(RewriteStages(pipeline.stages()));
-  }
-


why was this removed?

ehsannas · 2025-09-24T21:03:21Z

Firestore/core/src/core/view.cc

+    absl::optional<int64_t> limit = GetLastEffectiveLimit(query.pipeline());
+    if (limit) {
+      return limit;
+    }
+    return absl::nullopt;


all of this can be replaced with return GetLastEffectiveLimit(query.pipeline());

ehsannas · 2025-09-24T21:07:46Z

Firestore/core/src/core/view.cc

+LimitType View::GetLimitType(const QueryOrPipeline& query) {
+  if (query.IsPipeline()) {
+    absl::optional<int64_t> limit = GetLastEffectiveLimit(query.pipeline());
+    return limit > 0 ? LimitType::First : LimitType::Last;


Does this compile? 😮
I don't think operator > is defined for absl::optional. You could use value_or. something along the lines of:

return limit.value_or(0) > 0 ? LimitType::First : LimitType::Last;

ehsannas · 2025-09-24T21:23:49Z

Firestore/core/src/core/pipeline_util.h

+api::RealtimePipeline AsCollectionPipelineAtPath(
+    const api::RealtimePipeline& pipeline, const model::ResourcePath& path);
+
+absl::optional<int64_t> GetLastEffectiveLimit(


also add a comment for this function

cherylEnkidu · 2025-10-14T17:36:05Z

Firestore/Example/Tests/API/FIRQuerySnapshotTests.mm

  std::shared_ptr<Firestore> firestore = FSTTestFirestore().wrapped;
  core::Query query = Query("foo");
-  ViewSnapshot viewSnapshot(query, newDocuments, oldDocuments, std::move(documentChanges),
+  ViewSnapshot viewSnapshot(core::QueryOrPipeline(query), newDocuments, oldDocuments,


I feel it would be better to call it QueryOrRealtimePipeline since pipeline is not a part of it.

cherylEnkidu · 2025-10-14T20:51:52Z

Firestore/core/src/api/realtime_pipeline.cc

+    serializer_ =
+        std::make_unique<remote::Serializer>(other.serializer_->database_id());


Curious why this is a unique_ptr. RealtimePipeline::operator= might not be called often, but in the AddStage function below, using a shared_ptr could improve efficiency.

cherylEnkidu · 2025-10-15T14:38:25Z

Firestore/core/src/api/stages.h

+
 private:
-  std::vector<std::string> documents_;
+  std::set<std::string> documents_;


From my observation, most of the code references the documents() vector. The set type is only used once for sorting during object creation. It might be better to perform the sorting in the constructor, store the result as a vector, and have documents() return a reference to it.

cherylEnkidu · 2025-10-15T14:51:38Z

Firestore/core/src/api/stages.h

    return "where";
  }

+  const Expr* expr() const {


Any reason for using a raw pointer instead of a shared_ptr? We should generally avoid raw pointers whenever possible.

cherylEnkidu · 2025-10-15T17:09:27Z

Firestore/core/src/core/pipeline_util.cc

+    const std::vector<api::Ordering>& orderings) {
+  std::vector<api::Ordering> reversed;
+  reversed.reserve(orderings.size());
+  for (const auto& o : orderings) {


can we use std::reverse in this case?

cherylEnkidu · 2025-10-15T18:54:02Z

Firestore/core/src/local/leveldb_target_cache.cc

+    auto kk = index_iterator->key();
+    (void)kk;


google-oss-bot added the api: firestore label May 13, 2025

wu-hui force-pushed the wuandy/RealPpl_7 branch 2 times, most recently from aedab52 to 777c31f Compare May 13, 2025 20:11

wu-hui changed the title ~~[realppl 7] realppl integration with remote/local and unit/spec tests~~ [realppl 7] realppl integration with remote/local and unit tests May 13, 2025

wu-hui force-pushed the wuandy/RealPpl_7 branch from 777c31f to 7c97914 Compare May 14, 2025 23:38

wu-hui force-pushed the wuandy/RealPpl_6 branch from 7f35bbd to 0c3cc29 Compare May 14, 2025 23:38

wu-hui force-pushed the wuandy/RealPpl_7 branch from 7c97914 to 4dd578c Compare May 16, 2025 18:04

wu-hui force-pushed the wuandy/RealPpl_7 branch from 4dd578c to 6b3e105 Compare May 27, 2025 06:49

wu-hui force-pushed the wuandy/RealPpl_6 branch from 0c3cc29 to 00b7820 Compare May 27, 2025 06:49

wu-hui force-pushed the wuandy/RealPpl_7 branch from 6b3e105 to 76e5b20 Compare June 3, 2025 05:43

wu-hui force-pushed the wuandy/RealPpl_6 branch from 00b7820 to 96a4345 Compare June 3, 2025 05:43

wu-hui force-pushed the wuandy/RealPpl_7 branch from 76e5b20 to b95b616 Compare June 30, 2025 18:37

wu-hui force-pushed the wuandy/RealPpl_6 branch from 96a4345 to 63d75fb Compare June 30, 2025 18:37

wu-hui force-pushed the wuandy/RealPpl_7 branch from b95b616 to 8dd3591 Compare July 3, 2025 17:38

wu-hui force-pushed the wuandy/RealPpl_6 branch 2 times, most recently from 18f7d5c to f3752fa Compare July 7, 2025 17:19

wu-hui force-pushed the wuandy/RealPpl_7 branch from 8dd3591 to bcf6c5f Compare July 7, 2025 17:19

wu-hui force-pushed the wuandy/RealPpl_6 branch from f3752fa to 655d8cd Compare September 10, 2025 19:05

wu-hui force-pushed the wuandy/RealPpl_7 branch from bcf6c5f to 7865fe4 Compare September 11, 2025 14:39

wu-hui force-pushed the wuandy/RealPpl_6 branch from 655d8cd to aea7b2d Compare September 12, 2025 17:31

wu-hui force-pushed the wuandy/RealPpl_7 branch 2 times, most recently from 22651c5 to e0ccb49 Compare September 12, 2025 18:25

wu-hui force-pushed the wuandy/RealPpl_6 branch 2 times, most recently from 06aaef3 to 8efed9e Compare September 12, 2025 18:30

wu-hui force-pushed the wuandy/RealPpl_7 branch 2 times, most recently from 0a8821c to 9292af4 Compare September 15, 2025 15:40

wu-hui force-pushed the wuandy/RealPpl_6 branch from 8efed9e to 7c0150f Compare September 15, 2025 15:40

wu-hui force-pushed the wuandy/RealPpl_7 branch from 9292af4 to 958cee7 Compare September 15, 2025 15:42

wu-hui force-pushed the wuandy/RealPpl_6 branch from 7c0150f to 4e3240d Compare September 15, 2025 15:42

wu-hui force-pushed the wuandy/RealPpl_7 branch from 958cee7 to affb6e1 Compare September 15, 2025 15:44

wu-hui force-pushed the wuandy/RealPpl_6 branch 2 times, most recently from 9706f02 to edf7e9a Compare September 17, 2025 15:06

wu-hui force-pushed the wuandy/RealPpl_7 branch from affb6e1 to cc0e1cd Compare September 17, 2025 15:06

wu-hui requested a review from ehsannas September 17, 2025 15:07

wu-hui assigned ehsannas Sep 17, 2025

wu-hui force-pushed the wuandy/RealPpl_7 branch from cc0e1cd to 531ba2a Compare September 19, 2025 16:43

wu-hui force-pushed the wuandy/RealPpl_6 branch from edf7e9a to cb6e366 Compare September 19, 2025 16:43

wu-hui force-pushed the wuandy/RealPpl_7 branch from 531ba2a to 82b77f2 Compare September 19, 2025 16:45

wu-hui force-pushed the wuandy/RealPpl_6 branch from cb6e366 to 3abafda Compare September 19, 2025 16:45

gemini-code-assist bot reviewed Sep 24, 2025

View reviewed changes

ehsannas reviewed Sep 24, 2025

View reviewed changes

wu-hui force-pushed the wuandy/RealPpl_7 branch from 82b77f2 to a2c7960 Compare September 29, 2025 15:33

wu-hui force-pushed the wuandy/RealPpl_6 branch from 3abafda to 28cf5fa Compare September 29, 2025 15:33

Base automatically changed from wuandy/RealPpl_6 to cheryllin/ppl September 29, 2025 15:34

[realppl 7] realppl integration with remote/local and unit tests

68e64f0

wu-hui force-pushed the wuandy/RealPpl_7 branch from a2c7960 to 68e64f0 Compare September 29, 2025 15:38

cherylEnkidu self-requested a review October 14, 2025 17:14

cherylEnkidu assigned cherylEnkidu and unassigned ehsannas Oct 14, 2025

cherylEnkidu requested changes Oct 15, 2025

View reviewed changes

cherylEnkidu approved these changes Nov 19, 2025

View reviewed changes

cherylEnkidu assigned wu-hui and unassigned cherylEnkidu Nov 19, 2025

		serializer_ =
		std::make_unique<remote::Serializer>(other.serializer_->database_id());

[realppl 7] realppl integration with remote/local and unit tests #14853

Are you sure you want to change the base?

[realppl 7] realppl integration with remote/local and unit tests #14853

Uh oh!

Conversation

wu-hui commented May 13, 2025

Uh oh!

gemini-code-assist bot commented May 13, 2025

Uh oh!

google-oss-bot commented May 13, 2025

Uh oh!

wu-hui commented Jul 24, 2025

Uh oh!

gemini-code-assist bot commented Jul 24, 2025

Summary of Changes

Highlights

Uh oh!

ehsannas commented Sep 24, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gemini-code-assist bot Sep 24, 2025

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants