Add Collection#bulkUpdate(Map<Key, Collection<SubDocumentUpdate>> updates, ..) by suddendust · Pull Request #288 · hypertrace/document-store

suddendust · 2026-04-12T16:36:58Z

Description

Since the older API Collection#bulkUpdateSubDocs is deprecated, we did not have any new method to update multiple records/docs at with its own set of updates. This change introduces this API. Also, unlike the older, deprecated API that supported only SET, this new API supports all 5 sub-doc operators.

Testing

Added integration tests

Checklist:

My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
Any dependent changes have been merged and published in downstream modules

Documentation

This method does not guarantee any atomicity. It means partial updates are allowed. FlatPostgresCollection#bulkUpdateSubDocs is not atomic as well, as per the contract.

codecov · 2026-04-12T16:39:53Z

Codecov Report

❌ Patch coverage is 81.81818% with 14 lines in your changes missing coverage. Please review.
✅ Project coverage is 81.46%. Comparing base (c445cc6) to head (5b73221).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
...documentstore/postgres/FlatPostgresCollection.java	82.89%	8 Missing and 5 partials ⚠️
.../org/hypertrace/core/documentstore/Collection.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##               main     #288      +/-   ##
============================================
- Coverage     81.51%   81.46%   -0.05%     
- Complexity     1531     1532       +1     
============================================
  Files           241      241              
  Lines          7334     7387      +53     
  Branches        702      712      +10     
============================================
+ Hits           5978     6018      +40     
- Misses          915      924       +9     
- Partials        441      445       +4

Flag	Coverage Δ
integration	`81.46% <81.81%> (-0.05%)`	⬇️
unit	`55.96% <9.09%> (-0.37%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2026-04-12T16:41:00Z

Test Results

121 files +1 121 suites +1 35s ⏱️ -1s
812 tests +2 811 ✅ +2 1 💤 ±0 0 ❌ ±0
1 149 runs +2 1 148 ✅ +2 1 💤 ±0 0 ❌ ±0

Results for commit 5b73221. ± Comparison against base commit c445cc6.

♻️ This comment has been updated with latest results.

puneet-traceable · 2026-04-13T09:51:40Z

...t-store/src/main/java/org/hypertrace/core/documentstore/postgres/FlatPostgresCollection.java

+      }
+
+      try {
+        boolean updated = updateSingleKey(key, keyUpdates, tableName, quotedPkColumn);


I see that this method tries getting a connection from pool per query. would it make sense to fire this on the same connection?

Oh, you mean use the same connection for the entire batch? Yes, that's a good optimisation. Will change this.

This is done

suresh-prakash · 2026-04-13T11:11:04Z

document-store/src/main/java/org/hypertrace/core/documentstore/Collection.java

+   * <p>This method supports all update operators (SET, UNSET, ADD, APPEND_TO_LIST,
+   * ADD_TO_LIST_IF_ABSENT, REMOVE_ALL_FROM_LIST). Updates for each individual key are applied
+   * atomically, but there is no atomicity guarantee across different keys - some keys may be
+   * updated while others fail. Any atomicity guarantees are implementation-specific.


Any atomicity guarantees are implementation-specific.

This violates the Liskov's substitution principle ('L' in SOLID). This has severe implications, which breaks the purpose of having the document store abstraction in the first place, because the implementors now have to make their client code work differently for different underlying datastore. And, honestly, if people miss to read this code comment, and expect the same client-code to work perfectly fine in the other world (which is highly likely), it can lead to subtle bugs, hard to debug and reproduce.

The older methods (like update) have in fact violated this leading to the introduction of newer consistent methods. Perhaps, the behaviour can be restrictive (to at least the known databases which has the most restrictive atomicity support), but has to be consistent.

@suresh-prakash Thanks for the detailed comment, totally agree with you. I feel batch-level atomicity could be controlled by adding them in the UpdateOptions. Or, this method's contract can say that batch-level atomicity is not guaranteed, while per-key update atomicity has to be guaranteed. This is the least-restrictive contract that almost any implementation can implement. If any implementation wants to provide batch-level atomicity guarantees, that's even better.

Wdyt?

batch-level atomicity is not guaranteed, while per-key update atomicity has to be guaranteed

Yes. This makes total sense.

If any implementation wants to provide batch-level atomicity guarantees, that's even better.

Better definitely in terms of performance. But, that can create functional digression, especially when one of the multiple updates is invalid. In one case (supporting per-key atomicity), documents before that key would have been updated. While, in another case (supporting batch-level atomicity), the entire update would fail.

If we are using document-store, all implementors have to be consistent as much as possible in terms of functionality. Performance would (and can) differ based on the underlying database used, indexes created, etc.

From this standpoint, I think, it would make sense to implement Postgres also perform key-level atomic updates (at least by default). If performance is critical the client can opt-in as you mentioned via. UpdateOptions with the implications explicitly known.

Basically, the design goal here is that the caller/client should be aware of the implications even when they missed to read out these comments and hence prefer cross-database consistency over performance (since the performance differences are evident and inevitable).

Makes sense, I have updated the doc.

suresh-prakash · 2026-04-14T07:29:06Z

...t-store/src/main/java/org/hypertrace/core/documentstore/postgres/FlatPostgresCollection.java

+    Set<Key> updatedKeys = new HashSet<>();
+
+    try (Connection connection = client.getPooledConnection()) {
+      for (Map.Entry<Key, Collection<SubDocumentUpdate>> entry : updates.entrySet()) {


Based on our conversation @ #288 (comment), seems like this is making the whole operations (batch-level) atomic (having a different variant than the Mongo implementation to come in the future).

Could you please check?

Is it? For batch-level transactions, we would acquire a client.getTransactionalConnection(). Might be wrong here but afai understand it's atomic for each key, not the entire batch.

My bad. I thought you are doing transactions here. You are right.

suresh-prakash · 2026-04-14T08:07:14Z

...t-store/src/main/java/org/hypertrace/core/documentstore/postgres/FlatPostgresCollection.java

+    Set<Key> updatedKeys = new HashSet<>();
+
+    try (Connection connection = client.getPooledConnection()) {
+      for (Map.Entry<Key, Collection<SubDocumentUpdate>> entry : updates.entrySet()) {


My bad. I thought you are doing transactions here. You are right.

WIP

ad828f4

suddendust requested review from avinashkolluru, kotharironak, puneet-traceable, skjindal93 and suresh-prakash as code owners April 12, 2026 16:37

WIP

b6c0237

puneet-traceable reviewed Apr 13, 2026

View reviewed changes

Execute entire batch on a single pooled connection

8679979

suresh-prakash reviewed Apr 13, 2026

View reviewed changes

Update javadocs

5b73221

suresh-prakash reviewed Apr 14, 2026

View reviewed changes

suresh-prakash approved these changes Apr 14, 2026

View reviewed changes

suddendust merged commit 045fac2 into main Apr 14, 2026
7 checks passed

suddendust deleted the pg_bulkUpdateSubDocs branch April 14, 2026 08:17

Conversation

suddendust commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Testing

Checklist:

Documentation

Uh oh!

codecov bot commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions bot commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

puneet-traceable Apr 13, 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!

suddendust Apr 13, 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!

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!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

suddendust commented Apr 12, 2026 •

edited

Loading

codecov bot commented Apr 12, 2026 •

edited

Loading

github-actions bot commented Apr 12, 2026 •

edited

Loading

puneet-traceable Apr 13, 2026 •

edited

Loading

suddendust Apr 13, 2026 •

edited

Loading