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

Generate and add _id field in Doc if not passed #32

Merged
merged 8 commits into from
Jan 17, 2023

Conversation

tatu-at-datastax
Copy link
Contributor

Fixes: #28, _id will be generated (using UUID.random()) if needed, will be included in docJson regardless of source.

@tatu-at-datastax tatu-at-datastax requested a review from a team as a code owner January 13, 2023 22:36
@tatu-at-datastax tatu-at-datastax self-assigned this Jan 13, 2023
String id = null;
// We will extract id if there is one; stored separately, but also included in JSON document
// before storing in persistence. Need to make copy to avoid modifying input doc
ObjectNode docWithoutId = ((ObjectNode) doc).objectNode().setAll((ObjectNode) doc);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we need to remove id and add it back again to new document? Can just do docOb.get("_id") and later do docOb.put("_id", id)? So don't need to create new ObjectNode (docWithoutId and docWithId)

Copy link
Contributor Author

@tatu-at-datastax tatu-at-datastax Jan 13, 2023

Choose a reason for hiding this comment

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

Ah. Forgot to add a note on that: while not mandatory, was thinking that we want to order things so that _id is always the first property. Just a preference, but I think this is what happens with Mongo.

But there is also the additional complexity in traversal: code later on assumes that _id is not passed so there won't be index fields for it. So if it was not removed, more checks would be needed not to expose _id via callbacks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added note: underlying ObjectNode uses LinkedHashMap to preserve ordering. While this is bit of an implementation detail it is very unlikely to change for Jackson (esp. 2.x) since changing that would be major compatibility change in practice.

* <p>This will be based on the ideas in the python lab, and extended to do things like make better
* decisions about when to use a hash and when to use the actual value. i.e. a hash of "a" is a lot
* longer than "a".
* <p>Implementation is based on the ideas in the python lab, and extended to do things like make
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the production code make reference to the python lab?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably not. I think I will change it to more generic language but retaining otherwise as-is, for now.

@tatu-at-datastax tatu-at-datastax merged commit 10b0fc1 into main Jan 17, 2023
@tatu-at-datastax tatu-at-datastax deleted the tatu/28-add-id-field-to-doc branch January 17, 2023 23:33
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.

Add _id field to the document
3 participants