Skip to content

Commit 578c733

Browse files
fmeheustfmeheust
committed
Changes after Michael's code review
1 parent 756e1ad commit 578c733

3 files changed

Lines changed: 33 additions & 42 deletions

File tree

‎data/semantickernel-data-oracle/src/main/java/com/microsoft/semantickernel/data/jdbc/oracle/OracleVectorStoreQueryProvider.java‎

Lines changed: 31 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,9 @@
66
*/
77
package com.microsoft.semantickernel.data.jdbc.oracle;
88

9+
import com.fasterxml.jackson.core.JsonFactory;
910
import com.fasterxml.jackson.core.JsonProcessingException;
11+
import com.fasterxml.jackson.core.JsonGenerator;
1012
import com.fasterxml.jackson.databind.JsonNode;
1113
import com.fasterxml.jackson.databind.MapperFeature;
1214
import com.fasterxml.jackson.databind.ObjectMapper;
@@ -33,10 +35,13 @@
3335
import oracle.jdbc.OracleStatement;
3436
import oracle.jdbc.OracleTypes;
3537
import oracle.sql.TIMESTAMPTZ;
38+
import oracle.jdbc.provider.oson.OsonFactory;
3639

3740
import javax.annotation.Nonnull;
3841
import javax.annotation.concurrent.GuardedBy;
3942
import javax.sql.DataSource;
43+
import java.io.ByteArrayOutputStream;
44+
import java.io.IOException;
4045
import java.math.BigDecimal;
4146
import java.sql.Connection;
4247
import java.sql.PreparedStatement;
@@ -48,12 +53,10 @@
4853
import java.util.Arrays;
4954
import java.util.Collections;
5055
import java.util.List;
51-
import java.util.Map;
5256
import java.util.UUID;
5357
import java.util.concurrent.locks.ReentrantLock;
5458
import java.util.logging.Logger;
5559
import java.util.stream.Collectors;
56-
import java.util.stream.StreamSupport;
5760

5861
/**
5962
* JDBC Vector Store for the Oracle Database
@@ -329,57 +332,48 @@ private void setUpsertStatementValues(PreparedStatement upsertStatement, Object
329332
// Some field types require special treatment to convert the java type to the
330333
// DB type
331334
if (field instanceof VectorStoreRecordVectorField) {
332-
333-
// Convert the vector field to a string
334-
if (field.getFieldType().equals(String.class)) {
335-
String json = (valueNode == null || valueNode.isNull())
336-
? null
337-
: valueNode.asText();
338-
double[] values = (json == null)
339-
? null
340-
: objectMapper.readValue(json, double[].class);
341-
342-
int dim = ((VectorStoreRecordVectorField) field).getDimensions();
343-
if (values != null && values.length != dim) {
344-
throw new SKException("Vector dimension mismatch: expected " + dim);
345-
}
346-
347-
upsertStatement.setObject(i + 1, values, OracleTypes.VECTOR_FLOAT32);
348-
continue;
349-
}
350-
351-
// If the vector field is not set as a string convert to an array of doubles
335+
// If the vector field is not set as a string convert to an array of floats
352336
// and set the value
353337
if (!field.getFieldType().equals(String.class)) {
354-
double[] values = (valueNode == null || valueNode.isNull())
355-
? null
356-
: StreamSupport.stream((
357-
(ArrayNode)valueNode).spliterator(), false)
358-
.mapToDouble(d -> d.asDouble()).toArray();
359-
360-
upsertStatement.setObject(i + 1, values, OracleTypes.VECTOR_FLOAT32);
338+
if (valueNode != null && !valueNode.isNull() && valueNode.isArray()) {
339+
final float[] values = new float[valueNode.size()];
340+
for (int j = 0; j < ((ArrayNode)valueNode).size(); j++) {
341+
values[j] = ((ArrayNode)valueNode).get(j).floatValue();
342+
}
343+
upsertStatement.setObject(i + 1, values, OracleTypes.VECTOR_FLOAT32);
344+
} else {
345+
upsertStatement.setNull(i + 1, OracleTypes.VECTOR_FLOAT32);
346+
}
361347
continue;
362348
}
363349
} else if (field instanceof VectorStoreRecordDataField) {
364-
// Lists are stored as JSON objects, write the list as a JSON string representation
365-
// of the list.
350+
// Lists are stored as JSON objects, write the list using the JDBC OSON
351+
// extensions.
366352
if (field.getFieldType().equals(List.class)) {
367-
upsertStatement.setObject(i + 1, objectMapper.writeValueAsString(valueNode));
353+
JsonFactory osonFactory = new OsonFactory();
354+
try (ByteArrayOutputStream out = new ByteArrayOutputStream()) {
355+
try (JsonGenerator osonGen = osonFactory.createGenerator(out)) {
356+
objectMapper.writeValue(osonGen, valueNode);
357+
}
358+
upsertStatement.setBytes(i + 1, out.toByteArray());
359+
} catch (IOException ioEx) {
360+
throw new SKException("Failed to convert list to JSON value", ioEx);
361+
}
368362
continue;
369363
}
370364
// Convert UUID to string before setting the value.
371365
if (field.getFieldType().equals(UUID.class)) {
372366
upsertStatement.setObject(i + 1, valueNode.isNull() ? null : valueNode.asText());
373367
continue;
374368
}
375-
// Convert OffsetDateTime to TIMESTAMPTZ before setting the value.
369+
// Convert value node (its representations depends on Jackson JSON features)
370+
// to OffsetDateTime before setting the value.
376371
if (field.getFieldType().equals(OffsetDateTime.class)) {
377372
if (valueNode == null || valueNode.isNull()) {
378373
upsertStatement.setNull(i + 1, OracleTypes.TIMESTAMPTZ);
379374
} else {
380375
OffsetDateTime offsetDateTime = (OffsetDateTime) objectMapper.convertValue(valueNode, field.getFieldType());
381-
((OraclePreparedStatement) upsertStatement).setTIMESTAMPTZ(i + 1,
382-
TIMESTAMPTZ.of(offsetDateTime));
376+
upsertStatement.setObject(i + 1, offsetDateTime);
383377
}
384378
continue;
385379
}
@@ -388,8 +382,8 @@ private void setUpsertStatementValues(PreparedStatement upsertStatement, Object
388382
// For all other field type use setObject with the field value
389383
upsertStatement.setObject(i + 1,
390384
objectMapper.convertValue(valueNode,field.getFieldType()));
391-
} catch (SQLException | JsonProcessingException e) {
392-
throw new RuntimeException(e);
385+
} catch (SQLException e) {
386+
throw new SKException(e);
393387
}
394388
}
395389
}

‎data/semantickernel-data-oracle/src/main/java/com/microsoft/semantickernel/data/jdbc/oracle/OracleVectorStoreRecordMapper.java‎

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,9 +18,7 @@
1818
import com.microsoft.semantickernel.data.vectorstorage.options.GetRecordOptions;
1919
import com.microsoft.semantickernel.exceptions.SKException;
2020
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
21-
import oracle.jdbc.OracleResultSet;
2221
import oracle.jdbc.provider.oson.OsonModule;
23-
import oracle.sql.TIMESTAMPTZ;
2422
import java.sql.ResultSet;
2523
import java.sql.SQLException;
2624
import java.util.Map;
@@ -183,8 +181,7 @@ public OracleVectorStoreRecordMapper<Record> build() {
183181
value = resultSet.getBoolean(field.getEffectiveStorageName());
184182
break;
185183
case OracleDataTypesMapping.OFFSET_DATE_TIME:
186-
TIMESTAMPTZ timestamptz = ((OracleResultSet)resultSet).getTIMESTAMPTZ(field.getEffectiveStorageName());
187-
value = timestamptz != null ? timestamptz.offsetDateTimeValue() : null;
184+
value = resultSet.getObject(field.getEffectiveStorageName(), fieldType);
188185
break;
189186
case OracleDataTypesMapping.BYTE_ARRAY:
190187
value = resultSet.getBytes(field.getEffectiveStorageName());

‎data/semantickernel-data-oracle/src/test/java/com/microsoft/semantickernel/data/jdbc/oracle/OracleVectorStoreRecordCollectionTest.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,7 @@ public void searchWithTagFilter() {
290290
.searchAsync(SEARCH_EMBEDDINGS, options).block().getResults();
291291
assertNotNull(results);
292292
assertEquals(1, results.size());
293-
// The first hotel should be the most similar
293+
// The second hotel contains the tag we are searching for
294294
assertEquals(hotels.get(1).getId(), results.get(0).getRecord().getId());
295295
}
296296

0 commit comments

Comments
 (0)