Skip to content

Conversation

@suddendust
Copy link
Contributor

@suddendust suddendust commented Jan 19, 2026

Description

Key Changes:

  1. Implements createOrReplace for FlatPostgresCollection: This handles upserts for flat postgres collections. The SQL is generated as:
INSERT INTO table (col1, col2, pk_col) VALUES (?, ?, ?) ON CONFLICT (pk_col) DO UPDATE SET col1 = EXCLUDED.col1, col2 = EXCLUDED.col2 RETURNING (xmax = 0) AS is_insert

If there's a conflict on the PK column, it basically references values from the "excluded" row (EXCLUDED is a special table that references the row that would have been inserted had there been no conflict).

RETURNING (xmax = 0) AS is_insert basically returns a 0 is it was an insert, or a 1 if it was an update. This helps us to differentiate b/w inserts and updates, which is also what the createAndReturn API returns.

  1. Handles primary keys properly: The system now queries the schema registry for the PK column. If it's unable to find it, it fallbacks to they key column that is hardcoded.

  2. Adds a MissingColumnStrategy.defaultStrategy() that is SKIP currently.

@codecov
Copy link

codecov bot commented Jan 19, 2026

Codecov Report

❌ Patch coverage is 83.15789% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.44%. Comparing base (c855b61) to head (9c6d018).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...documentstore/postgres/FlatPostgresCollection.java 75.00% 13 Missing and 3 partials ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##               main     #268      +/-   ##
============================================
- Coverage     80.70%   80.44%   -0.26%     
- Complexity     1394     1396       +2     
============================================
  Files           239      239              
  Lines          6529     6598      +69     
  Branches        601      605       +4     
============================================
+ Hits           5269     5308      +39     
- Misses          868      888      +20     
- Partials        392      402      +10     
Flag Coverage Δ
integration 80.44% <83.15%> (-0.26%) ⬇️
unit 55.47% <25.26%> (-0.47%) ⬇️

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.

@suddendust suddendust changed the title [Draft] Pg write create or replace FlatPostgresCollection createOrReplace Jan 21, 2026
throw new RuntimeException("Invalid JSON format: 'statements' array not found");
}

try (Connection connection = pgDatastore.getPostgresClient()) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just a clarification, will this cause connection close each time we are done running these statements?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's AutoCloseable. Is that a concern?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so connection closed after every query/transaction? We barely utilize persistent connections?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

suresh-prakash
suresh-prakash previously approved these changes Feb 2, 2026
* @return optional of the primary key column name, or empty if no primary key is found
*/
@Override
public Optional<String> getPrimaryKeyColumn(String tableName) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we assuming that we can't have composite primary keys? We just have to make it explicit and ensure the assumption is carried forward everywhere and the client's must be aware of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Currently, we only have a single impl of Key : SingleValueKey. I will document this for now. Ultimately, we can pick up support for composite keys.

suresh-prakash
suresh-prakash previously approved these changes Feb 2, 2026
@suddendust suddendust merged commit faeac22 into hypertrace:main Feb 2, 2026
5 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants