Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

oracle.soda.OracleOperationBuilder#mergeOne sets json_document to null #14

Open
aoksenholt opened this issue Mar 2, 2021 · 9 comments

Comments

@aoksenholt
Copy link

aoksenholt commented Mar 2, 2021

When doing
collection.find().key(key).mergeOne(documentToMerge)
where documentToMerge.contentAsString.length > 4000 the json_document column is set to null without any exception or error
The JSON_DOCUMENT column is of type BLOB, see metadata below.
Jdbc-driver used: ojdbc8

Should oracle.soda.rdbms.impl.TableCollectionImpl#setPayloadBlobWorkaround be used at https://github.com/oracle/soda-for-java/blob/master/src/oracle/soda/rdbms/impl/TableCollectionImpl.java#L2410 instead of oracle.soda.rdbms.impl.TableCollectionImpl#setPayloadBlob ?

{
  "schemaName": "OUR_SCHEMA_NAME",
  "tableName": "OurCollection",
  "keyColumn": {
    "name": "ID",
    "sqlType": "VARCHAR2",
    "maxLength": 255,
    "assignmentMethod": "CLIENT"
  },
  "contentColumn": {
    "name": "JSON_DOCUMENT",
    "sqlType": "BLOB",
    "compress": "NONE",
    "cache": true,
    "encrypt": "NONE",
    "validation": "STANDARD"
  },
  "versionColumn": {
    "name": "VERSION",
    "type": "String",
    "method": "SHA256"
  },
  "lastModifiedColumn": {
    "name": "LAST_MODIFIED"
  },
  "creationTimeColumn": {
    "name": "CREATED_ON"
  },
  "readOnly": false
}
@morgiyan
Copy link
Member

morgiyan commented Mar 2, 2021

We'll take a look and get back to you, can you please let us know which database release you're trying this against?

@petteja
Copy link

petteja commented Mar 2, 2021

This is 18.12.0.0.201020.

@morgiyan
Copy link
Member

morgiyan commented Mar 2, 2021

Thanks, we'll take a look and get back to you asap.

@morgiyan
Copy link
Member

morgiyan commented Mar 3, 2021

Reproduced and identified the issue, working on a fix (it's not in the setPayload ... methods). Our apologies for this bug. I'll post an update here very soon.

@morgiyan
Copy link
Member

morgiyan commented Mar 5, 2021

@aoksenholt @petteja we're working on getting the a new SODA jar out with this fix. Have you compiled SODA from source? If so, would it be helpful to commit the source file fix sooner for you guys, so that you can compile from source? Or would you prefer to just wait until we publish the new jar?

@aoksenholt
Copy link
Author

We're doing a workaround using replace instead of merge (manually handling the merge), but it is nice if you would commit code before new release is ready so we'll be able to test it.

@morgiyan
Copy link
Member

morgiyan commented Mar 5, 2021

@aoksenholt ok, will do that soon (the fix is undergoing our internal testing now).

@morgiyan
Copy link
Member

morgiyan commented Mar 15, 2021

@aoksenholt @petteja I uploaded the source code fix for this issue, as well as some other efficiency improvements for merge methods.

The changes only affect one file, OracleOperationBuilderImpl.java. Please feel free to recompile and test. The new release (1.1.8) with these fixes will be out shortly.

Important: under the covers, both mergeOne and mergeOneAndGet methods require an extra round-trip when versioning methods are hash based (i.e. SHA256, MD5). This is because hash based versioning methods require computation of the version on the updated content, and that's not easily doable in the same single round-trip during which the content is updated.

To avoid this extra round-trip, and thus significantly improve performance of the merge methods, please use UUID versioning or another non-hash based SODA versioning method (note also that UUID is the new SODA versioning default method when running against 21c or above database, assuming compatible is set to 20 and above. It's a highly efficient versioning method, and we recommend using it unless there's a very strong reason to use another non-hash based versioning method).

Here's how to create a collection with the default shape but versioning set to UUID instead of SHA256 (which is the default when running against pre 21c database, and hence also 18c, which you're using):

    OracleDocument meta = cl.createMetadataBuilder().versionColumnMethod("UUID").build();
    OracleCollection col = db.admin().createCollection("collectionNameHere", meta);

@aoksenholt
Copy link
Author

Nice!
We'll give it a try when you have published to a public maven repo (#13)

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

No branches or pull requests

3 participants