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

Fix #240: surround String-valued DocumentId in single quotes #241

Merged
merged 7 commits into from
Mar 9, 2023

Conversation

tatu-at-datastax
Copy link
Contributor

@tatu-at-datastax tatu-at-datastax commented Mar 9, 2023

What this PR does:

Enclose String-valued DocumentId in single quotes, for use in Exception messages -- avoids confusion wrt id type, contents.

Also adds a separate method to use for constructing DB primary keys, instead of Document.toString().

Which issue(s) this PR fixes:
Fixes #240

Checklist

  • Changes manually tested
  • Automated Tests added/updated
  • Documentation added/updated
  • CLA Signed: DataStax CLA

@tatu-at-datastax tatu-at-datastax self-assigned this Mar 9, 2023
@tatu-at-datastax tatu-at-datastax changed the title (WIP) Fix #240: surroung String-valued DocumentId in single quotes Fix #240: surround String-valued DocumentId in single quotes Mar 9, 2023
@tatu-at-datastax tatu-at-datastax marked this pull request as ready for review March 9, 2023 20:30
@tatu-at-datastax tatu-at-datastax requested a review from a team as a code owner March 9, 2023 20:30
@@ -64,7 +64,7 @@ public static Map<QueryOuterClass.Value, QueryOuterClass.Value> getDoubleMapValu
public static List<QueryOuterClass.Value> getDocumentIdValue(DocumentId documentId) {
// Temporary implementation until we convert it to Tuple in DB
List<QueryOuterClass.Value> tupleValues =
List.of(Values.of(documentId.typeId()), Values.of(documentId.toString()));
List.of(Values.of(documentId.typeId()), Values.of(documentId.asDBKey()));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Necessary to avoid single-quotes from being added wrt DocumentId.toString() change: also semantically good to have separate method for use case.

Copy link
Contributor

@maheshrajamani maheshrajamani left a comment

Choose a reason for hiding this comment

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

LGTM

@tatu-at-datastax tatu-at-datastax merged commit 586d6eb into main Mar 9, 2023
@tatu-at-datastax tatu-at-datastax deleted the tatu/240-doc-id-in-quotes branch March 9, 2023 21:23
@vkarpov15
Copy link
Collaborator

LGTM

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.

Enclose String-backed DocumentId values in quotes (for Exception messages)
3 participants