On Fri, Nov 30, 2018 at 01:45:48PM +0300, Vladislav Shpilevoy wrote:
On 30/11/2018 13:19, Vladimir Davydov wrote:
On Thu, Nov 29, 2018 at 05:04:06PM +0300, Vladislav Shpilevoy wrote:
On 29/11/2018 13:53, Vladimir Davydov wrote:
On Tue, Nov 27, 2018 at 10:25:43PM +0300, imeevma@xxxxxxxxxxxxx wrote:
@@ -627,81 +610,53 @@ sql_prepare_and_execute(const struct
sql_request *request,
}
int
-sql_response_dump(struct sql_response *response, int *keys, struct
obuf *out)
+sql_response_dump(struct sql_response *response, int *keys,
+ struct mpstream *stream)
{
sqlite3 *db = sql_get();
struct sqlite3_stmt *stmt = (struct sqlite3_stmt *)
response->prep_stmt;
- struct port_tuple *port_tuple = (struct port_tuple *)
&response->port;
int rc = 0, column_count = sqlite3_column_count(stmt);
if (column_count > 0) {
- if (sql_get_description(stmt, out, column_count) != 0) {
+ if (sql_get_description(stmt, stream, column_count) !=
0) {
err:
rc = -1;
goto finish;
}
*keys = 2;
- int size = mp_sizeof_uint(IPROTO_DATA) +
- mp_sizeof_array(port_tuple->size);
- char *pos = (char *) obuf_alloc(out, size);
- if (pos == NULL) {
- diag_set(OutOfMemory, size, "obuf_alloc",
"pos");
- goto err;
- }
- pos = mp_encode_uint(pos, IPROTO_DATA);
- pos = mp_encode_array(pos, port_tuple->size);
- /*
- * Just like SELECT, SQL uses output format compatible
- * with Tarantool 1.6
- */
- if (port_dump_msgpack_16(&response->port, out) < 0) {
+ mpstream_encode_uint(stream, IPROTO_DATA);
+ mpstream_flush(stream);
+ if (port_dump_msgpack(&response->port, stream->ctx) <
0) {
stream->ctx isn't guaranteed to be an obuf
And when you introduce vstream later, you simply move this code to
another file. This is confusing. May be we should pass alloc/reserve
used in mpstream to port_dump instead of obuf?
Good idea, though not sure, if it is worth slowing down port_dump_msgpack
adding a new level of indirection. Since port_dump_msgpack is a hot path
and is used for box.select.
Maybe it is better to just rename port_dump_msgpack to port_dump_obuf
and rename vstream_port_dump to vstream_port_dump_obuf? If we ever will
dump port to not obuf, then we will just add a new method to port_vtab.
Also, it would make port_dump_obuf name consistent with port_dump_lua -
in both cases we not just dump in a specific format, but to a concrete
destination: obuf and lua stack. Now port_dump_msgpack anyway is
restricted
by obuf destination.
There's port_dump_plain, which dumps port contents in a specific format.
So port_dump_obuf would look ambiguous.
If you worry about how to call sql_response_dump() to not obuf, then there
is another option. Anyway rename port_dump_msgpack to port_dump_obuf and
introduce a new method: port_dump_mpstream. It will take mpstream and use
its reserve/alloc/error functions. It allows us to do not slow down
box.select,
but use the full power of virtual functions in execute.c, which
definitely is
not hot.
That would interconnect port and mpstream, make them dependent on each
other. I don't think that would be good.
mpstream implementation of vstream will call port_dump_mpstream, and
luastream implementation of vstream will call port_dump_lua as it does
now.
box.select and iproto_call will use port_dump_obuf.
I prefer the second option: introduce port_dump_mpstream. It is ok for
you?
I may be wrong, but IMO there isn't much point in optimizing box.select,
because it's very limited in its applicability. People already prefer to
use box.call over box.insert/select/etc over iproto, and with the
appearance of box.execute they are likely to stop using plain box.select
at all.
That said, personally I would try to pass reserve/alloc methods to port,
see how it goes.
I do not see a reason to slow down box.select if we can don't do it.
Yeas, people use IPROTO_CALL, but in stored functions they use box
functions including select.
Ok, instead of port_dump_mpstream we can rename port_dump_msgpack to
port_dump_obuf and add port_dump_msgpack which does not depend on
mpstream and takes alloc/reserve/ctx directly.
