Modify /query endpoint to work with POST and limit GET

[pauldix]

We should modify the /query endpoint so that everything works with POST requests. We should then limit GET requests so that only SELECT and SHOW queries work there (except for SELECT INTO queries, which should only work through a POST

For the 0.13.0 release we should add the POST functionality and put a deprecation warning in the GET requests for any queries that won't work later.

After 0.13.0 is released we should then add the limitations to GET requests

[jsternberg]

The deprecated or warning message that needs to be sent back is related to #5707 which also needs that functionality. It's likely easiest to do that by adding something to the results object.

[jsternberg]

@pauldix is something like this what you envisioned?

jsternberg:[~/g/s/g/i/influxdb] $ (js-6290-query-post-endpoint *$) influx
Visit https://enterprise.influxdata.com to register for updates, InfluxDB server management, and monitoring.
Connected to http://localhost:8086 version unknown
InfluxDB shell 0.9
> kill query 4
warning: deprecated use of 'KILL QUERY 4' in a read only context, please use a POST request instead.
ERR: no such query id: 4

Of course, we would modify the influx client to use a POST as part of the final PR, but I want to know if I'm heading in the correct direction.

[pauldix]

@jsternberg sure that looks good.

[gunnaraasen]

I like the idea of allowing POST requests, but am strongly against disallowing GET requests for modifying queries, which muddles the relationship between our SQL-like InfluxQL syntax and the HTTP protocol.

InfluxQL syntax combines a DDL and DML for interacting with the database. In the context of interacting with the database, HTTP currently is a dumb transport for sending InfluxQL queries. Tying InfluxQL syntax with HTTP verbs is redundant to InfluxQL's built-in permissions model and adds an unnecessary relationship to the HTTP protocol.

Additionally, disallowing some queries via GET requests would break pretty much every existing InfluxDB client and require a non-trivial implementation to fully support the syntax. To add support, clients would be force to either

• switch to only send queries as POST requests, or
• parse the InfluxQL of every query to ensure a GET is not used for modifying queries.

To me, there are minimal practical benefits to limiting GET requests and significant costs to fully support the change. If anything, InfluxDB should service queries regardless of the verb used in the HTTP request.

[pauldix]

@gunnaraasen I think it's a rather trivial change in most clients to change to send everything through a POST. The main client that might have a problem is javascript, but we own that so we could update it accordingly.

[gunnaraasen]

Changing all clients to use POST is still a significant undertaking. It still feels like there's weak justification for restricting some query statements sent with the GET verb. I've not been able to find an example of any other database with an SQL engine that takes the HTTP verb into consideration when executing a query.

[ekini]

Sorry, what was the reason for making golang client/v2 use POST for queries?

We use caching nginx in front of influxdb to ease its burgen, but unfortunately it doesn't work with POST requests.

[jsternberg]

The intention was to separate queries that modify the database (using POST) and those that just query the database (using GET). Select queries are supposed to use GET, but the current client isn't smart enough to have different functions for those two different queries. While this is something that's easier to fix for someone using the client, I don't know how we would fix it for the influx CLI tool.

[ekini]

Thanks for the explanation.
Although the change still looks unjustified to me, because it doesn't solve any problem, but just adds complication, I've found a way to cache POST requests with nginx :)

proxy_cache_methods POST;
proxy_cache_key "$request_uri|$request_body";

[simonklb]

When can it be expected for queries that modifies the database to be disallowed when using GET requests rather than just a deprecation warning?

I think letting GET be read-only is a great way to create more reliable clients.

[vgt]

Not sure if this related, but I have a similar problem to what is described here grafana/grafana#5987.
As far as I can tell, POST for queries is supported, but the query itself must always be part of the URI. Are there plans to use the body for carrying the query instead? I tried finding a features request / issue for this, but have not found anything.
Any hint is highly appreciated.

Edit: Found out in file handler.go that MultipartForm can be used to post query parameter as well. This solves my question, though it is not mentioned in the documentation as far as I can tell.