Store: add persistent dependency registry and notify hook by sejalpunwatkar · Pull Request #2 · BigDataAnalyticsGroup/funqdb

sejalpunwatkar · 2026-03-19T07:31:13Z

Summary

This PR introduces a persistent dependency mechanism for AttributeFunctions within the Store, enabling a foundation for observer-style updates across sessions.

What’s implemented

Added a dependency registry stored inside the existing SqliteDict using a reserved key
Implemented register_dependency(key, dependent_id) to track relationships between AttributeFunctions
Hooked into Store.put() to trigger notifications for dependent AttributeFunctions
Ensured registry persistence across Store reloads

Design decisions

Used a single-store approach (same SqliteDict) for both data and metadata to avoid cross-store consistency issues
Kept the implementation minimal and non-intrusive, without modifying existing AttributeFunction or observer semantics
Did not assume a fixed signature for update() methods to maintain compatibility with current implementations

Tests

Added test to verify dependency registration and persistence
Added test to ensure notification hook executes without breaking existing behavior

Scope

This PR focuses on establishing a persistent observer foundation. It does not enforce or redefine observer/update behavior, leaving room for future API design and refinement.

jensdittrich · 2026-03-20T07:47:12Z

tests/store/test_store.py

+    store.put(af1)
+    store.put(af2)
+
+    store.register_dependency(af1.uuid, af2.uuid)


so this is basically a dependency mechanism independent from observable/observers, this means it is up to the user to additonally register those dependencies? That sounds error-prone to me.

In general, this unit test is very short.

jensdittrich · 2026-03-20T07:48:13Z

store/store.py

+    def _get_registry(self):
+        return self.sqlite_dict[self._registry_key]
+
+    def register_dependency(self, key: int, af_id: int):


please make sure to follow the contribution guideline

jensdittrich · 2026-03-20T07:48:55Z

store/store.py

        )

+        # dependency registry (persistent)
+        self._registry_key = "__dependency_registry__"


what is the purpose of this string?

jensdittrich · 2026-03-20T07:51:55Z

tests/store/test_store.py

+    try:
+        store.put(af1)
+    except TypeError:
+        pass


You are suppressing a possible exception here? Why?

- Implement __dependency_registry__ in Store for persistent AF relationships. - Integrate _notify into Store.put for automated, cross-session updates. - Standardize on string keys for SqliteDict to ensure ID consistency. - Add lifecycle unit test with class-level mock for notification verification.

sejalpunwatkar · 2026-03-20T20:08:43Z

Hi @jensdittrich ,
Thank you for the feedback. I have updated the code to address your concerns:

Automated Notifications: I moved the notification logic into Store.put(). This means dependencies are updated automatically when an object is saved, so the user doesn't have to do it manually.
Purpose of the String: The dependency_registry string is used as a private key to save the dependency map inside the SqliteDict. This allows the relationships to be persistent and survive a store restart.
Exception Handling: I removed the try/except block. The test now uses a proper mock function that handles the expected arguments, so it no longer suppresses errors.
Improved Testing: I expanded the unit test to verify the full cycle: registering a dependency, triggering an automatic update, and checking that the data persists after closing and reopening the store.
Guidelines: I updated the variable names to use uuid.UUID and added docstrings to match the project's contribution guidelines.
Please let me know if you have further suggestions.

sejalpunwatkar · 2026-03-27T16:32:37Z

@jensdittrich I've updated the code to address your feedback, including the automated notifications and expanded tests. Please take a look whenever you have a moment!
Since you mentioned you're currently focusing on the FQL operators and the query push-down challenge, I’d love to help out there as well. Would you like me to look into the group_by rewrites or start exploring how to improve the where() API for the store?

jensdittrich · 2026-03-27T17:24:13Z

I am currently returnign from EDBT, I will take a look at your code next week.

sejalpunwatkar · 2026-03-28T14:52:52Z

Thank you for the update. I hope the conference was successful and look forward to your feedback next week. In the meantime, I’ll continue reviewing the FQL operators and the group_by logic to see where I can best assist with the next steps.

jensdittrich · 2026-03-31T08:37:38Z

sorry for the delay, I was travelling

please use the inline comments for discussions where possible, like that we can discuss each issue in a separate thred, I will follow up there now...

jensdittrich · 2026-03-31T08:39:03Z

store/store.py

        """Store an AttributeFunction in the persistent store.
        @param af: The AttributeFunction to store.
        """
+        uuid_str = str(af.uuid)


still no typehint, I would expect
uuid_str: str = ...
or
uuid: str = ...

jensdittrich · 2026-03-31T08:40:42Z

store/store.py

+        return self.sqlite_dict.get(self._registry_key, {})

-    def register_dependency(self, key: int, af_id: int):
+    def register_dependency(self, parent_uuid: uuid.UUID, child_uuid: uuid.UUID):


so the idea here is to have a separte dependency registry, i.e. all subscriptions are additionally subscribed here, correct?
Is this supposed to be redundant to the data keptin the AFs? Why not store it with the AFs directly?

Yes, the idea is to maintain a separate persistent dependency registry that mirrors the subscription relationships.

The reason for not storing dependencies directly within the AttributeFunctions is to keep the dependency mechanism at the Store level and independent of the in-memory state of AFs. This ensures that:

Dependencies persist even when AFs are not loaded in memory
Notifications can be triggered correctly after reloading the store
The internal structure of AttributeFunction does not need to be modified for persistence concerns

So the registry is not meant to be redundant, but rather a persistence layer that enables subscriptions to work transparently across sessions.

jensdittrich · 2026-03-31T08:41:02Z

store/store.py

        registry = self._get_registry()

-        key = str(key)
+        p_uuid_str = str(parent_uuid)


same typhint comment as above, fix globally

jensdittrich · 2026-03-31T08:41:31Z

store/store.py

-                dependent_af.update()
+        parent_af = self.get(parent_uuid)
+
+        for dependent_id in registry[p_uuid_str]:


typehint missing

jensdittrich · 2026-03-31T08:41:36Z

store/store.py

-            # trigger recomputation / update
-            if hasattr(dependent_af, "update"):
-                dependent_af.update()
+        parent_af = self.get(parent_uuid)


typehint missing

jensdittrich · 2026-03-31T08:42:10Z

tests/store/test_store.py

+WAS_UPDATED = False
+
+def global_update_mock(self, other=None, *args, **kwargs):
+    """Picklable global mock that accepts the 'other' argument."""


from this comment I do not understand the purpose of this, extend the explanation

why do you need a global variable here?

I’ve extended the docstring to clarify the purpose of the mock. It explains why a global flag is used to detect the internal update() call and why the function is defined at module level to remain picklable.

jensdittrich · 2026-03-31T08:43:32Z

tests/store/test_store.py

+    store.put(child_af)
+    store.put(parent_af) 
+
+    store.put(parent_af)


why are you putting it twice?, see 249

jensdittrich · 2026-03-31T08:48:44Z

tests/store/test_store.py

+    Test persistent dependency mechanism in Store.
+
+    This test covers:
+    1. Registering a dependency between two AttributeFunctions (AFs)


this is again phrased separate fro mthe subscription mechanism

I would expect the test to create AFs with subscriptions and then check through the store whether notificaitons work through dependencies even in the present of data being not available in main memory

So dependencies can be a way to enable subscriptions to work through the store. But the dependency mechanism should be invisible to the user.

tests/store/test_store.py

Store: add persistent dependency registry and notify hook

f2d0be8

jensdittrich reviewed Mar 20, 2026

View reviewed changes

jensdittrich reviewed Mar 31, 2026

View reviewed changes

tests/store/test_store.py Show resolved Hide resolved

sejalpunwatkar added 2 commits April 1, 2026 12:31

fix: address review comments (type hints, test cleanup, docstrings)

d7ea97a

Merge branch 'main' into feature/persistent-observer-store

9de4aa6

Conversation

sejalpunwatkar commented Mar 19, 2026

Summary

What’s implemented

Design decisions

Tests

Scope

Uh oh!

jensdittrich Mar 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

sejalpunwatkar commented Mar 20, 2026

Uh oh!

sejalpunwatkar commented Mar 27, 2026

Uh oh!

jensdittrich commented Mar 27, 2026

Uh oh!

sejalpunwatkar commented Mar 28, 2026

Uh oh!

jensdittrich commented Mar 31, 2026

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!

jensdittrich Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

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!

jensdittrich Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

jensdittrich Mar 20, 2026 •

edited

Loading

jensdittrich Mar 31, 2026 •

edited

Loading

jensdittrich Mar 31, 2026 •

edited

Loading