Nested Filtering For MsSql Design #957
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Aniruddh25
requested review from
JelteF,
Mathos1432,
aaronburtle,
aaronpowell,
abhishekkumams,
ayush3797,
gledis69,
jarupatj,
junsu0ms,
mbhaskar,
milismsft,
seantleonard,
severussundar and
tarazou9
as code owners
Aniruddh25
commented
Nov 8, 2022
Aniruddh25
commented
Nov 8, 2022
Fixes from proof reading
This was referenced Nov 10, 2022
aaronburtle
reviewed
Nov 11, 2022
aaronburtle
approved these changes
Nov 11, 2022
Contributor
aaronburtle
left a comment
aaronburtle
left a comment
There was a problem hiding this comment.
LGTM! Just had one question about pagination.
Aniruddh25
added a commit
that referenced
this pull request
Nov 17, 2022
…#975) ## Why make this change? - This is a precursor to #825 the design for which is discussed in #957. - It is purely a refactor necessary to write a recursive graphql filter parser for SQL. ## What is this change? - Although many files seem to be changed, main file to focus on is `GQLFilterParser`. - Change the input arguments for `GQLFilterParser`.`Parse` to include a `BaseQueryStructure` so that it can be called recursively for nested filters both for SQL and Cosmos. Currently, used only for `Cosmos` by setting the `DatabaseObject.Name` to `parentSourceName.NestedFilterFieldname` before each recursive call. Eventually resetting to `_containerAlias` once back to the first caller in `CosmosQueryEngine` - With this modification, moved some of the properties from `BaseSqlQueryStructure` to the base class `BaseQueryStructure` which are useful for Cosmos too : + `EntityName` = the entity name from the config, + `SourceAlias` = the alias of the main entity being filtered used in the query that's generated (renamed from `TableAlias`). Is the `containerAlias` on the first instance of the call for Cosmos. + `MetadataProvider` = either the Sql or Cosmos metadata provider (renamed from `SqlMetadataProvider`) + `DatabaseObject` = has the schema name and source name + `AuthorizationResolver` = useful to add DbPolicyPredicates for any of the entities used in the nested filter - Make `GQLFilterParser` a singleton service dependent on `ISqlMetadataProvider` since we will (in a future PR) use the information from the `MetadataProvider` to obtain `DatabaseObject` of the nested entity. - Constructor modifications to trickle the `GQLFilterParser` and `AuthorizationResolver` to the `BaseQueryStructure` ## How was this tested? - Existing tests since this is only a refactor, no change in functionality.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Why make this change?
What is this change?
How was this tested?
To Do
Investigate similar queries for PgSql, MySql