influxdb icon indicating copy to clipboard operation
influxdb copied to clipboard

InfluxDB 2.0: /query API should accept Query Parameters

Open rickspencer3 opened this issue 6 years ago • 16 comments

Proposal: When calling: /api/v2/query in addition to the query, I would like to be able to supply a list of variable values in the post body

Current behavior: You can call this by supplying a query string, where the query is a Flux query. However, to set variable, you have to replace strings in your code. For example:

  |> range(start: -{$userInput}
  |> filter(fn: (r) => r._measurement == "light")
  |> filter(fn: (r) => r._field == "light")

This code takes user input, replaces "{{time}}," then stuffs it into the query payload:

 {
  "query":"my replaced string",
  "type":"flux"
 }

This has many problems:

  1. It is hard to write clean code in this manner. This is similar to the bad old days of dynamically creating SQL strings.
  2. This makes my application vulnerable to an injection attack, so I have to take extra care.

Desired behavior: Allow me to define a set of variables and pass those in separately:

query = "  |> range(start: v.timeRangeStart)
  |> filter(fn: (r) => r._measurement == "light")
  |> filter(fn: (r) => r._field == "light")
"

Then I define my payload in the following manner:

 {
  "query":"$query",
  "type":"flux",
  "variables":{"timeRangeStart":"$userInput"}
 }

In this code:

  1. I can copy cope directly from the query designer and it works.
  2. My code is much cleaner
  3. Critically, I can rely on the flux engine to do type checking on the userInput, and, in this way, easily thwart injection attacks.

Alternatives considered: Alternatives include sanitizing user input.

Use case: This makes it much easier to use InfluxDB 2.0 as a backend in almost any language.

rickspencer3 avatar Dec 04 '19 13:12 rickspencer3

The query API currently allows users to specify a parameter called extern that does something similar to this. That is how the UI currently supplies external variables. We do not do any string replacement. The trouble is that it's pretty painful to use as it requires the user to specify the payload as a flux file format, which is flux AST.

https://github.com/influxdata/influxdb/blob/master/http/swagger.yml#L6243 https://github.com/influxdata/influxdb/blob/master/http/swagger.yml#L6283

{
  "query": "<query>",
  "extern": {
    "type": "File",
    "package": null,
    "imports": null,
    "body": [
      {
        "type": "OptionStatement",
        "assignment": {
          "type": "VariableAssignment",
          "id": {
            "type": "Identifier",
            "name": "v"
          },
          "init": {
            "type": "ObjectExpression",
            "properties": [
              {
                "type": "Property",
                "key": {
                  "type": "Identifier",
                  "name": "timeRangeStart"
                },
                "value": {
                  "type": "UnaryExpression",
                  "operator": "-",
                  "argument": {
                    "type": "DurationLiteral",
                    "values": [
                      {
                        "magnitude": 1,
                        "unit": "h"
                      }
                    ]
                  }
                }
              },
              {
                "type": "Property",
                "key": {
                  "type": "Identifier",
                  "name": "timeRangeStop"
                },
                "value": {
                  "type": "CallExpression",
                  "callee": {
                    "type": "Identifier",
                    "name": "now"
                  }
                }
              }
            ]
          }
        }
      }
    ]
  },
  "dialect": {<dialect specifications>}
}

Working with extern has been a giant pain. I'd prefer to see something like what you have above, or potentially just flux like

option v = {timeRangeStart: -15m, timeRangeStop: now()}

desa avatar Dec 04 '19 15:12 desa

It's good to know that it is technically supported, but this does not feel like a design that was made for me as a developer, as you say.

Note that I am totally fine pursuing other designs like the one you propose, I just suggested what made sense to me, but I understand that there are probably people who know more about he domain and would make a more sensible design.

rickspencer3 avatar Dec 04 '19 15:12 rickspencer3

I actually think I prefer having something like

 {
  "query":"$query",
  "type":"flux",
  "variables":{"timeRangeStart":"$userInput"}
 }

The only thing I think that might be hard is how to deal with all of the different native types that flux supports. Specifically how would we tell the difference between a string and a duration/time literal.

desa avatar Dec 04 '19 16:12 desa

@rickspencer3 @desa Do we want to prioritize this work, and who would own it?

imogenkinsman avatar Mar 05 '20 21:03 imogenkinsman

I'd imagine it'd be the compute team. I would definitely like to see this get in.

desa avatar Mar 05 '20 21:03 desa

+1.

I have tried to use Python APIs to pass timeRangeStart/timeRangeStop/periodWindow to support queries from dashboard more easily and serializing the extern "file" is very difficult to do.

As an alternative, could I just pass some Flux code that generates specific values? That could avoid issues with handling of data types and passing like "-24h".

wojciechka avatar Mar 07 '20 13:03 wojciechka

i think the "extern" property gives a lot of flexibility and isnt too hard to facilitate -> https://github.com/influxdata/influxdb-client-js/pull/178

but then the flux AST would be part of the API semver promise, what could be undesireable.

a parameters option should support all flux types:

{
  "query": "...",
  "parameters": {
    "fooString": "foo string", // same as { "type": "string", "value": "foo string" }
    "fooBoolean": true, // same as { "type": "boolean", "value": "true" }
    "fooNull": null, // same as { "type": "null" }
    "fooInteger": 42, // same as { "type": "int", "value": "42" }
    "fooDouble": 4.2, // same as { "type": "float", "value": "4.2" }
    "fooArray": [1, 2, 3], // same as { "type": "array", "elements": [1, 2, 3] }
    "fooUnsignedInteger": { "type": "uint", "value": "123" },
    "foo???": { "type": "expression", "source": "4 + 6" }, // a flux expression
    "fooArray": [{ "type": "uint", "value": "42"}],
    "fooObject": {
      "type": "object",
      "properties": {
        "bar": 42, // an integer
        "baz": { "type": "uint", "value": "42 }
      }
    }
  }
}

backbone87 avatar Apr 20 '20 18:04 backbone87

There is now influxdata/influxdb-client-js#178 PR that is using the existing extern property to supply query parameters. It would be definitely better to avoid AST dependencies (extern) in influxdb v2 clients, but if there is no better way planned, we could proceed with extern. The clients can provide the necessary isolation layer so that flux AST is not required to know.

Independently on whether we should rely upon existing extern or wait for a future new API, query parameters should be easily recognized in a flux query. Influxd UI wraps all query parameters in a variable named v. Is v an established convention to follow or shall we introduce a better/self-describing name (such as params)?

@rickspencer3 @desa Can you please share your point of view?

sranka avatar Apr 23 '20 05:04 sranka

Obviously since I logged this issue, I would like to see this feature implemented. Currently, the Flux team is 100% focused on Flux performance. We'll take a look at simplifying this part of the API after we get through the first batch of that effort.

rickspencer3 avatar Apr 29 '20 16:04 rickspencer3

Hi @rickspencer3, I am also trying to integrate my Python script with InfluxDB 2.0 through REST API, I am trying to make a post-call bypassing flux query into the body but not getting how to achieve this. I have done basic post call through postman and it worked but with python script facing difficulties. Would you guide me here what I need to take, Also I want to make like the value which I have set in flux query for field should be dynamic?

ssaurav-1987 avatar May 18 '20 13:05 ssaurav-1987

This conversation resurfaced today in the context of Templates. Without this functionality in Tasks, we cannot have Tasks be part of Templates....which would seem to me like an important shortcoming of the Templates feature.

Any resource a template author puts into a template that requires custom input from the consumer of their template (i.e., bucket names, input urls, output urls, etc.) requires parameterization. For resources like Telegraf configs, these custom values can be injected via environment variables. For variables, other variables can depend on and call other variables.

Tasks require a bucket to read from and often a different bucket to which to write. Since Tasks don't support parameterization today, they can't be used in Templates unless the template prescribes the buckets beforehand. That goes against the Template best practices for a number of reasons so I think that option may be off the table.

I like the idea @desa proposed above. For templating Tasks, this might look like:

option task {
    fromBucket: $INFLUX_FROM_BUCKET,
    toBucket: $INFLUX_TO_BUCKET,
}

samhld avatar Jun 23 '20 22:06 samhld

@rickspencer3 with the arrival of API Invokable Scripts.... where does that leave us wrt to this issue?

The extern approach has been wrapped by the JS, C#, and Python libraries to make it easier to develop against. Shall we continue to expand support for that approach with the remaining libraries? or should we be guiding folks to use /scripts?

timhallinflux avatar Nov 17 '21 16:11 timhallinflux

According the swagger (https://github.com/influxdata/openapi/blob/f3057edb916dbae0a42690a5b06139843cc16e68/contracts/oss.yml#L7058) both Cloud2 and OSS should support parameterized queries. All client libraries already contain generated Query model with optional map of parameters. But it is really supported only in Cloud2 as described in https://docs.influxdata.com/influxdb/cloud/query-data/parameterized-queries/.

OSS now returns "undefined identifier params" error, we should fix this in OSS.

rhajek avatar Dec 01 '21 14:12 rhajek

Is there any update on this?

deefdragon avatar May 30 '22 09:05 deefdragon

any updates?

paulwer avatar Jun 21 '23 16:06 paulwer

+1, any updates? It's been ~2 years since this was implemented in the cloud.

ylhan avatar Dec 14 '23 17:12 ylhan