MySQLnd: Simplify management of zval row buffer

Something odd was being done here, with the row packet having a
flag for whether it should allocate the zval buffer, which would
then get moved into the result set.

Keep the management of this buffer purely at the result set level.
This also allows us to easily reuse the same buffer for all results,
rather than allocating a new one for each fetch.

diff --git a/ext/mysqlnd/mysqlnd_ps.c b/ext/mysqlnd/mysqlnd_ps.c
index c7b92ab042877fe4ca70f15b55192e8f2b8b95ba..ba6085fce4e165f57843a4f8608b5ae698d405a5 100644 (file)
--- a/ext/mysqlnd/mysqlnd_ps.c
+++ b/ext/mysqlnd/mysqlnd_ps.c
@@ -841,9 +841,6 @@ mysqlnd_stmt_fetch_row_unbuffered(MYSQLND_RES * result, void * param, const unsi
                DBG_RETURN(FAIL);
        }
 
-       /* Let the row packet fill our buffer and skip additional malloc + memcpy */
-       row_packet->skip_extraction = stmt && stmt->result_bind? FALSE:TRUE;
-
        checkpoint = result->memory_pool->checkpoint;
        mysqlnd_mempool_save_state(result->memory_pool);
 
@@ -854,12 +851,10 @@ mysqlnd_stmt_fetch_row_unbuffered(MYSQLND_RES * result, void * param, const unsi
        if (PASS == (ret = PACKET_READ(conn, row_packet)) && !row_packet->eof) {
                unsigned int i, field_count = result->field_count;
 
-               if (!row_packet->skip_extraction) {
+               if (stmt && stmt->result_bind) {
                        result->unbuf->m.free_last_data(result->unbuf, conn->stats);
 
-                       result->unbuf->last_row_data = row_packet->fields;
                        result->unbuf->last_row_buffer = row_packet->row_buffer;
-                       row_packet->fields = NULL;
                        row_packet->row_buffer.ptr = NULL;
 
                        if (PASS != result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,
@@ -1029,19 +1024,15 @@ mysqlnd_fetch_stmt_row_cursor(MYSQLND_RES * result, void * param, const unsigned
 
        }
 
-       row_packet->skip_extraction = stmt->result_bind? FALSE:TRUE;
-
        UPSERT_STATUS_RESET(stmt->upsert_status);
        if (PASS == (ret = PACKET_READ(conn, row_packet)) && !row_packet->eof) {
                const MYSQLND_RES_METADATA * const meta = result->meta;
                unsigned int i, field_count = result->field_count;
 
-               if (!row_packet->skip_extraction) {
+               if (stmt->result_bind) {
                        result->unbuf->m.free_last_data(result->unbuf, conn->stats);
 
-                       result->unbuf->last_row_data = row_packet->fields;
                        result->unbuf->last_row_buffer = row_packet->row_buffer;
-                       row_packet->fields = NULL;
                        row_packet->row_buffer.ptr = NULL;
 
                        if (PASS != result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,

diff --git a/ext/mysqlnd/mysqlnd_result.c b/ext/mysqlnd/mysqlnd_result.c
index f67f8bdcc172f30a653ddbe3a60e5c71fd9843d1..af042030ce6e5518ea9899b51428bbcd0a3c70dd 100644 (file)
--- a/ext/mysqlnd/mysqlnd_result.c
+++ b/ext/mysqlnd/mysqlnd_result.c
@@ -147,18 +147,12 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, free_last_data)(MYSQLND_RES_UNBUFFERED
        }
 
        DBG_INF_FMT("field_count=%u", unbuf->field_count);
-       if (unbuf->last_row_data) {
-               unsigned int i;
-               for (i = 0; i < unbuf->field_count; i++) {
-                       zval_ptr_dtor_nogc(&(unbuf->last_row_data[i]));
-               }
-
-               /* Free last row's zvals */
-               mnd_efree(unbuf->last_row_data);
-               unbuf->last_row_data = NULL;
-       }
        if (unbuf->last_row_buffer.ptr) {
                DBG_INF("Freeing last row buffer");
+               for (unsigned i = 0; i < unbuf->field_count; i++) {
+                       zval_ptr_dtor_nogc(&unbuf->last_row_data[i]);
+               }
+
                /* Nothing points to this buffer now, free it */
                unbuf->result_set_memory_pool->free_chunk(
                        unbuf->result_set_memory_pool, unbuf->last_row_buffer.ptr);
@@ -612,7 +606,7 @@ static const size_t *
 MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_lengths)(const MYSQLND_RES_UNBUFFERED * const result)
 {
        /* simulate output of libmysql */
-       return (result->last_row_data || result->eof_reached)? result->lengths : NULL;
+       return (result->last_row_buffer.ptr || result->eof_reached)? result->lengths : NULL;
 }
 /* }}} */
 
@@ -660,8 +654,6 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_row_c)(MYSQLND_RES * result, voi
                /* Not fully initialized object that is being cleaned up */
                DBG_RETURN(FAIL);
        }
-       /* Let the row packet fill our buffer and skip additional mnd_malloc + memcpy */
-       row_packet->skip_extraction = FALSE;
 
        checkpoint = result->memory_pool->checkpoint;
        mysqlnd_mempool_save_state(result->memory_pool);
@@ -673,52 +665,48 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_row_c)(MYSQLND_RES * result, voi
        if (PASS == (ret = PACKET_READ(conn, row_packet)) && !row_packet->eof) {
                result->unbuf->m.free_last_data(result->unbuf, conn->stats);
 
-               result->unbuf->last_row_data = row_packet->fields;
                result->unbuf->last_row_buffer = row_packet->row_buffer;
-               row_packet->fields = NULL;
                row_packet->row_buffer.ptr = NULL;
 
                MYSQLND_INC_CONN_STATISTIC(conn->stats, STAT_ROWS_FETCHED_FROM_CLIENT_NORMAL_UNBUF);
 
-               if (!row_packet->skip_extraction) {
-                       unsigned int i, field_count = meta->field_count;
+               unsigned int i, field_count = meta->field_count;
+
+               enum_func_status rc = result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,
+                                                                               result->unbuf->last_row_data,
+                                                                               field_count,
+                                                                               row_packet->fields_metadata,
+                                                                               conn->options->int_and_float_native,
+                                                                               conn->stats);
+               if (PASS != rc) {
+                       mysqlnd_mempool_restore_state(result->memory_pool);
+                       result->memory_pool->checkpoint = checkpoint;
+                       DBG_RETURN(FAIL);
+               }
+               {
+                       *row = mnd_emalloc(field_count * sizeof(char *));
+                       MYSQLND_FIELD * field = meta->fields;
+                       size_t * lengths = result->unbuf->lengths;
 
-                       enum_func_status rc = result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,
-                                                                                       result->unbuf->last_row_data,
-                                                                                       field_count,
-                                                                                       row_packet->fields_metadata,
-                                                                                       conn->options->int_and_float_native,
-                                                                                       conn->stats);
-                       if (PASS != rc) {
-                               mysqlnd_mempool_restore_state(result->memory_pool);
-                               result->memory_pool->checkpoint = checkpoint;
-                               DBG_RETURN(FAIL);
-                       }
-                       {
-                               *row = mnd_emalloc(field_count * sizeof(char *));
-                               MYSQLND_FIELD * field = meta->fields;
-                               size_t * lengths = result->unbuf->lengths;
-
-                               for (i = 0; i < field_count; i++, field++) {
-                                       zval * data = &result->unbuf->last_row_data[i];
-                                       const size_t len = (Z_TYPE_P(data) == IS_STRING)? Z_STRLEN_P(data) : 0;
+                       for (i = 0; i < field_count; i++, field++) {
+                               zval * data = &result->unbuf->last_row_data[i];
+                               const size_t len = (Z_TYPE_P(data) == IS_STRING)? Z_STRLEN_P(data) : 0;
 
 /* BEGIN difference between normal normal fetch and _c */
-                                       if (Z_TYPE_P(data) != IS_NULL) {
-                                               convert_to_string(data);
-                                               (*row)[i] = Z_STRVAL_P(data);
-                                       } else {
-                                               (*row)[i] = NULL;
-                                       }
+                               if (Z_TYPE_P(data) != IS_NULL) {
+                                       convert_to_string(data);
+                                       (*row)[i] = Z_STRVAL_P(data);
+                               } else {
+                                       (*row)[i] = NULL;
+                               }
 /* END difference between normal normal fetch and _c */
 
-                                       if (lengths) {
-                                               lengths[i] = len;
-                                       }
+                               if (lengths) {
+                                       lengths[i] = len;
+                               }
 
-                                       if (field->max_length < len) {
-                                               field->max_length = len;
-                                       }
+                               if (field->max_length < len) {
+                                       field->max_length = len;
                                }
                        }
                }
@@ -788,8 +776,6 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_row)(MYSQLND_RES * result, void
                /* Not fully initialized object that is being cleaned up */
                DBG_RETURN(FAIL);
        }
-       /* Let the row packet fill our buffer and skip additional mnd_malloc + memcpy */
-       row_packet->skip_extraction = row? FALSE:TRUE;
 
        checkpoint = result->memory_pool->checkpoint;
        mysqlnd_mempool_save_state(result->memory_pool);
@@ -801,14 +787,12 @@ MYSQLND_METHOD(mysqlnd_result_unbuffered, fetch_row)(MYSQLND_RES * result, void
        if (PASS == (ret = PACKET_READ(conn, row_packet)) && !row_packet->eof) {
                result->unbuf->m.free_last_data(result->unbuf, conn->stats);
 
-               result->unbuf->last_row_data = row_packet->fields;
                result->unbuf->last_row_buffer = row_packet->row_buffer;
-               row_packet->fields = NULL;
                row_packet->row_buffer.ptr = NULL;
 
                MYSQLND_INC_CONN_STATISTIC(conn->stats, STAT_ROWS_FETCHED_FROM_CLIENT_NORMAL_UNBUF);
 
-               if (!row_packet->skip_extraction) {
+               if (row) {
                        unsigned int i, field_count = meta->field_count;
 
                        enum_func_status rc = result->unbuf->m.row_decoder(&result->unbuf->last_row_buffer,
@@ -1263,8 +1247,6 @@ MYSQLND_METHOD(mysqlnd_res, store_result_fetch_data)(MYSQLND_CONN_DATA * const c
        row_packet.binary_protocol = binary_protocol;
        row_packet.fields_metadata = meta->fields;
 
-       row_packet.skip_extraction = TRUE; /* let php_mysqlnd_rowp_read() not allocate row_packet.fields, we will do it */
-
        while (FAIL != (ret = PACKET_READ(conn, &row_packet)) && !row_packet.eof) {
                if (!free_rows) {
                        MYSQLND_ROW_BUFFER * new_row_buffers;
@@ -1301,7 +1283,6 @@ MYSQLND_METHOD(mysqlnd_res, store_result_fetch_data)(MYSQLND_CONN_DATA * const c
                set->row_count++;
 
                /* So row_packet's destructor function won't efree() it */
-               row_packet.fields = NULL;
                row_packet.row_buffer.ptr = NULL;
 
                /*
@@ -1459,7 +1440,9 @@ MYSQLND_METHOD(mysqlnd_res, skip_result)(MYSQLND_RES * const result)
                                                                                                                                                STAT_FLUSHED_PS_SETS);
 
                while ((PASS == result->m.fetch_row(result, NULL, 0, &fetched_anything)) && fetched_anything == TRUE) {
-                       /* do nothing */;
+                       MYSQLND_INC_CONN_STATISTIC(conn->stats,
+                               result->type == MYSQLND_RES_NORMAL
+                                       ? STAT_ROWS_SKIPPED_NORMAL : STAT_ROWS_SKIPPED_PS);
                }
        }
        DBG_RETURN(PASS);
@@ -1880,6 +1863,9 @@ mysqlnd_result_unbuffered_init(MYSQLND_RES *result, const unsigned int field_cou
        ret->lengths = pool->get_chunk(pool, field_count * sizeof(size_t));
        memset(ret->lengths, 0, field_count * sizeof(size_t));
 
+       ret->last_row_data = pool->get_chunk(pool, field_count * sizeof(zval));
+       memset(ret->last_row_data, 0, field_count * sizeof(zval));
+
        ret->result_set_memory_pool = pool;
        ret->field_count= field_count;
        ret->ps = ps;

diff --git a/ext/mysqlnd/mysqlnd_wireprotocol.c b/ext/mysqlnd/mysqlnd_wireprotocol.c
index ef32d90274183d53392d4e669177e72ff811635b..d33e266b6dccae24a0f8b2ea8ad8eddc9ecdff3b 100644 (file)
--- a/ext/mysqlnd/mysqlnd_wireprotocol.c
+++ b/ext/mysqlnd/mysqlnd_wireprotocol.c
@@ -1696,10 +1696,6 @@ php_mysqlnd_rowp_read_text_protocol_c(MYSQLND_ROW_BUFFER * row_buffer, zval * fi
 
 
 /* {{{ php_mysqlnd_rowp_read */
-/*
-  if normal statements => packet->fields is created by this function,
-  if PS => packet->fields is passed from outside
-*/
 static enum_func_status
 php_mysqlnd_rowp_read(MYSQLND_CONN_DATA * conn, void * _packet)
 {
@@ -1760,33 +1756,10 @@ php_mysqlnd_rowp_read(MYSQLND_CONN_DATA * conn, void * _packet)
                        DBG_INF_FMT("server_status=%u warning_count=%u", packet->server_status, packet->warning_count);
                }
        } else {
+               packet->eof = FALSE;
                MYSQLND_INC_CONN_STATISTIC(stats,
                                                                        packet->binary_protocol? STAT_ROWS_FETCHED_FROM_SERVER_PS:
                                                                                                                         STAT_ROWS_FETCHED_FROM_SERVER_NORMAL);
-
-               packet->eof = FALSE;
-               /* packet->field_count is set by the user of the packet */
-
-               if (!packet->skip_extraction) {
-                       if (!packet->fields) {
-                               DBG_INF("Allocating packet->fields");
-                               /*
-                                 old-API will probably set packet->fields to NULL every time, though for
-                                 unbuffered sets it makes not much sense as the zvals in this buffer matter,
-                                 not the buffer. Constantly allocating and deallocating brings nothing.
-
-                                 For PS - if stmt_store() is performed, thus we don't have a cursor, it will
-                                 behave just like old-API buffered. Cursors will behave like a bit different,
-                                 but mostly like old-API unbuffered and thus will populate this array with
-                                 value.
-                               */
-                               packet->fields = mnd_ecalloc(packet->field_count, sizeof(zval));
-                       }
-               } else {
-                       MYSQLND_INC_CONN_STATISTIC(stats,
-                                                                               packet->binary_protocol? STAT_ROWS_SKIPPED_PS:
-                                                                                                                                STAT_ROWS_SKIPPED_NORMAL);
-               }
        }
 
 end:
@@ -1807,13 +1780,6 @@ php_mysqlnd_rowp_free_mem(void * _packet)
                p->result_set_memory_pool->free_chunk(p->result_set_memory_pool, p->row_buffer.ptr);
                p->row_buffer.ptr = NULL;
        }
-       /*
-         Don't free packet->fields :
-         - normal queries -> store_result() | fetch_row_unbuffered() will transfer
-           the ownership and NULL it.
-         - PS will pass in it the bound variables, we have to use them! and of course
-           not free the array. As it is passed to us, we should not clean it ourselves.
-       */
        DBG_VOID_RETURN;
 }
 /* }}} */

diff --git a/ext/mysqlnd/mysqlnd_wireprotocol.h b/ext/mysqlnd/mysqlnd_wireprotocol.h
index e917736bb5bbf9ab49d1ec736c7252fec44024a2..28d521644e1418b6c69277a6097a87f0ed793ce7 100644 (file)
--- a/ext/mysqlnd/mysqlnd_wireprotocol.h
+++ b/ext/mysqlnd/mysqlnd_wireprotocol.h
@@ -206,7 +206,6 @@ typedef struct st_mysqlnd_packet_res_field {
 /* Row packet */
 typedef struct st_mysqlnd_packet_row {
        MYSQLND_PACKET_HEADER   header;
-       zval            *fields;
        uint32_t        field_count;
        zend_bool       eof;
        /*
@@ -219,7 +218,6 @@ typedef struct st_mysqlnd_packet_row {
        MYSQLND_ROW_BUFFER      row_buffer;
        MYSQLND_MEMORY_POOL * result_set_memory_pool;
 
-       zend_bool               skip_extraction;
        zend_bool               binary_protocol;
        MYSQLND_FIELD   *fields_metadata;