Support long prepared statements #5868
Conversation
|
@dain, you weighed in on the issue I filed a bit ago, can you take a look when you get a chance? |
ghost
added
the
|
Can you explain the protocol?
|
|
Sure. There's a new header "X-Presto-Prepared-Statement-In-Body". If that key is specified -- with any value, though StatementClient uses the string "true" -- the client creates a QuerySubmission object which contains the query and the map of prepared statements (that used to be sent in the headers), and passes a json-serialized version of that object to the server. The server parses the object via Jackson, and everything else proceeds as before. QueryResults also has to change, to include added prepared statements and deallocated prepared statements that also used to be sent via HTTP headers. If the new header is not specified, everything works as before (I've tested with versions of the CLI without this patch). |
cawallin
force-pushed
the
cw/long-prepared-statements/3
branch
from
678d815
to
0b64dd2
Compare
|
Travis actually passed, there's something wrong with the cache calcluation or something that made it report a failure at the end. |
cawallin
force-pushed
the
cw/long-prepared-statements/3
branch
from
0b64dd2
to
a0fe85f
Compare
cawallin
force-pushed
the
cw/long-prepared-statements/3
branch
from
a0fe85f
to
a44a057
Compare
cawallin
force-pushed
the
cw/long-prepared-statements/3
branch
from
a44a057
to
c792de6
Compare
cawallin
force-pushed
the
cw/long-prepared-statements/3
branch
from
c792de6
to
178c818
Compare
cawallin
force-pushed
the
cw/long-prepared-statements/3
branch
from
178c818
to
831b3f1
Compare
|
Rebased, and tests pass |
HTTP request headers have a 64k limit, by default 4k in Jetty. Prepared statements can be much longer than that, so if the header "X-Presto-Prepared-Statement-In-Body" is specified, the server assumes that the query has been sent as a Jackson-serialized QuerySubmission and parses the request appropriately. In order to preserve backwards compatibility for clients, it is still possible to send a raw query string if the above header is not sent. Only /v1/statement supports sending the query via a QuerySubmission object, since /v1/query and /v1/execute don't support prepared statements.
In order to support >4k prepared statements, send the prepared statements along with the query via the Jackson-serialized object QuerySubmission.
cawallin
force-pushed
the
cw/long-prepared-statements/3
branch
from
831b3f1
to
8b828c2
Compare
|
Rebased. |
|
Sorry for the long delay on this. @dain and I came up with a strawman proposal for the protocol changes that @martint will review: https://gist.github.com/electrum/a382d3f8e10fde5e2e0c1bfc1ea3182f |
|
Thanks for taking a look! Switching outright to v2 of the protocol makes a lot of sense (and will also help with allowing commas in session properties, etc). I'll close this PR, then, and look out for the new protocol changes. Let me know if you want me to write or review that change. |
Fixes #5759 by passing prepared statements in the HTTP request body rather than the headers, which have a limit in Jetty of 4k bytes.
Whether prepared statements are passed in the headers or the body is configurable (via an HTTP header), to allow other clients a chance to update.