Skip to content

Feature/handle multiple connection strings and list filters#3187

Open
anthomaxcool wants to merge 3 commits intoAzure:mainfrom
anthomaxcool:feature/handle-multiple-connection-strings-and-list-filters
Open

Feature/handle multiple connection strings and list filters#3187
anthomaxcool wants to merge 3 commits intoAzure:mainfrom
anthomaxcool:feature/handle-multiple-connection-strings-and-list-filters

Conversation

@anthomaxcool
Copy link

Why make this change?

There's currently a bug that makes the use of multiple sources with nosql cosmos database impossible. This closes #2437

Also added list filters based on the HotChocolate documentation.

What is this change?

Fix data-source-files in dab-config.json and allows list filtering.

How was this tested?

  • Unit Tests

Sample Request(s)

{
    planets(filter : {tags: { some: { contains : ""tag""}}})
    {
        items {
            id
            name
        }
    }
}

@anthomaxcool
Copy link
Author

@anthomaxcool please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
@microsoft-github-policy-service agree
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.
@microsoft-github-policy-service agree company="Microsoft"

Contributor License Agreement

@microsoft-github-policy-service agree

Copy link
Contributor

@JerryNixon JerryNixon left a comment

Choose a reason for hiding this comment

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

@anthomaxcool

1. In parser:

  • any: true => ARRAY_ANY -> SQL ARRAY_LENGTH(field) > 0
  • any: false => coerced to IS NULL

That makes “no elements” equivalent to “is null”, but empty array is not null. Expected
semantics for any: false are usually “array is null OR length == 0” (depending on product definition). This currently only checks null. If intended behavior is “not non-empty”, this is a bug.

2. CosmosQueryBuilder builds nested predicate text and then Replace(arrayField, elementAlias).

This is fragile when:

  • field string appears in literals or function args
  • nested structures include same token as substring
  • multiple levels of nesting reuse similar field names

Safer would be AST-aware rendering with scoped symbol binding, not text replacement.

3. Build(PredicateOperation op) returns tokens not used as SQL operators

You added:

  • ARRAY_SOME, ARRAY_NONE, ARRAY_ALL, ARRAY_ANY

But actual SQL emitted for these is custom branches in Build(Predicate?). Returning string constants here may be misleading and could accidentally leak if future code paths call Build(op) directly.

4. GraphQLSchemaCreator now dedups by INamedSyntaxNode.Name.Value across datasource roots.

If two sources define same type name differently, second definition is silently dropped. This may:

  • mask config errors
  • produce surprising schema depending on iteration order

You should likely detect conflict and throw explicit validation error on differing shapes.

5. Silent continue in Query/Mutation builders can hide config/schema mismatch

Skipping object types not in entities avoids crashes, but silently drops fields/types.

Good for robustness, but this should at least log diagnostic warnings; otherwise users may get “missing operations” with no clue why.

6. CreateListFilter adds legacy operators (contains, startsWith, endsWith) for every scalar list type via elementScalarType.

For non-string scalars these operators are semantically odd/unusable unless downstream validates/rejects later.

Could be intentional for backward compatibility, but deserves explicit gating or tests confirming behavior by scalar type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug]: An item with the same key has already been added. Cosmos DB using two config files

3 participants