Skip to content
This repository has been archived by the owner on Oct 18, 2023. It is now read-only.

Add support for queries over HTTP #12

Merged
merged 7 commits into from
Jan 10, 2023
Merged

Add support for queries over HTTP #12

merged 7 commits into from
Jan 10, 2023

Conversation

MarinPostma
Copy link
Collaborator

@MarinPostma MarinPostma commented Jan 9, 2023

This PR adds support for queries over HTTP.

It is enabled by runnign the server with the --http-listen-addr <SOCKET_ADDR> option.

The server is responding to POST / with a payload json with the following shape:

{
        "statements": ["some SQL statement", "some other statement", ...]
    }

if the query should return rows, they are returned to the JSON format.

The BLOB rows are encoded in base64 with the standard alphabet and no padding.

unlike stateful connections that open a DB connection for every session, the HTTP approach uses a connection pool instead. The number of connections in the pool can scale up and down depending on load

I also added end variables for all cli args, to simplify configuration from docker-compose. Env vars are documented in the --help command for the binary.

@petehunt
Copy link

petehunt commented Jan 9, 2023

What does the response look like?

Additionally, I think it would be a good idea if I could provide placeholders in the SQL query. I'm thinking something like this in the request body:

{
  "query": "select email from users where user_id=:userId",
  "variables": {
    "userId": "1234"
  }
}

That way you wouldn't have to implement SQL escaping logic in your client if you don't want to.

Also - how would transactions work with this approach?

@MarinPostma
Copy link
Collaborator Author

Hey @petehunt !

The API is still very crude, the response is just an array of JSON objects representing the rows.

Taking inspiration from PlanetScale serverless driver, I expect parameter binding and transaction management to be handled at the driver level. This makes sense since the HTTP API is stateless, and interactive transactions are disallowed

@penberg
Copy link
Contributor

penberg commented Jan 9, 2023

@petehunt To add what @MarinPostma, the array of statements are run in a transaction similar to https://github.com/planetscale/database-js#transactions

@penberg
Copy link
Contributor

penberg commented Jan 9, 2023

@petehunt here's an example JSON output:

[penberg@turing ~]$ curl -d '{"statements": ["SELECT * FROM users"]}' localhost:8080
[{"email":"penberg@iki.fi"}] 

@penberg penberg mentioned this pull request Jan 9, 2023
@petehunt
Copy link

petehunt commented Jan 9, 2023

Sweet! I think that is a great compromise for transactions.

I still think parameter binding would be pretty useful though. If you had that your DB driver implementation would be trivial and much easier to bring to every platform under the sun. I also like curl-ability for adhoc queries :)

@penberg
Copy link
Contributor

penberg commented Jan 9, 2023

@MarinPostma I think Pete's binding idea makes sense. We can work on that as a follow-up to this, of course.

@MarinPostma
Copy link
Collaborator Author

I'm not against it, we need to think a bit about the design

@haaawk
Copy link
Collaborator

haaawk commented Jan 10, 2023

I support the binding idea. Mostly because I think we should consider adding support for prepared statements and then binding parameters will be necessary.

server/src/http/mod.rs Outdated Show resolved Hide resolved
server/src/http/mod.rs Outdated Show resolved Hide resolved
@haaawk
Copy link
Collaborator

haaawk commented Jan 10, 2023

LGTM

@MarinPostma MarinPostma marked this pull request as ready for review January 10, 2023 12:55
@MarinPostma MarinPostma requested review from haaawk and penberg and removed request for haaawk January 10, 2023 12:55
server/src/lib.rs Outdated Show resolved Hide resolved
@MarinPostma MarinPostma merged commit 8653df9 into main Jan 10, 2023
@penberg penberg deleted the http-proto branch January 10, 2023 15:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants