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

[FEATURE REQ] Change to how case sensitivity is added to functions in queries #29543

Closed
2 tasks done
azizabah opened this issue Jun 17, 2022 · 3 comments · Fixed by #29597
Closed
2 tasks done

[FEATURE REQ] Change to how case sensitivity is added to functions in queries #29543

azizabah opened this issue Jun 17, 2022 · 3 comments · Fixed by #29597
Assignees
Labels
azure-spring-cosmos Spring cosmos related issues. Client This issue points to a problem in the data-plane of the library. Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that

Comments

@azizabah
Copy link

Is your feature request related to a problem? Please describe.
There is a performance issue when using Case Insensitive querying with CosmosDB and functions. Currently the code is wrapping all of those calls with UPPER() around the parameters. UPPER() forces a table scan without indexes per https://docs.microsoft.com/en-us/azure/cosmos-db/sql/sql-query-upper. This is easy to see the performance implications with some simple querying:

--13.87 RUs
SELECT * FROM c where c.boatId = "TEST3"

--3,268.83 RUs
SELECT * FROM c where STARTSWITH(UPPER(c.boatId), UPPER("TEST3"))

--14.37 RUs
SELECT * FROM c where STARTSWITH(c.boatId, "TEST3", true)

It becomes very clear when comparing this scenario that there is room for improvement. Those numbers are from actual queries in an actual database.

Describe the solution you'd like
The example above alludes to my suggestion for a solution also. This method -

private String getFunctionCondition(final Part.IgnoreCaseType ignoreCase, final String sqlKeyword,
currently handles the UPPER() logic. If it were adapted more along these lines:

 private fun getFunctionCondition(
        ignoreCase: IgnoreCaseType, sqlKeyword: String,
        subject: String, parameter: String
    ): String {
        return if (IgnoreCaseType.NEVER == ignoreCase) {
            String.format("%s(r.%s, @%s)", sqlKeyword, subject, parameter)
        } else {
            String.format("%s(r.%s, @%s, true)", sqlKeyword, subject, parameter)
        }
    }

Then it would benefit from the index scan performance improvements.

Additional context
That call relies on a check of CriteriaType.isFunction(CriteriaType type). Currently that check would also return true for IS_NULL and IS_NOT_NULL. Those functions do not support the third parameter per https://docs.microsoft.com/en-us/azure/cosmos-db/sql/sql-query-is-null and would need some additional logic to prevent accidental usage. Perhaps a check of some kind around CriteraType.isUnary(CriteriaType type) returning false or something along those lines.

Information Checklist
Kindly make sure that you have added all the following information above and checkoff the required fields otherwise we will treat the issuer as an incomplete report

  • Description Added
  • Expected solution specified
@ghost ghost added needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. customer-reported Issues that are reported by GitHub users external to the Azure organization. question The issue doesn't require a change to the product in order to be resolved. Most issues start as that labels Jun 17, 2022
@azizabah
Copy link
Author

@kushagraThapar - If you've got someone looking at the QueryGenerator code, here's an easy, big win.

@kushagraThapar
Copy link
Member

Thank you @azizabah - this is great, appreciate the insights. We will investigate and incorporate this.

@azizabah azizabah changed the title [FEATURE REQ] [FEATURE REQ] Change to how case sensitivity is added to functions in queries Jun 17, 2022
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jun 20, 2022
@ghost
Copy link

ghost commented Jun 20, 2022

Thanks for the feedback! We are routing this to the appropriate team for follow-up. cc @kushagraThapar, @simplynaveen20, @abinav2307

@kushagraThapar kushagraThapar added Client This issue points to a problem in the data-plane of the library. needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. labels Jun 20, 2022
@ghost ghost removed the needs-triage Workflow: This is a new issue that needs to be triaged to the appropriate team. label Jun 20, 2022
@trande4884 trande4884 linked a pull request Jun 22, 2022 that will close this issue
6 tasks
@xinlian12 xinlian12 added the azure-spring-cosmos Spring cosmos related issues. label Jun 23, 2022
@ghost ghost added the needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team label Jun 23, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Apr 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
azure-spring-cosmos Spring cosmos related issues. Client This issue points to a problem in the data-plane of the library. Cosmos customer-reported Issues that are reported by GitHub users external to the Azure organization. needs-team-attention Workflow: This issue needs attention from Azure service team or SDK team question The issue doesn't require a change to the product in order to be resolved. Most issues start as that
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants