CartoDB/carto-react

[RFC] Allow queryParameters for SQL queries

eamador opened this issue ยท 6 comments

Target Use Case

Parameter queryParameters is now supported in the SQL & Map APIs, so we should allow this parameter to be received in carto-react in order to have the widgets in sync with the actual features shown in the map.

We developed a PoC to show a typical datepicker use case

Dates_UseCase.mov

Proposal

New prop

queryParameters need to be pass along the following packages / functions:

  • React-api

SQL - executeSQL
Hooks - useCartoLayerProps

  • React-redux

cartoSlice - addSource

Usage example

{
   query: `SELECT * FROM \`heartquakes\` WHERE date >= @start AND date <= @end`,
   queryParameters: [
        {
          name: "start",
          type: "date",
          value: "2020-01-01"
        },
        {
          name: "end",
          type: "date",
          value: "2021-01-01"
        }
      ]
}

I am not sure if i miss something, @padawannn could you please help here in order to define what changes we need to do and where?

We'd also need to clarify how widgets should behave, for instance in the global mode..

Thank you!

LGTM. On redux modifying addSource to include in queryParameters should be enough. For useCartoLayerProps, we have a SourceProps type where this addition fits.

@eamador I think there is another relevant piece here to verify. When dealing with client-side widgets, this addition to sources wouldn't change the type of results, so I'm assuming client-side calculations should work the same.

But when using widgets in global mode, a request is done to //v3/sql/${source.connection}/model/${model}`; See packages/react-api/src/api/model.js and my question would be: don't we need to send also queryParameters to that endpoint? If we don't do it so, I guess that we would send just templated query in source, is that right?

https://cartoteam.slack.com/archives/C03JJHB2GRL/p1658450619238119?thread_ts=1658450533.766699&cid=C03JJHB2GRL

Yes, the requests to model service need to include the queryParameters. The query template is only usable by builder so no service should work with the queryTemple. C4R always should work with the original query, never with the query template.

The histogram and range widgets call to the Stats API that also needs to include the queryParameters.

Remember that in builder we also call the Stats API we must include the queryParameters in these calls

Keeping in mind this comments i have created a draft PR so that you could start a quick review and confirm it's ok.

@VictorVelarde @padawannn

Already implemented and released. Nice work @eamador