Skip to content

QueryCriteria.where() adds backticks to ALL keys, breaks keys that use META() #1066

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

Closed
aaronjwhiteside opened this issue Jan 22, 2021 · 5 comments · Fixed by #1082
Closed
Labels
status: waiting-for-triage An issue we've not yet triaged

Comments

@aaronjwhiteside
Copy link
Contributor

For example:

import static org.springframework.data.couchbase.core.query.N1QLExpression.*;
...
QueryCriteria criteria = QueryCriteria.where(path(meta(escapedBucket(getCouchbaseOperations().getBucketName())), "cas").toString()).eq(version);

produces a query like so:

(`META(`YS_Apps`).cas` = 1611287177404088320)
@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Jan 22, 2021
@mikereiche
Copy link
Collaborator

mikereiche commented Feb 1, 2021

the "key" of a criteria is a String (fieldname) and gets back-ticks around it. Also where() takes a String argument (which in turn becomes the "key" of the criteria so there are couple issues.

Writing the predicate the other way around will give the correct form for the RHS, but the LHS will again be incorrect as it is encapsulated in back-ticks.
QueryCriteria criteria1 = QueryCriteria.where(version.toString()).eq(path(meta(escapedBucket("my_bucket")), "cas"));
`1612203204822368300` = META(`my_bucket`).cas

@aaronjwhiteside
Copy link
Contributor Author

aaronjwhiteside commented Feb 2, 2021

@mikereiche I'm not sure if that helps?

What is needed here is probably a way to take a higher level object other than a String that can be used to represent complex keys that are not subject to the default quotation..

And I imagine that you could probably refactor much of the QueryCriteria logic into that new object, and use it internally when String it passed to where().

Something vaguely like this, for inspiration.

QueryCriteria.where(Condition.field("deleted")).eq(false);
QueryCriteria.where(Condition.meta("bucketname", "cas")).eq(version);

@mikereiche
Copy link
Collaborator

mikereiche commented Feb 2, 2021

What is needed here is probably a way to take a higher level object other than a String

Right. That's why I pointed out the limitation "where() takes a String argument". Allowing the argument to be a N1qlExpression would work (there's no reason that it should be restricted to a string property-name). That's why I pointed out that the argument for eq() is allowed to be a N1qlExpression and does not have the same problem. An even simpler solution would be to not assume the string arg to where() is a property-name and should be escaped.

@aaronjwhiteside
Copy link
Contributor Author

aaronjwhiteside commented Feb 2, 2021

OK, you didn't mention N1QLExpression in your initial response, so I was confused :)

So what are the next steps?

@mikereiche
Copy link
Collaborator

Adding support for a where(N1qlExpression) method. This would require changing the "key" of QueryCriteria from a String to a N1QLExpression.

Unrelated to the where(N1lqExpression) issue - the CAS in n1ql should always be treated as a string, not a number.
https://issues.couchbase.com/browse/MB-24464
https://issues.couchbase.com/browse/MB-44048

mikereiche added a commit that referenced this issue Feb 17, 2021
This is a merge of mmonti's changeset into master.  The changeset was made
on top of 4.1.x instead of master so it has a little catching up to do.

Merge branch 'gh-1066-n1qlexpression-as-querycriteria-key' of github.com:mmonti/spring-data-couchbase into datacouch_1066_1072_mmonti_n1ql_query_creator_expression

Original pull request #1076
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: waiting-for-triage An issue we've not yet triaged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants