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

SearchKit - Allow functions in the WHERE clause #22241

Merged
merged 4 commits into from
Dec 31, 2021

Conversation

colemanw
Copy link
Member

Overview

Advanced feature for SearchKit allows SQL functions in the WHERE clause. They are selected in the same way as the "Field Transformations" tab.

image

@civibot
Copy link

civibot bot commented Dec 13, 2021

(Standard links)

@civibot civibot bot added the master label Dec 13, 2021
Copy link
Contributor

@pfigel pfigel left a comment

Choose a reason for hiding this comment

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

This looks great, thanks @colemanw! I've done a quick round of testing and discovered that the generated API4 request appears to be missing the isExpression bool in where clauses with functions. The generated request for Year Only Birth Date = 1991 looks like this:

{
  "version": 4,
  "select": [
    "id",
    "display_name"
  ],
  "orderBy": [],
  "where": [
    [
      "YEAR(birth_date)",
      "=",
      1991
    ]
  ],
  "groupBy": [],
  "join": [],
  "having": []
}

It should look like this:

{
  "version": 4,
  "select": [
    "id",
    "display_name"
  ],
  "orderBy": [],
  "where": [
    [
      "YEAR(birth_date)",
      "=",
      1991,
      true
    ]
  ],
  "groupBy": [],
  "join": [],
  "having": []
}

@colemanw
Copy link
Member Author

@pfigel are you sure about that? My understanding is that the 4th boolean argument controls how the 3rd argument is interpreted, not the first. I think the first argument is always parsed as an expression, because a field name is expected.
This PR is limited to the 1st argument of a where clause. The 3rd is still assumed to be a scalar value.

@pfigel
Copy link
Contributor

pfigel commented Dec 22, 2021

@colemanw I'm fairly certain, yes. where without the bool hits this if, which expects a field, whereas the bool variant goes here and accepts expressions. The request without isExpression=true fails with Invalid field 'YEAR(birth_date)'.

@colemanw
Copy link
Member Author

Thanks @pfigel - you're right, but IMO that was too restrictive. The first arg of the WHERE clause is always interpreted as an expression, so functions ought to be allowed regardless of the 4th param.

I've loosened the restriction and added a unit test to ensure it works correctly in both modes.

@eileenmcnaughton
Copy link
Contributor

Not sure my searches are super useful but they worked....

image

image

@eileenmcnaughton eileenmcnaughton merged commit 72296b0 into civicrm:master Dec 31, 2021
@eileenmcnaughton eileenmcnaughton deleted the searchKitFunctions branch December 31, 2021 03:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants