[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?
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.
Already implemented and released. Nice work @eamador