Refactor TABLE_CREATE allocation

Using std::string for names removes the need to handle memory
allocation. Moving the column attributes into a class of its own greatly
simplifies the creation of the TABLE_CREATE as well as modifications that
are done to it.

server/modules/routing/avrorouter/avro_file.cc

@@ -933,7 +933,7 @@ bool is_alter_table_statement(Avro *router, char* ptr, size_t len)
 bool save_and_replace_table_create(Avro *router, TABLE_CREATE *created)
 {
     char table_ident[MYSQL_TABLE_MAXLEN + MYSQL_DATABASE_MAXLEN + 2];
-    snprintf(table_ident, sizeof(table_ident), "%s.%s", created->database, created->table);
+    snprintf(table_ident, sizeof(table_ident), "%s.%s", created->database.c_str(), created->table.c_str());
 
     auto it = router->created_tables.find(table_ident);
 
@@ -951,7 +951,7 @@ bool save_and_replace_table_create(Avro *router, TABLE_CREATE *created)
     }
 
     router->created_tables[table_ident] = STableCreate(created);
-    ss_dassert(created->columns > 0);
+    ss_dassert(created->columns.size() > 0);
     return true;
 }
 

server/modules/routing/avrorouter/avro_rbr.cc

@@ -104,7 +104,7 @@ bool handle_table_map_event(Avro *router, REP_HEADER *hdr, uint8_t *ptr)
 
     if (create != router->created_tables.end())
     {
-        ss_dassert(create->second->columns > 0);
+        ss_dassert(create->second->columns.size() > 0);
         auto it = router->table_maps.find(table_ident);
         STableMap map(table_map_alloc(ptr, ev_len, create->second.get()));
 
@@ -305,7 +305,7 @@ bool handle_row_event(Avro *router, REP_HEADER *hdr, uint8_t *ptr)
 
         TABLE_CREATE* create = map->table_create;
 
-        if (table && create && ncolumns == map->columns && create->columns == map->columns)
+        if (table && create && ncolumns == map->columns && create->columns.size() == map->columns)
         {
             avro_value_t record;
             avro_generic_value_new(table->avro_writer_iface, &record);
@@ -363,7 +363,7 @@ bool handle_row_event(Avro *router, REP_HEADER *hdr, uint8_t *ptr)
                       "binary logs or the stored schema was not correct.",
                       map->database, map->table);
         }
-        else if (ncolumns == map->columns && create->columns != map->columns)
+        else if (ncolumns == map->columns && create->columns.size() != map->columns)
         {
             MXS_ERROR("Table map event has a different column count for table "
                       "%s.%s than the CREATE TABLE statement. Possible "
@@ -577,7 +577,8 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value
 
     for (long i = 0; i < ncolumns && npresent < ncolumns; i++)
     {
-        ss_debug(int rc = )avro_value_get_by_name(record, create->column_names[i], &field, NULL);
+        ss_debug(int rc = )avro_value_get_by_name(record, create->columns[i].name.c_str(),
+                                                  &field, NULL);
         ss_dassert(rc == 0);
 
         if (bit_is_set(columns_present, ncolumns, i))
@@ -722,7 +723,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value
                 struct tm tm;
                 ptr += unpack_temporal_value(map->column_types[i], ptr,
                                              &metadata[metadata_offset],
-                                             create->column_lengths[i], &tm);
+                                             create->columns[i].length, &tm);
                 format_temporal_value(buf, sizeof(buf), map->column_types[i], &tm);
                 avro_value_set_string(&field, buf);
                 sprintf(trace[i], "[%ld] %s: %s", i, column_type_to_string(map->column_types[i]), buf);

server/modules/routing/avrorouter/avro_schema.cc

@@ -123,18 +123,13 @@ char* json_new_schema_from_table(TABLE_MAP *map)
     json_array_append_new(array, json_pack_ex(&err, 0, "{s:s, s:o}", "name", avro_event_type,
                                               "type", event_types));
 
-    for (uint64_t i = 0; i < map->columns && i < create->columns; i++)
+    for (uint64_t i = 0; i < map->columns && i < create->columns.size(); i++)
     {
-        ss_info_dassert(create->column_names[i] && *create->column_names[i],
-                        "Column name should not be empty or NULL");
-        ss_info_dassert(create->column_types[i] && *create->column_types[i],
-                        "Column type should not be empty or NULL");
-
         json_array_append_new(array, json_pack_ex(&err, 0, "{s:s, s:s, s:s, s:i}",
-                                                  "name", create->column_names[i],
+                                                  "name", create->columns[i].name.c_str(),
                                                   "type", column_type_to_avro_type(map->column_types[i]),
-                                                  "real_type", create->column_types[i],
+                                                  "real_type", create->columns[i].type.c_str(),
-                                                  "length", create->column_lengths[i]));
+                                                  "length", create->columns[i].length));
     }
     json_object_set_new(schema, "fields", array);
     char* rval = json_dumps(schema, JSON_PRESERVE_ORDER);
@@ -166,7 +161,7 @@ static inline bool not_generated_field(const char* name)
  * @param table The TABLE_CREATE object to populate
  * @return True on success successfully, false on error
  */
-bool json_extract_field_names(const char* filename, TABLE_CREATE *table)
+bool json_extract_field_names(const char* filename, std::vector<Column>& columns)
 {
     bool rval = false;
     json_error_t err;
@@ -175,17 +170,9 @@ bool json_extract_field_names(const char* filename, TABLE_CREATE *table)
 
     if ((obj = json_load_file(filename, 0, &err)) && (arr = json_object_get(obj, "fields")))
     {
-        ss_dassert(json_is_array(arr));
         if (json_is_array(arr))
         {
             int array_size = json_array_size(arr);
-            table->column_names = (char**)MXS_MALLOC(sizeof(char*) * (array_size));
-            table->column_types = (char**)MXS_MALLOC(sizeof(char*) * (array_size));
-            table->column_lengths = (int*)MXS_MALLOC(sizeof(int) * (array_size));
-
-            if (table->column_names && table->column_types && table->column_lengths)
-            {
-                int columns = 0;
             rval = true;
 
             for (int i = 0; i < array_size; i++)
@@ -194,27 +181,6 @@ bool json_extract_field_names(const char* filename, TABLE_CREATE *table)
 
                 if (json_is_object(val))
                 {
-                        json_t* value;
-
-                        if ((value = json_object_get(val, "real_type")) && json_is_string(value))
-                        {
-                            table->column_types[columns] = MXS_STRDUP_A(json_string_value(value));
-                        }
-                        else
-                        {
-                            table->column_types[columns] = MXS_STRDUP_A("unknown");
-                            MXS_WARNING("No \"real_type\" value defined. Treating as unknown type field.");
-                        }
-
-                        if ((value = json_object_get(val, "length")) && json_is_integer(value))
-                        {
-                            table->column_lengths[columns] = json_integer_value(value);
-                        }
-                        else
-                        {
-                            table->column_lengths[columns] = -1;
-                            MXS_WARNING("No \"length\" value defined. Treating as default length field.");
-                        }
 
                     json_t *name = json_object_get(val, "name");
 
@@ -224,33 +190,27 @@ bool json_extract_field_names(const char* filename, TABLE_CREATE *table)
 
                         if (not_generated_field(name_str))
                         {
-                                table->column_names[columns] = MXS_STRDUP_A(name_str);
+                            columns.emplace_back(name_str);
 
                             json_t* value;
 
                             if ((value = json_object_get(val, "real_type")) && json_is_string(value))
                             {
-                                    table->column_types[columns] = MXS_STRDUP_A(json_string_value(value));
+                                columns.back().type = json_string_value(value);
                             }
                             else
                             {
-                                    table->column_types[columns] = MXS_STRDUP_A("unknown");
+                                MXS_WARNING("No \"real_type\" value defined. Treating as unknown type field.");
-                                    MXS_WARNING("No \"real_type\" value defined. "
-                                                "Treating as unknown type field.");
                             }
 
                             if ((value = json_object_get(val, "length")) && json_is_integer(value))
                             {
-                                    table->column_lengths[columns] = json_integer_value(value);
+                                columns.back().length = json_integer_value(value);
                             }
                             else
                             {
-                                    table->column_lengths[columns] = -1;
+                                MXS_WARNING("No \"length\" value defined. Treating as default length field.");
-                                    MXS_WARNING("No \"length\" value defined. "
-                                                "Treating as default length field.");
                             }
-
-                                columns++;
                         }
                     }
                     else
@@ -267,8 +227,6 @@ bool json_extract_field_names(const char* filename, TABLE_CREATE *table)
                     rval = false;
                 }
             }
-                table->columns = columns;
-            }
         }
         else
         {
@@ -639,65 +597,29 @@ int count_columns(const char* ptr)
  * @param nameptr table definition
  * @return Number of processed columns or -1 on error
  */
-static int process_column_definition(const char *nameptr, char*** dest, char*** dest_types, int** dest_lens)
+static void process_column_definition(const char *nameptr, std::vector<Column>& columns)
 {
-    int n = count_columns(nameptr);
-    *dest = static_cast<char**>(MXS_MALLOC(sizeof(char*) * n));
-    *dest_types = static_cast<char**>(MXS_MALLOC(sizeof(char*) * n));
-    *dest_lens = static_cast<int*>(MXS_MALLOC(sizeof(int) * n));
-
-    char **names = *dest;
-    char **types = *dest_types;
-    int *lengths = *dest_lens;
     char colname[512];
-    int i = 0;
 
     while ((nameptr = extract_field_name(nameptr, colname, sizeof(colname))))
     {
-        ss_dassert(i < n);
         char type[100] = "";
         int len = extract_type_length(nameptr, type);
         nameptr = next_field_definition(nameptr);
         fix_reserved_word(colname);
-
+        columns.emplace_back(colname, type, len);
-        lengths[i] = len;
-        types[i] = MXS_STRDUP_A(type);
-        names[i] = MXS_STRDUP_A(colname);
-        ss_info_dassert(*names[i] && *types[i], "`name` and `type` must not be empty");
-        i++;
     }
-
-    return i;
 }
 
 TABLE_CREATE* table_create_from_schema(const char* file, const char* db,
                                        const char* table, int version)
 {
-    db = MXS_STRDUP(db);
+    TABLE_CREATE* newtable = NULL;
-    table = MXS_STRDUP(table);
+    std::vector<Column> columns;
 
-    TABLE_CREATE* newtable = (TABLE_CREATE*)MXS_MALLOC(sizeof(TABLE_CREATE));
+    if (json_extract_field_names(file, columns))
-
-    if (!db || !table || !newtable)
     {
-        MXS_FREE((void*)db);
+        newtable = new (std::nothrow)TABLE_CREATE(db, table, version, columns);
-        MXS_FREE((void*)table);
-        MXS_FREE(newtable);
-
-        return NULL;
-    }
-
-    newtable->table = (char*)table;
-    newtable->database = (char*)db;
-    newtable->version = version;
-    newtable->was_used = true;
-
-    if (!json_extract_field_names(file, newtable))
-    {
-        MXS_FREE(newtable->table);
-        MXS_FREE(newtable->database);
-        MXS_FREE(newtable);
-        newtable = NULL;
     }
 
     return newtable;
@@ -743,50 +665,21 @@ TABLE_CREATE* table_create_alloc(char* ident, const char* sql, int len)
     strcpy(database, ident);
     strcpy(table, tbl_start);
 
-    int* lengths = NULL;
+    std::vector<Column> columns;
-    char **names = NULL;
+    process_column_definition(statement_sql, columns);
-    char **types = NULL;
-    int n_columns = process_column_definition(statement_sql, &names, &types, &lengths);
-    ss_dassert(n_columns > 0);
 
-    /** We have appear to have a valid CREATE TABLE statement */
     TABLE_CREATE *rval = NULL;
-    if (n_columns > 0)
-    {
-        if ((rval = new (std::nothrow) TABLE_CREATE))
-        {
-            rval->version = resolve_table_version(database, table);
-            rval->was_used = false;
-            rval->column_names = names;
-            rval->column_lengths = lengths;
-            rval->column_types = types;
-            rval->columns = n_columns;
-            rval->database = MXS_STRDUP(database);
-            rval->table = MXS_STRDUP(table);
-        }
 
-        if (rval == NULL || rval->database == NULL || rval->table == NULL)
+    if (!columns.empty())
     {
-            if (rval)
+        int version = resolve_table_version(database, table);
-            {
+        rval = new (std::nothrow) TABLE_CREATE(database, table, version, columns);
-                MXS_FREE(rval->database);
-                MXS_FREE(rval->table);
-                delete rval;
-            }
-
-            for (int i = 0; i < n_columns; i++)
-            {
-                MXS_FREE(names[i]);
-            }
-
-            MXS_FREE(names);
-            rval = NULL;
-        }
     }
     else
     {
         MXS_ERROR("No columns in a CREATE TABLE statement: %.*s", stmt_len, statement_sql);
     }
+
     return rval;
 }
 
@@ -994,33 +887,12 @@ TABLE_CREATE* table_create_copy(Avro *router, const char* sql, size_t len, const
 
         if (it != router->created_tables.end())
         {
-            auto old = it->second;
+            rval = new (std::nothrow) TABLE_CREATE(*it->second);
-            int n = old->columns;
-            char** names = static_cast<char**>(MXS_MALLOC(sizeof(char*) * n));
-            char** types = static_cast<char**>(MXS_MALLOC(sizeof(char*) * n));
-            int* lengths = static_cast<int*>(MXS_MALLOC(sizeof(int) * n));
-            rval = new (std::nothrow) TABLE_CREATE;
-
-            MXS_ABORT_IF_FALSE(names && types && lengths && rval);
-
-            for (uint64_t i = 0; i < old->columns; i++)
-            {
-                names[i] = MXS_STRDUP_A(old->column_names[i]);
-                types[i] = MXS_STRDUP_A(old->column_types[i]);
-                lengths[i] = old->column_lengths[i];
-            }
-
-            rval->version = 1;
-            rval->was_used = false;
-            rval->column_names = names;
-            rval->column_lengths = lengths;
-            rval->column_types = types;
-            rval->columns = old->columns;
-            rval->database = MXS_STRDUP_A(db);
-
             char* table = strchr(target, '.');
             table = table ? table + 1 : target;
-            rval->table = MXS_STRDUP_A(table);
+            rval->table = table;
+            rval->version = 1;
+            rval->was_used = false;
         }
         else
         {
@@ -1365,34 +1237,6 @@ void make_avro_token(char* dest, const char* src, int length)
     fix_reserved_word(dest);
 }
 
-int get_column_index(TABLE_CREATE *create, const char *tok, int len)
-{
-    int idx = -1;
-    char safe_tok[len + 2];
-    memcpy(safe_tok, tok, len);
-    safe_tok[len] = '\0';
-
-    if (*safe_tok == '`')
-    {
-        int toklen = strlen(safe_tok) - 2; // Token length without backticks
-        memmove(safe_tok, safe_tok + 1, toklen); // Overwrite first backtick
-        safe_tok[toklen] = '\0'; // Null-terminate the string before the second backtick
-    }
-
-    fix_reserved_word(safe_tok);
-
-    for (size_t x = 0; x < create->columns; x++)
-    {
-        if (strcasecmp(create->column_names[x], safe_tok) == 0)
-        {
-            idx = x;
-            break;
-        }
-    }
-
-    return idx;
-}
-
 static bool not_column_operation(const char* tok, int len)
 {
     const char* keywords[] =
@@ -1458,15 +1302,17 @@ bool table_create_alter(TABLE_CREATE *create, const char *sql, const char *end)
                     tok = get_tok(tok + len, &len, end);
                 }
 
-                if (tok_eq(ptok, "add", plen))
-                {
                 char avro_token[len + 1];
                 make_avro_token(avro_token, tok, len);
+
+                if (tok_eq(ptok, "add", plen))
+                {
+
                     bool is_new = true;
 
-                    for (uint64_t i = 0; i < create->columns; i++)
+                    for (auto it = create->columns.begin(); it != create->columns.end(); it++)
                     {
-                        if (strcmp(avro_token, create->column_names[i]) == 0)
+                        if (it->name == avro_token)
                         {
                             is_new = false;
                             break;
@@ -1475,22 +1321,11 @@ bool table_create_alter(TABLE_CREATE *create, const char *sql, const char *end)
 
                     if (is_new)
                     {
-                        create->column_names =
-                            static_cast<char**>(MXS_REALLOC(create->column_names,
-                                                            sizeof(char*) * (create->columns + 1)));
-                        create->column_types =
-                            static_cast<char**>(MXS_REALLOC(create->column_types,
-                                                            sizeof(char*) * (create->columns + 1)));
-                        create->column_lengths =
-                            static_cast<int*>(MXS_REALLOC(create->column_lengths,
-                                                          sizeof(int) * (create->columns + 1)));
-
                         char field_type[200] = ""; // Enough to hold all types
                         int field_length = extract_type_length(tok + len, field_type);
-                        create->column_names[create->columns] = MXS_STRDUP_A(avro_token);
+                        create->columns.emplace_back(std::string(avro_token),
-                        create->column_types[create->columns] = MXS_STRDUP_A(field_type);
+                                                     std::string(field_type),
-                        create->column_lengths[create->columns] = field_length;
+                                                     field_length);
-                        create->columns++;
                         updates++;
                     }
                     tok = get_next_def(tok, end);
@@ -1498,53 +1333,42 @@ bool table_create_alter(TABLE_CREATE *create, const char *sql, const char *end)
                 }
                 else if (tok_eq(ptok, "drop", plen))
                 {
-                    int idx = get_column_index(create, tok, len);
+                    for (auto it = create->columns.begin(); it != create->columns.end(); it++)
-
-                    if (idx != -1)
                     {
-                        MXS_FREE(create->column_names[idx]);
+                        if (it->name == avro_token)
-                        MXS_FREE(create->column_types[idx]);
-                        for (int i = idx; i < (int)create->columns - 1; i++)
                         {
-                            create->column_names[i] = create->column_names[i + 1];
+                            create->columns.erase(it);
-                            create->column_types[i] = create->column_types[i + 1];
+                            break;
-                            create->column_lengths[i] = create->column_lengths[i + 1];
+                        }
                     }
 
-                        create->column_names =
-                            static_cast<char**>(MXS_REALLOC(create->column_names,
-                                                            sizeof(char*) * (create->columns - 1)));
-                        create->column_types =
-                            static_cast<char**>(MXS_REALLOC(create->column_types,
-                                                            sizeof(char*) * (create->columns - 1)));
-                        create->column_lengths =
-                            static_cast<int*>(MXS_REALLOC(create->column_lengths,
-                                                          sizeof(int) * (create->columns - 1)));
-                        create->columns--;
                     updates++;
-                    }
 
                     tok = get_next_def(tok, end);
                     len = 0;
                 }
                 else if (tok_eq(ptok, "change", plen))
                 {
-                    int idx = get_column_index(create, tok, len);
+                    for (auto it = create->columns.begin(); it != create->columns.end(); it++)
-
+                    {
-                    if (idx != -1 && (tok = get_tok(tok + len, &len, end)))
+                        if (it->name == avro_token)
+                        {
+                            if ((tok = get_tok(tok + len, &len, end)))
                             {
-                        MXS_FREE(create->column_names[idx]);
-                        MXS_FREE(create->column_types[idx]);
                                 char avro_token[len + 1];
                                 make_avro_token(avro_token, tok, len);
                                 char field_type[200] = ""; // Enough to hold all types
                                 int field_length = extract_type_length(tok + len, field_type);
-                        create->column_names[idx] = MXS_STRDUP_A(avro_token);
+                                it->name = avro_token;
-                        create->column_types[idx] = MXS_STRDUP_A(field_type);
+                                it->type = field_type;
-                        create->column_lengths[idx] = field_length;
+                                it->length = field_length;
                                 updates++;
                             }
 
+                        }
+
+                    }
+
                     tok = get_next_def(tok, end);
                     len = 0;
                 }

server/modules/routing/avrorouter/avrorouter.hh

@@ -18,6 +18,7 @@
 #include <stdbool.h>
 #include <stdint.h>
 #include <string>
+#include <vector>
 #include <tr1/memory>
 #include <blr_constants.h>
 #include <maxscale/alloc.h>
@@ -120,29 +121,37 @@ typedef enum avro_binlog_end
 /** How many bytes each thread tries to send */
 #define AVRO_DATA_BURST_SIZE (32 * 1024)
 
+/** A single column in a CREATE TABLE statement */
+struct Column
+{
+    Column(std::string name, std::string type = "unknown", int length = -1):
+        name(name),
+        type(type),
+        length(length)
+    {
+    }
+
+    std::string name;
+    std::string type;
+    int         length;
+};
+
 /** A CREATE TABLE abstraction */
 struct TABLE_CREATE
 {
-    ~TABLE_CREATE()
+    TABLE_CREATE(std::string db, std::string table, int version, std::vector<Column>& cols):
+        table(table),
+        database(db),
+        version(version),
+        was_used(false)
+
     {
-        for (uint64_t i = 0; i < columns; i++)
+        columns.swap(cols);
-        {
-            MXS_FREE(column_names[i]);
-            MXS_FREE(column_types[i]);
-        }
-        MXS_FREE(column_names);
-        MXS_FREE(column_types);
-        MXS_FREE(column_lengths);
-        MXS_FREE(table);
-        MXS_FREE(database);
     }
 
-    uint64_t columns;
+    std::vector<Column>        columns;
-    char**   column_names;
+    std::string                table;
-    char**   column_types;
+    std::string                database;
-    int*     column_lengths;
-    char*    table;
-    char*    database;
     int                        version;  /**< How many versions of this table have been used */
     bool                       was_used; /**< Has this schema been persisted to disk */
 };