Update PIP-445 based on review feedback

Author: namest504

pip/pip-445.md

@@ -1,4 +1,4 @@
-# PIP-445: Expose Raw Message Access in TableView API
+# PIP-445: Add Builder Methods to Create Message-based TableView
 
 # Background knowledge
 
@@ -12,60 +12,63 @@ The current `TableView` API provides a `get(String key)` method that only return
 
 For instance, a user might need to inspect the message **properties** to get a trace-id or check the **event time** to determine if the data is recent. Currently, the only way to access this metadata is to create a separate, redundant `Consumer` on the same topic, which is inefficient and undermines the convenience of using a `TableView`.
 
-This proposal aims to solve this problem by adding a method to the `TableView` API to retrieve the entire, raw `Message<T>` object.
+This proposal aims to solve this problem by providing a way to create a `TableView` that exposes the entire `Message<T>` object.
 
 # Goals
 
 ## In Scope
 
-* Add a new method, `getRawMessage(String key)`, to the `TableView<T>` interface.
-* Allow users to access the complete `Message<T>` object, including its payload, properties, and all other metadata.
-* Ensure the change is fully backward-compatible with the existing API.
+* Add new methods, `createForMessages()` and `createForMessagesAsync()`, to the `TableViewBuilder<T>` interface.
+* Allow users to create a `TableView<Message<T>>` instance, which provides access to the complete `Message<T>` object for each key, including its payload, properties, and all other metadata.
+* Ensure the change is fully backward-compatible and does not impact the performance of existing `TableView` users.
 
 ## Out of Scope
 
-* Modifying the behavior of the existing `get(String key)` method.
+* Modifying the behavior of the existing `create()` and `createAsync()` methods in the builder.
 * Changing the underlying topic compaction logic or any broker-side functionality.
 
 # High Level Design
 
-The proposed solution is a simple and non-breaking addition to the public client API.
+The proposed solution is a simple and non-breaking addition to the public client API. Instead of modifying the existing `TableView` implementation, we will introduce new methods to the `TableViewBuilder`.
 
-1.  A new method, `Message<T> getRawMessage(String key)`, will be added to the `TableView<T>` interface.
-2.  The internal implementation (`TableViewImpl`) will be updated to store the entire `Message<T>` object for each key, instead of just the deserialized value.
-3.  The existing `get(String key)` method will remain unchanged in its signature and behavior. Internally, it will now retrieve the stored `Message<T>` and simply return its value, ensuring 100% backward compatibility.
+1.  New methods, `TableView<Message<T>> createForMessages()` and `CompletableFuture<TableView<Message<T>>> createForMessagesAsync()`, will be added to the `TableViewBuilder<T>` interface.
+2.  These methods will create a new, specialized `TableView` implementation (`MessageTableViewImpl`) that stores the entire `Message<T>` object for each key.
+3.  The existing `create()` and `createAsync()` methods will continue to create the standard `TableView` that stores only the message value (`T`).
 
-This design provides the new functionality efficiently without impacting existing users.
+This opt-in design provides the new functionality efficiently without impacting the performance or behavior of existing `TableView` use cases.
 
 # Detailed Design
 
 ## Design & Implementation Details
 
 The changes will be confined to the Pulsar client library.
 
-* **Interface `org.apache.pulsar.client.api.TableView<T>`**:
-  A new method will be added to this interface.
+* **Interface `org.apache.pulsar.client.api.TableViewBuilder<T>`**:
+  New methods will be added to this interface to create a `TableView` for messages.
+
+* **Class `org.apache.pulsar.client.impl.TableViewBuilderImpl<T>`**:
+  The new `createForMessages` methods will be implemented to instantiate a new `MessageTableViewImpl`.
+
+* **New Class `org.apache.pulsar.client.impl.MessageTableViewImpl<T>`**:
+  A new class will be created that implements `TableView<Message<T>>`. It will be based on the existing `TableViewImpl` but its internal map will store `Message<T>` objects instead of just `T` values. Its `get(key)` method will return the full `Message<T>` object.
 
 * **Class `org.apache.pulsar.client.impl.TableViewImpl<T>`**:
-  The internal data structure, a `ConcurrentNavigableMap`, will be changed to store `Message<T>` objects instead of just `T` values.
-    * The message listener within `TableViewImpl` will now place the entire received `Message<T>` object into the map.
-    * The `get(key)` method will be adapted to fetch the `Message<T>` from the map and return `message.getValue()`.
-    * The new `getRawMessage(key)` method will fetch and return the `Message<T>` object directly.
+  This class will remain unchanged, ensuring no impact on existing users.
 
 ## Public-facing Changes
 
 ### Public API
 
-A new method will be added to the `org.apache.pulsar.client.api.TableView<T>` interface.
+New methods will be added to the `org.apache.pulsar.client.api.TableViewBuilder<T>` interface.
 
-* **Method Signature**:
+* **Method Signatures**:
     ```java
-    Message<T> getRawMessage(String key);
+    TableView<Message<T>> createForMessages() throws PulsarClientException;
+
+    CompletableFuture<TableView<Message<T>>> createForMessagesAsync();
     ```
-* **Description**: Retrieves the latest raw `Message<T>` object for the given key. This object contains the message payload as well as all associated metadata (properties, event time, etc.).
-* **Parameters**:
-    * `key`: The `String` key to look up.
-* **Return Value**: The `Message<T>` object associated with the key, or `null` if the key does not exist.
+* **Description**: Creates a `TableView` instance where the values in the map are the full `Message<T>` objects, including payload and metadata. This allows access to message properties, event time, etc.
+* **Return Value**: A `TableView<Message<T>>` instance.
 
 ### Binary protocol
 
@@ -95,27 +98,29 @@ This proposal has no security implications. The new method exposes message metad
 
 This change is fully backward-compatible.
 
-* The addition of a new method to an interface is a non-breaking change for existing applications. Code compiled with an older version of the client library will continue to run without issue.
+* The addition of new methods to the builder interface is a non-breaking change. Existing code that uses `create()` or `createAsync()` will continue to function as before with no performance or behavioral changes.
 
 ## Upgrade
 
-The upgrade process is seamless. Applications can update their client dependency to a version containing this feature and start using the new method without any other changes.
+The upgrade process is seamless. Applications can update their client dependency to a version containing this feature and start using the new builder methods without any other changes.
 
 ## Downgrade / Rollback
 
-A downgrade is also seamless. If an application that uses the new `getRawMessage` method is rolled back to an older client version, it will fail at compile time. Applications that do not use the new method can be rolled back without any issues.
+A downgrade is also seamless. If an application that uses the new `createForMessages` methods is rolled back to an older client version, it will fail at compile time. Applications that do not use the new methods can be rolled back without any issues.
 
 ## Pulsar Geo-Replication Upgrade & Downgrade/Rollback Considerations
 
 This is a client-side change and has no impact on geo-replication.
 
 # Alternatives
 
-An alternative considered was to add multiple specific getter methods to the `TableView` interface, such as `getProperties(String key)` and `getEventTime(String key)`.
+## Add `getRawMessage(String key)` to `TableView`
+
+An alternative considered was to add a `getRawMessage(String key)` method directly to the `TableView` interface. This would have required modifying the existing `TableViewImpl` to store the entire `Message<T>` object for all users.
 
-This approach was rejected because it would clutter the API and be less flexible. If new metadata is added to the `Message` object in the future, the `TableView` API would need to be changed again. Returning the entire `Message<T>` object is a cleaner, more efficient, and more future-proof solution.
+This approach was rejected because it would be a **breaking change in terms of performance**. It would increase memory and CPU consumption for all `TableView` users, even those who do not need access to the raw message. The proposed builder-based approach is superior as it makes this an opt-in feature, preserving the performance characteristics of the existing `TableView`.
 
 # Links
 
 * Mailing List discussion thread: TBD
-* Mailing List voting thread: TBD
+* Mailing List voting thread: TBD