Skip to content

Conversation

@arleytm
Copy link
Contributor

@arleytm arleytm commented Jun 13, 2023

This PR enables conjunction for specific operators in the CAP GraphQL Adapter, allowing for combining multiple conditions in queries. The proposed configuration has been refined based on team discussions and stakeholders and incorporates valuable input from all members.

Changes Made

Updated the configuration to enable conjunction for the following operators

  • ne (Not Equal)
  • contains

Disabled conjunction for the following operators

  • eq (Equal)
  • gt (Greater Than)
  • ge (Greater Than or Equal)
  • le (Less Than or Equal)
  • lt (Less Than)
  • startswith
  • endswith

Sample query using contains operator as a filter

query MyQuery {
  AdminService {
    Books(filter: {title: {contains: ["Wuthering", "Heights"]}}) {
      nodes {
        title
      }
    }
  }
}

Reasoning

  • Enabling conjunction for ne and contains allows combining multiple inequality and string condition in queries, providing greater flexibility.
  • Disabling conjunction for other operators simplifies the query evaluation process. When multiple values are provided for operators such as eq, gt, ge, le, lt, only the lowest or highest value in the list will be considered, making the other values without effect. By disabling conjunction for these operators, the query can be simplified to a single value without evaluating additional values, enhancing query performance and reducing complexity.

…apter

This PR enables conjunction for specific operators in the CAP GraphQL Adapter, allowing for combining multiple
conditions in queries. The proposed configuration has been refined based on team discussions and stakeholders and
incorporates valuable input from all members. The final decision ensures a balanced approach, considering use cases
and potential conflicts.

Changes Made:
- Updated the configuration in the CAP GraphQL Adapter to enable conjunction for the following operators:
  - `ne` (Not Equal)
  - `contains`

- Disabled conjunction for the following operators:
  - `eq` (Equal)
  - `gt` (Greater Than)
  - `ge` (Greater Than or Equal)
  - `le` (Less Than or Equal)
  - `lt` (Less Than)
  - `startswith`
  - `endswith`

Sample using `contains`:

query MyQuery {
  AdminService {
    Books(filter: {title: {contains: ["Wuthering", "Heights"]}}) {
      nodes {
        title
      }
    }
  }
}

Reasoning:
- Enabling conjunction for `ne` and `contains` allows combining multiple inequality and string condition in queries,
providing greater flexibility.
- Disabling conjunction for other operators simplifies the query evaluation process. When multiple values are provided
for operators such as `eq`, `gt`, `ge`, `le`, `lt`, only the lowest or highest value in the list will be considered,
making the other values without effect. By disabling conjunction for these operators, the query can be simplified to a
single value without evaluating additional values, enhancing query performance and reducing complexity.
@arleytm arleytm self-assigned this Jun 13, 2023
@arleytm arleytm changed the title [FEATURE!] Enable Conjunction for Certain Operators [FEATURE!] Disable Conjunction for Certain Operators Jun 13, 2023
@arleytm arleytm marked this pull request as ready for review June 13, 2023 10:15
@arleytm arleytm requested a review from a team as a code owner June 13, 2023 10:15
@arleytm arleytm requested a review from schwma June 13, 2023 10:15
@arleytm arleytm requested a review from schwma June 13, 2023 10:49
Copy link
Member

@schwma schwma left a comment

Choose a reason for hiding this comment

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

Thanks for the changes, LGTM 👍

Maybe we should still wait for @johannes-vogel approval of the proposal before merging

@arleytm arleytm enabled auto-merge June 14, 2023 08:46
@arleytm arleytm merged commit da008d5 into main Jun 14, 2023
@arleytm arleytm deleted the conjunction-support branch June 14, 2023 08:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants