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.

Sign up for GitHub

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

Jump to bottom

[Bugfix] [SQL-Search] Fix SQL code generation when :not operator is included. #2785

Merged
merged 12 commits into from
Sep 16, 2022
Merged

[Bugfix] [SQL-Search] Fix SQL code generation when :not operator is included. #2785

merged 12 commits into from
Sep 16, 2022

Conversation

fhibf
Copy link
Contributor

@fhibf fhibf commented Aug 31, 2022 •
edited
Loading

Description

Queries with the :not operator were returning more results than the expected.
When reviewing the existing logic, it was noticed that queries with the :not operator were not taking in consideration previous CTE than the one with the :not clause.

After changing the SearchParamTableExpressionKind to Normal (instead of All), the CTE usage was fixed.

Related issues

Addresses [issue #].

Testing

  • Unit Tests validation.
  • E2E Tests validation.
  • Ad-hoc test validation.

FHIR Team Checklist

  • Update the title of the PR to be succinct and less than 50 characters
  • Add a milestone to the PR for the sprint that it is merged (i.e. add S47)
  • Tag the PR with the type of update: Bug, Dependencies, Enhancement, or New-Feature
  • Tag the PR with Azure API for FHIR if this will release to the Azure API for FHIR managed service (CosmosDB or common code related to service)
  • Tag the PR with Azure Healthcare APIs if this will release to the Azure Healthcare APIs managed service (Sql server or common code related to service)
  • CI is green before merge
  • Review squash-merge requirements

Semver Change (docs)

Patch|Skip|Feature|Breaking (reason)

@fhibf fhibf added Bug Bug bug bug. Area-Search Area related to search. Area-SQL Area related to the SQL Server data provider Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs labels Aug 31, 2022
@fhibf fhibf added this to the S96 milestone Aug 31, 2022
@fhibf fhibf requested a review from a team as a code owner August 31, 2022 01:09
@fhibf fhibf merged commit 4ac11f4 into main Sep 16, 2022
@fhibf fhibf deleted the personal/fhibf/fixNotOperatorUsage branch September 16, 2022 20:34
ccfromms pushed a commit that referenced this pull request Sep 19, 2022
@fhibf @ccfromms
[Bugfix] [SQL-Search] Fix SQL code generation when :not operator is i…
80f34f4 
…ncluded. (#2785)

* Changes in NotExpressionRewriter.
* Adding categories to tests.
* Adding tests to fix bug when using :not operator combined with other search criteria.
ccfromms added a commit that referenced this pull request Oct 21, 2022
@ccfromms @PTaladay
@LTA-Thinking @dependabot @brendankowitz @fhibf
rebuild index (#2759)
b37dba3 
* rebuild index

* Modify the length of Ind and the Generator setting

* Modify 39.diff.sql

* update

* update sql

* modify 40.diff.sql

* modify the code for table IndexProperties

* Modfiy and add tests

* restore the global.json

* Modify E2E test for rebuild indexes

* Remove rebuild index progress

* Delete lists not used

* Delete unused files

* Remove useless assignment and change token

* fix error in test

* Remove delay

* Remove useless

* Combine flag in importtaskconfiguration

* Change DefaultSqlLongRunningOperationTimeoutInSec to 0

* Additional fix for Failed Export due to RequestEntityToolarge (#2803)

* Added actionable error message for customer when hitting a RequestEntityTooLargeException during export.

* Added in missing logging.

* Now clearing the output for the export job if it runs into the exceeded size so we can save the failed state. The output information is really necessary if it failed since it would be incomplete.

* Fix long job info crash (#2802)

* Bump System.Configuration.ConfigurationManager from 6.0.0 to 6.0.1 (#2801)

Bumps [System.Configuration.ConfigurationManager](https://github.com/dotnet/runtime) from 6.0.0 to 6.0.1.
- [Release notes](https://github.com/dotnet/runtime/releases)
- [Commits](dotnet/runtime@v6.0.0...v6.0.1)

---
updated-dependencies:
- dependency-name: System.Configuration.ConfigurationManager
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Standalone resource parser (#2751)

* Adds SDL analysis tools, cleaup PR builds (#2797)

+semver: skip

* [Bugfix] [SQL-Search] Fix SQL code generation when :not operator is included. (#2785)

* Changes in NotExpressionRewriter.
* Adding categories to tests.
* Adding tests to fix bug when using :not operator combined with other search criteria.

* Bump FluentValidation from 11.2.1 to 11.2.2 (#2800)

Bumps [FluentValidation](https://github.com/JeremySkinner/fluentvalidation) from 11.2.1 to 11.2.2.
- [Release notes](https://github.com/JeremySkinner/fluentvalidation/releases)
- [Changelog](https://github.com/FluentValidation/FluentValidation/blob/main/Changelog.txt)
- [Commits](FluentValidation/FluentValidation@11.2.1...11.2.2)

---
updated-dependencies:
- dependency-name: FluentValidation
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Package integration tests (#2806)

* restore 40

* add 41.diff

* change taskconfiguration

* remove incorrect set

* modify stored procedures

* fix stored procedures

* Add IndexRebuilder

* add sqlserver.database

* update sqlserver.database.sqlproj

* update suppress warning

* add log

* remove unnecessary reference

* modify sql

* modify encoding

* update BuildTimeCodeGenerator and Tasks version to 6.1.129

* update

* add attribute for test

* remove set and restore lines

* modify nuget version

* restore encoding

* restore encoding

* remove lines

* modify pros

* modify props

* modify props

* modify

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Paul Taladay <PTaladay@users.noreply.github.com>
Co-authored-by: Robert Johnson <rbrucej@gmail.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Brendan Kowitz <bkowitz@microsoft.com>
Co-authored-by: Fernando Henrique Inocêncio Borba Ferreira <fernfe@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@PTaladay PTaladay PTaladay left review comments

@LTA-Thinking LTA-Thinking LTA-Thinking approved these changes

Assignees
No one assigned
Labels
Area-Search Area related to search. Area-SQL Area related to the SQL Server data provider Azure Healthcare APIs Label denotes that the issue or PR is relevant to the FHIR service in the Azure Healthcare APIs Bug Bug bug bug.
Projects
None yet
Milestone
S96
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@fhibf @PTaladay @LTA-Thinking