Skip to content

Commit 65909a2

Browse files
authored
Fix createOrReplace behaviour (#276)
1 parent d112d17 commit 65909a2

2 files changed

Lines changed: 40 additions & 15 deletions

File tree

document-store/src/integrationTest/java/org/hypertrace/core/documentstore/FlatCollectionWriteTest.java

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
import static org.junit.jupiter.api.Assertions.assertEquals;
55
import static org.junit.jupiter.api.Assertions.assertFalse;
66
import static org.junit.jupiter.api.Assertions.assertNotNull;
7+
import static org.junit.jupiter.api.Assertions.assertNull;
78
import static org.junit.jupiter.api.Assertions.assertThrows;
89
import static org.junit.jupiter.api.Assertions.assertTrue;
910

@@ -667,7 +668,8 @@ public Stream<? extends Arguments> provideArguments(ExtensionContext context) {
667668
class CreateOrReplaceTests {
668669

669670
@Test
670-
@DisplayName("Should create new document and return true")
671+
@DisplayName(
672+
"Should create new document and return true. Cols not specified should be set of default NULL")
671673
void testCreateOrReplaceNewDocument() throws Exception {
672674

673675
String docId = getRandomDocId(4);
@@ -691,6 +693,10 @@ void testCreateOrReplaceNewDocument() throws Exception {
691693
assertEquals("New Upsert Item", rs.getString("item"));
692694
assertEquals(500, rs.getInt("price"));
693695
assertEquals(25, rs.getInt("quantity"));
696+
// assert on some fields that they're set to null correctly
697+
assertNull(rs.getObject("sales"));
698+
assertNull(rs.getObject("categoryTags"));
699+
assertNull(rs.getObject("date"));
694700
});
695701
}
696702

@@ -714,7 +720,6 @@ void testCreateOrReplaceExistingDocument() throws Exception {
714720
ObjectNode updatedNode = OBJECT_MAPPER.createObjectNode();
715721
updatedNode.put("id", docId);
716722
updatedNode.put("item", "Updated Item");
717-
updatedNode.put("price", 999);
718723
updatedNode.put("quantity", 50);
719724
Document updatedDoc = new JSONDocument(updatedNode);
720725

@@ -727,7 +732,8 @@ void testCreateOrReplaceExistingDocument() throws Exception {
727732
rs -> {
728733
assertTrue(rs.next());
729734
assertEquals("Updated Item", rs.getString("item"));
730-
assertEquals(999, rs.getInt("price"));
735+
// this should be the default since price is not present in the updated document
736+
assertNull(rs.getObject("price"));
731737
assertEquals(50, rs.getInt("quantity"));
732738
});
733739
}

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

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -862,7 +862,14 @@ private boolean createOrReplaceWithRetry(Key key, Document document, boolean isR
862862
PostgresDataType pkType = getPrimaryKeyType(tableName, pkColumn);
863863
parsed.add(quotedPkColumn, key.toString(), pkType, false);
864864

865-
String sql = buildUpsertSql(parsed.getColumns(), quotedPkColumn);
865+
List<String> docColumns = parsed.getColumns();
866+
List<String> allColumns =
867+
schemaRegistry.getSchema(tableName).values().stream()
868+
.map(PostgresColumnMetadata::getName)
869+
.map(PostgresUtils::wrapFieldNamesWithDoubleQuotes)
870+
.collect(Collectors.toList());
871+
872+
String sql = buildCreateOrReplaceSql(allColumns, docColumns, quotedPkColumn);
866873
LOGGER.debug("Upsert SQL: {}", sql);
867874

868875
return executeUpsert(sql, parsed);
@@ -882,15 +889,16 @@ private boolean createOrReplaceWithRetry(Key key, Document document, boolean isR
882889
*
883890
* <ul>
884891
* <li>Inserts a new row if no conflict on the primary key
885-
* <li>Updates all non-PK columns if a row with the same PK already exists
892+
* <li>If the row with that PK already exists, it is replaced in entirety. Cols not present in
893+
* the latest upsert are set to their default values (as defined in the schema)
886894
* </ul>
887895
*
888896
* <p><b>Generated SQL pattern:</b>
889897
*
890898
* <pre>{@code
891-
* INSERT INTO table (col1, col2, pk_col)
899+
* INSERT INTO table (col1, col2,, col3, pk_col)
892900
* VALUES (?, ?, ?)
893-
* ON CONFLICT (pk_col) DO UPDATE SET col1 = EXCLUDED.col1, col2 = EXCLUDED.col2
901+
* ON CONFLICT (pk_col) DO UPDATE SET col1 = EXCLUDED.col1, col2 = EXCLUDED.col2, col3 = DEFAULT
894902
* RETURNING (xmax = 0) AS is_insert
895903
* }</pre>
896904
*
@@ -909,19 +917,30 @@ private boolean createOrReplaceWithRetry(Key key, Document document, boolean isR
909917
* <li>Thus, {@code is_insert = true} means INSERT, {@code is_insert = false} means UPDATE
910918
* </ul>
911919
*
912-
* @param columns List of quoted column names to include in the upsert (including PK)
920+
* @param allTableColumns all cols present in the table
921+
* @param docColumns cols present in the document
913922
* @param pkColumn The quoted primary key column name used for conflict detection
914923
* @return The complete upsert SQL statement with placeholders for values
915924
*/
916-
private String buildUpsertSql(List<String> columns, String pkColumn) {
917-
String columnList = String.join(", ", columns);
918-
String placeholders = String.join(", ", columns.stream().map(c -> "?").toArray(String[]::new));
919-
920-
// Build SET clause for non-PK columns: col = EXCLUDED.col
925+
private String buildCreateOrReplaceSql(
926+
List<String> allTableColumns, List<String> docColumns, String pkColumn) {
927+
String columnList = String.join(", ", docColumns);
928+
String placeholders =
929+
String.join(", ", docColumns.stream().map(c -> "?").toArray(String[]::new));
930+
Set<String> docColumnsSet = new HashSet<>(docColumns);
931+
932+
// Build SET clause for non-PK columns.
921933
String setClause =
922-
columns.stream()
934+
allTableColumns.stream()
923935
.filter(col -> !col.equals(pkColumn))
924-
.map(col -> col + " = EXCLUDED." + col)
936+
.map(
937+
col -> {
938+
if (docColumnsSet.contains(col)) {
939+
return col + " = EXCLUDED." + col;
940+
} else {
941+
return col + " = DEFAULT";
942+
}
943+
})
925944
.collect(Collectors.joining(", "));
926945

927946
return String.format(

0 commit comments

Comments
 (0)