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

(POC, WIP) Allow querying APIv4 with SQL expressions #17054

Closed
wants to merge 2 commits into from

Conversation

totten
Copy link
Member

@totten totten commented Apr 10, 2020

Preface

As I was reading some of the PRs for dev/report#31 like #16947 and #17047, I was struck by the sense that they were, essentially, expanding APIv4's PHP API to incorporate a small DSL within the select (e.g. $params['select'][]='avg(fee_amount) as average_fee';)

This struck me for a couple reasons. On one hand, it's bit confusing because my impression is that APIv4's PHP API is supposed to be a straight-up 'query-builder' pattern using methods and arrays which are amenable to data-integration and complex UIs, so the DSL feels out-of-character. On the other hand, I can relate because I found it easier to use APIv4 through a DSL (ahem cv api4 and dev/core#1489).

To be sure, I think that it's useful to have both a DSL-pattern (for simple improvisations) and a query-builder-pattern (for data-integrations and complex UI building). Ideally, you would be able to summon either pattern when convenient, and you'd be able to interchange them.

But there's a problem with the current design -- the select DSL and for cv api4 DSL don't currently work together.

If only there were some standard DSL for writing queries, that could be interpreted consistently in different layers. Maybe something which already had existing parsers. Something that would take the name of the data-set and a list of return-values and a list of filters...

Overview

This is a proof-of-concept for \Civi\Api4::sql($expr) which allows you to construct an APIv4 call using a SQL expression.

Consider a query written in APIv4's query-building notation:

\Civi\Api4\Contact::get()
  ->addSelect('id', 'display_name', 'phones.phone')
  ->addWhere('id', '>', 10)
  ->execute();

Of course, in a DSL like SQL, you don't need as many funny symbols. It's just one line:

SELECT id, display_name, phones.phone FROM Contact WHERE id > 10

Before

You have to use query-builder notation all the time.

After

You can construct the APIv4 query like so:

\Civi\Api4::sql('SELECT id, display_name, phones.phone FROM Contact WHERE id > 10')
  ->execute();

For good and ill, this will execute APIv4 -- applying the same permission model, custom-data mechanics, option-value-mappings, BAO fiddly-bits, etc.

You can also mingle the styles -- seeding a base-query with the SQL and then mixing-in extra bits with the query-building interface:

\Civi\Api4::sql('SELECT id, display_name, phones.phone FROM Contact WHERE id > 10')
  ->setLimit(10)
  ->addOrderBy('display_name')
  ->execute();

Comments

  1. It may be confusing for some people that MySQL-SQL and APIv4-SQL are different dialects. One could take a cue from Doctrine, Hibernate, Salesforce, etal and rebrand it. (API4QL? 4QL? A4? Fourql? CiviQL? ad nauseum)

  2. You can see this in action by checking out the PR and running one of these commands:

## Run a query as demo user (permissioned)
cv ev -U demo  'return Civi\Api4::sql("SELECT display_name, phones.phone FROM Contact ORDER BY id desc LIMIT 5")->execute();'

## Run a query as the system (un-permissioned)
cv ev 'return Civi\Api4::sql("SYS SELECT display_name, phones.phone FROM Contact ORDER BY id desc LIMIT 5")->execute();'
  1. Benchmarking data for some PHP-SQL parsers: https://gist.github.com/totten/409f9f1f6d96a17d83e1d100e2eca3a6

  2. This PR is a "proof-of-concept" for a few reasons. Trivially, it doesn't have a unit-test, and there's a lot of 'FIXME' bits for mapping different parts of the query. Additionally, some of the functionality will be more doable as dev/report#31 progresses in supporting SQL-features in APv4 queries. More broadly, I think there should be a bit more thought to the inter-change of structured/query-building representations and DSL/pithy representations. Maybe things like this would allow you freely move among notations:

    $get = Civi\Api4\Contact::get()
     ->withParams([...query fragment expressed like APIv4 $params...])
     ->withSql('...query fragment expressed like SQL...');
    $result = $get->execute(); // Returns the result-set.
    $paramArray = $get->toParams(); // Returns a list of $params that are equivalent to $get.
    $sqlExpr = $get->toSQL(); // Returns a SQL string that is equivalent to $get.

@civibot
Copy link

civibot bot commented Apr 10, 2020

(Standard links)

@civibot civibot bot added the master label Apr 10, 2020
@colemanw
Copy link
Member

Interesting. Looks like it could be useful.
When I originally imagined a SQL parser as a CiviCRM api, I got bogged down in thinking about validation and all the edge-cases that entails.
This bypasses that problem neatly, it just takes a string and converts it to an api call, allowing the validation to happen in the usual way.

@eileenmcnaughton
Copy link
Contributor

I guess my worry was around permissions & validation - but I guess this manages that

@artfulrobot
Copy link
Contributor

If this catches up and stays aligned to all the queries that Api4 can build, it could be very helpful to discovery, where the explorer runs out of road. eg does api4 support a certain type of query? Well we just write the query and see if it throws an exception! Be interesting to see how/if this syntax supports parameters (inc arrays of parameters for IN() ).

@colemanw
Copy link
Member

In order to support this from js and other bindings (which all go through civicrm_api4()) we could overload that function just a little bit where passing "sql" as the action would permit a query string as the first param, like this:

Current signature: civicrm_api(string $entity, string $action, array $params)

Updated signature: civicrm_api(string $entityOrQuery, string $action, array $params)

Example:

civicrm_api4("SELECT * FROM Contact WHERE id = 5", 'sql', ['checkPermissions' => FALSE, 'limit' => 1]);

This nicely allows array params and sql arguments to be mixed & matched.

@eileenmcnaughton
Copy link
Contributor

@totten @colemanw are we proceeding with this?

@eileenmcnaughton
Copy link
Contributor

@totten @colemanw I'm closing this at this point because it's stale but can you check whether it's a concept we want to proceed with?

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.

4 participants