diff --git a/BUILD/mdbci/build.sh b/BUILD/mdbci/build.sh index abdb0f4b2..a18c16f92 100755 --- a/BUILD/mdbci/build.sh +++ b/BUILD/mdbci/build.sh @@ -66,9 +66,7 @@ $(<${script_dir}/templates/build.json.template) # destroying existing box if [ -d "$MDBCI_VM_PATH/${name}" ]; then - cd $MDBCI_VM_PATH/${name} - vagrant destroy -f - cd ${dir} + ${mdbci_dir}/mdbci destroy $name fi # starting VM for build @@ -78,9 +76,7 @@ $(<${script_dir}/templates/build.json.template) ${mdbci_dir}/mdbci up --attempts=1 $name if [ $? != 0 ] ; then echo "Error starting VM" - cd $MDBCI_VM_PATH/${name} rm ~/vagrant_lock - cd $dir exit 1 fi echo "copying public keys to VM" @@ -122,11 +118,7 @@ if [ "$try_already_running" == "yes" ] ; then fi if [[ "$do_not_destroy_vm" != "yes" && "$try_already_running" != "yes" ]] ; then echo "Destroying VM" - vagrant destroy -f - cd .. - rm -rf $name - rm -rf ${name}.json - rm -rf ${name}_netwotk_config + ${mdbci_dir}/mdbci destroy $name fi cd $dir diff --git a/BUILD/mdbci/upgrade_test.sh b/BUILD/mdbci/upgrade_test.sh index 530969a5c..45b6974b1 100755 --- a/BUILD/mdbci/upgrade_test.sh +++ b/BUILD/mdbci/upgrade_test.sh @@ -30,9 +30,7 @@ echo $JOB_NAME-$BUILD_NUMBER >> ~/vagrant_lock # destroying existing box if [ -d "install_$box" ]; then - cd $MDBCI_VM_PATH/$name - vagrant destroy -f - cd $dir + ${mdbci_dir}/mdbci destroy $name fi ${mdbci_dir}/repository-config/generate_all.sh repo.d @@ -44,11 +42,9 @@ ${mdbci_dir}/mdbci up $name --attempts=1 if [ $? != 0 ] ; then if [ $? != 0 ] ; then echo "Error starting VM" - cd ${MDBCI_VM_PATH}/$name if [ "x$do_not_destroy_vm" != "xyes" ] ; then - vagrant destroy -f + ${mdbci_dir}/mdbci destroy $name fi - cd $dir rm ~/vagrant_lock exit 1 fi @@ -126,9 +122,7 @@ scp $scpopt $sshuser@$IP:/var/log/maxscale/* $logs_publish_dir chmod a+r $logs_publish_dir/* if [ "x$do_not_destroy_vm" != "xyes" ] ; then - cd $MDBCI_VM_PATH/$name - vagrant destroy -f - cd $dir + ${mdbci_dir}/mdbci destroy $name fi kill $pid_to_kill exit $res diff --git a/maxscale-system-test/cdc_datatypes/cdc_datatypes.cpp b/maxscale-system-test/cdc_datatypes/cdc_datatypes.cpp index 65ee60ae6..4b1910864 100644 --- a/maxscale-system-test/cdc_datatypes/cdc_datatypes.cpp +++ b/maxscale-system-test/cdc_datatypes/cdc_datatypes.cpp @@ -36,6 +36,7 @@ static const char* decimal_types[] = "FLOAT", "DOUBLE", "DECIMAL(10, 2)", + "DECIMAL(32, 2)", NULL }; @@ -89,16 +90,48 @@ static const char* binary_values[] = NULL }; +static const char* datetime_types[] = +{ + "DATETIME(1)", + "DATETIME(2)", + "DATETIME(3)", + "DATETIME(4)", + "DATETIME(5)", + "DATETIME(6)", + "TIMESTAMP", + NULL +}; + +static const char* datetime_values[] = +{ + "'2018-01-01 11:11:11'", + NULL +}; + +static const char* date_types[] = +{ + "DATE", + NULL +}; + +static const char* date_values[] = +{ + "'2018-01-01'", + NULL +}; + struct { const char** types; const char** values; } test_set[] { - { integer_types, integer_values }, - { decimal_types, decimal_values }, - { string_types, string_values }, - { binary_types, binary_values }, + { integer_types, integer_values }, + { decimal_types, decimal_values }, + { string_types, string_values }, + { binary_types, binary_values }, + { datetime_types, datetime_values }, + { date_types, date_values }, { 0 } }; diff --git a/maxscale-system-test/check_backend.cpp b/maxscale-system-test/check_backend.cpp index 85b8fa07d..e78cafbdc 100644 --- a/maxscale-system-test/check_backend.cpp +++ b/maxscale-system-test/check_backend.cpp @@ -8,11 +8,14 @@ int main(int argc, char *argv[]) { - int exit_code; + TestConnections * Test = new TestConnections(argc, argv); - /*Test->maxscales->restart_maxscale(0); - sleep(5);*/ + std::string src = std::string(test_dir) + "/mdbci/add_core_cnf.sh"; + Test->maxscales->copy_to_node(0, src.c_str(), Test->maxscales->access_homedir[0]); + Test->maxscales->ssh_node_f(0, true, "%s/add_core_cnf.sh %s", Test->maxscales->access_homedir[0], + Test->verbose ? "verbose" : ""); + Test->set_timeout(10); Test->tprintf("Connecting to Maxscale maxscales->routers[0] with Master/Slave backend\n"); @@ -40,6 +43,7 @@ int main(int argc, char *argv[]) Test->maxscales->close_maxscale_connections(0); Test->check_maxscale_alive(0); + int exit_code = 0; char * ver = Test->maxscales->ssh_node_output(0, "maxscale --version-full", false, &exit_code); Test->tprintf("Maxscale_full_version_start:\n%s\nMaxscale_full_version_end\n", ver); diff --git a/maxscale-system-test/mdbci/add_core_cnf.sh b/maxscale-system-test/mdbci/add_core_cnf.sh index aa5db1828..21a69f460 100755 --- a/maxscale-system-test/mdbci/add_core_cnf.sh +++ b/maxscale-system-test/mdbci/add_core_cnf.sh @@ -1,4 +1,8 @@ -set -x +if [ "$1" == "verbose" ] +then + set -x +fi + chmod 777 /tmp/ echo 2 > /proc/sys/fs/suid_dumpable sed -i "s/start() {/start() { \n export DAEMON_COREFILE_LIMIT='unlimited'; ulimit -c unlimited; /" /etc/init.d/maxscale diff --git a/maxscale-system-test/mdbci/configure_core.sh b/maxscale-system-test/mdbci/configure_core.sh deleted file mode 100755 index 2c72b5ac9..000000000 --- a/maxscale-system-test/mdbci/configure_core.sh +++ /dev/null @@ -1,11 +0,0 @@ -#!/bin/bash -set -x - -ssh -i $maxscale_sshkey -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no $maxscale_access_user@$maxscale_IP '$maxscale_access_sudo service iptables stop' - -ssh -i $maxscale_sshkey -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no $maxscale_access_user@$maxscale_IP "$maxscale_access_sudo mkdir ccore; $maxscale_access_sudo chown $maxscale_access_user ccore" - -scp -i $maxscale_sshkey -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no ${script_dir}/add_core_cnf.sh $maxscale_access_user@$maxscale_IP:./ccore/ -ssh -i $maxscale_sshkey -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no $maxscale_access_user@$maxscale_IP "$maxscale_access_sudo /home/$maxscale_access_user/ccore/add_core_cnf.sh" - -set +x diff --git a/maxscale-system-test/mdbci/create_config.sh b/maxscale-system-test/mdbci/create_config.sh index e35874938..14cbb3f42 100755 --- a/maxscale-system-test/mdbci/create_config.sh +++ b/maxscale-system-test/mdbci/create_config.sh @@ -16,10 +16,12 @@ export repo_dir=$dir/repo.d/ export provider=`${mdbci_dir}/mdbci show provider $box --silent 2> /dev/null` export backend_box=${backend_box:-"centos_7_"$provider} +if [ "$product" == "mysql" ] ; then + export cnf_path=${script_dir}/cnf/mysql56 +fi + +${mdbci_dir}/mdbci destroy $name mkdir -p ${MDBCI_VM_PATH}/$name -cd ${MDBCI_VM_PATH}/$name -vagrant destroy -f -cd $dir export cnf_path="${MDBCI_VM_PATH}/$name/cnf/" if [ "$product" == "mysql" ] ; then diff --git a/maxscale-system-test/mdbci/destroy.sh b/maxscale-system-test/mdbci/destroy.sh deleted file mode 100755 index 8d9597732..000000000 --- a/maxscale-system-test/mdbci/destroy.sh +++ /dev/null @@ -1,10 +0,0 @@ -#!/bin/bash - -dir=`pwd` -cd ${MDBCI_VM_PATH}/${name} -vagrant destroy -f -cd $dir - -rm -rf ${MDBCI_VM_PATH}/${name} -rm -rf ${MDBCI_VM_PATH}/${name}.json -rm -rf ${MDBCI_VM_PATH}/${name}_network_config diff --git a/maxscale-system-test/mdbci/run_test.sh b/maxscale-system-test/mdbci/run_test.sh index c74ab8dc9..609d35e7c 100755 --- a/maxscale-system-test/mdbci/run_test.sh +++ b/maxscale-system-test/mdbci/run_test.sh @@ -63,7 +63,6 @@ res=$? ulimit -c unlimited if [ $res == 0 ] ; then -# . ${script_dir}/configure_backend.sh . ${script_dir}/set_env.sh $name cd ${script_dir}/.. cmake . -DBUILDNAME=$name -DCMAKE_BUILD_TYPE=Debug @@ -76,7 +75,7 @@ if [ $res == 0 ] ; then if [ $? != 0 ]; then echo "Backend broken!" if [ "${do_not_destroy_vm}" != "yes" ] ; then - ${script_dir}/destroy.sh + ${mdbci_dir}/mdbci destroy $name fi rm ~/vagrant_lock exit 1 @@ -90,13 +89,13 @@ if [ $res == 0 ] ; then else echo "Failed to create VMs, exiting" if [ "${do_not_destroy_vm}" != "yes" ] ; then - ${script_dir}/destroy.sh + ${mdbci_dir}/mdbci destroy $name fi rm ~/vagrant_lock exit 1 fi if [ "${do_not_destroy_vm}" != "yes" ] ; then - ${script_dir}/destroy.sh + ${mdbci_dir}/mdbci destroy $name echo "clean up done!" fi diff --git a/maxscale-system-test/mdbci/run_test_snapshot.sh b/maxscale-system-test/mdbci/run_test_snapshot.sh index aa7655320..fa3398f3f 100755 --- a/maxscale-system-test/mdbci/run_test_snapshot.sh +++ b/maxscale-system-test/mdbci/run_test_snapshot.sh @@ -48,15 +48,14 @@ export repo_dir=$dir/repo.d/ ${mdbci_dir}/mdbci snapshot revert --path-to-nodes $name --snapshot-name $snapshot_name if [ $? != 0 ]; then - ${script_dir}/destroy.sh + ${mdbci_dir}/mdbci destroy $name ${MDBCI_VM_PATH}/scripts/clean_vms.sh $name ${script_dir}/create_config.sh - checkExitStatus $? "Error creating configuration" $snapshot_lock_file - . ${script_dir}/configure_backend.sh + checkExitStatus $? "Error creating configuration" $snapshot_lock_file echo "Creating snapshot from new config" - $HOME/mdbci/mdbci snapshot take --path-to-nodes $name --snapshot-name $snapshot_name + ${mdbci_dir}/mdbci snapshot take --path-to-nodes $name --snapshot-name $snapshot_name fi . ${script_dir}/set_env.sh "$name" diff --git a/server/core/mysql_binlog.cc b/server/core/mysql_binlog.cc index 476460314..07392d812 100644 --- a/server/core/mysql_binlog.cc +++ b/server/core/mysql_binlog.cc @@ -262,47 +262,18 @@ static void unpack_datetime(uint8_t *ptr, int length, struct tm *dest) uint64_t val = 0; uint32_t second, minute, hour, day, month, year; - if (length == -1) - { - val = gw_mysql_get_byte8(ptr); - second = val - ((val / 100) * 100); - val /= 100; - minute = val - ((val / 100) * 100); - val /= 100; - hour = val - ((val / 100) * 100); - val /= 100; - day = val - ((val / 100) * 100); - val /= 100; - month = val - ((val / 100) * 100); - val /= 100; - year = val; - } - else - { - // TODO: Figure out why DATETIME(0) doesn't work like it others do - val = unpack_bytes(ptr, datetime_sizes[length]); - val *= log_10_values[6 - length]; - - if (val < 0) - { - val = -val; - } - - int subsecond = val % 1000000; - val /= 1000000; - - second = val % 60; - val /= 60; - minute = val % 60; - val /= 60; - hour = val % 24; - val /= 24; - day = val % 32; - val /= 32; - month = val % 13; - val /= 13; - year = val; - } + val = gw_mysql_get_byte8(ptr); + second = val - ((val / 100) * 100); + val /= 100; + minute = val - ((val / 100) * 100); + val /= 100; + hour = val - ((val / 100) * 100); + val /= 100; + day = val - ((val / 100) * 100); + val /= 100; + month = val - ((val / 100) * 100); + val /= 100; + year = val; memset(dest, 0, sizeof(struct tm)); dest->tm_year = year - 1900; @@ -502,7 +473,7 @@ static size_t temporal_field_size(uint8_t type, uint8_t decimals, int length) return 3 + ((decimals + 1) / 2); case TABLE_COL_TYPE_DATETIME: - return length < 0 || length > 6 ? 8 : datetime_sizes[length]; + return 8; case TABLE_COL_TYPE_TIMESTAMP: return 4; @@ -649,6 +620,7 @@ size_t unpack_numeric_field(uint8_t *src, uint8_t type, uint8_t *metadata, uint8 break; } + ss_dassert(size > 0); memcpy(dest, src, size); return size; } diff --git a/server/modules/authenticator/MySQLAuth/dbusers.c b/server/modules/authenticator/MySQLAuth/dbusers.c index 434e7422b..ecc55331f 100644 --- a/server/modules/authenticator/MySQLAuth/dbusers.c +++ b/server/modules/authenticator/MySQLAuth/dbusers.c @@ -44,11 +44,11 @@ #define NEW_LOAD_DBUSERS_QUERY "SELECT u.user, u.host, d.db, u.select_priv, u.%s \ FROM mysql.user AS u LEFT JOIN mysql.db AS d \ - ON (u.user = d.user AND u.host = d.host) %s \ + ON (u.user = d.user AND u.host = d.host) WHERE u.plugin = '' %s \ UNION \ SELECT u.user, u.host, t.db, u.select_priv, u.%s \ FROM mysql.user AS u LEFT JOIN mysql.tables_priv AS t \ - ON (u.user = t.user AND u.host = t.host) %s" + ON (u.user = t.user AND u.host = t.host) WHERE u.plugin = '' %s" static int get_users(SERV_LISTENER *listener, bool skip_local); static MYSQL *gw_mysql_init(void); @@ -59,7 +59,7 @@ static bool get_hostname(DCB *dcb, char *client_hostname, size_t size); static char* get_new_users_query(const char *server_version, bool include_root) { const char* password = strstr(server_version, "5.7.") ? MYSQL57_PASSWORD : MYSQL_PASSWORD; - const char *with_root = include_root ? "" : "WHERE u.user NOT IN ('root')"; + const char *with_root = include_root ? "" : " AND u.user NOT IN ('root')"; size_t n_bytes = snprintf(NULL, 0, NEW_LOAD_DBUSERS_QUERY, password, with_root, password, with_root); char *rval = MXS_MALLOC(n_bytes + 1); diff --git a/server/modules/routing/avrorouter/CMakeLists.txt b/server/modules/routing/avrorouter/CMakeLists.txt index c9be3ddfd..769df5fb9 100644 --- a/server/modules/routing/avrorouter/CMakeLists.txt +++ b/server/modules/routing/avrorouter/CMakeLists.txt @@ -6,6 +6,10 @@ if(AVRO_FOUND AND JANSSON_FOUND) set_target_properties(avrorouter PROPERTIES LINK_FLAGS -Wl,-z,defs) target_link_libraries(avrorouter maxscale-common ${JANSSON_LIBRARIES} ${AVRO_LIBRARIES} maxavro lzma) install_module(avrorouter core) + + if (BUILD_TESTS) + add_subdirectory(test) + endif() else() message(STATUS "No Avro C or Jansson libraries found, not building avrorouter.") endif() diff --git a/server/modules/routing/avrorouter/avro.c b/server/modules/routing/avrorouter/avro.c index a637ef6eb..a2c4ed5aa 100644 --- a/server/modules/routing/avrorouter/avro.c +++ b/server/modules/routing/avrorouter/avro.c @@ -60,7 +60,7 @@ static const char* avro_index_name = "avro.index"; static const char* create_table_regex = "(?i)create[a-z0-9[:space:]_]+table"; static const char* alter_table_regex = - "(?i)alter[[:space:]]+table.*column"; + "(?i)alter[[:space:]]+table"; /* The router entry points */ static MXS_ROUTER *createInstance(SERVICE *service, char **options); diff --git a/server/modules/routing/avrorouter/avro_file.c b/server/modules/routing/avrorouter/avro_file.c index f856cdd05..d37da88ce 100644 --- a/server/modules/routing/avrorouter/avro_file.c +++ b/server/modules/routing/avrorouter/avro_file.c @@ -1013,6 +1013,40 @@ void unify_whitespace(char *sql, int len) } } +/** + * A very simple function for stripping auto-generated executable comments + * + * Note that the string will not strip the trailing part of the comment, making + * the SQL invalid. + * + * @param sql String to modify + * @param len Pointer to current length of string, updated to new length if + * @c sql is modified + */ +static void strip_executable_comments(char *sql, int* len) +{ + if (strncmp(sql, "/*!", 3) == 0 || strncmp(sql, "/*M!", 4) == 0) + { + // Executable comment, remove it + char* p = sql + 3; + if (*p == '!') + { + p++; + } + + // Skip the versioning part + while (*p && isdigit(*p)) + { + p++; + } + + int n_extra = p - sql; + int new_len = *len - n_extra; + memmove(sql, sql + n_extra, new_len); + *len = new_len; + } +} + /** * @brief Handling of query events * @@ -1032,12 +1066,14 @@ void handle_query_event(AVRO_INSTANCE *router, REP_HEADER *hdr, int *pending_tra db[dblen] = 0; size_t sqlsz = len, tmpsz = len; - char *tmp = MXS_MALLOC(len); + char *tmp = MXS_MALLOC(len + 1); MXS_ABORT_IF_NULL(tmp); remove_mysql_comments((const char**)&sql, &sqlsz, &tmp, &tmpsz); sql = tmp; len = tmpsz; unify_whitespace(sql, len); + strip_executable_comments(sql, &len); + sql[len] = '\0'; static bool warn_not_row_format = true; @@ -1058,6 +1094,9 @@ void handle_query_event(AVRO_INSTANCE *router, REP_HEADER *hdr, int *pending_tra } } + char ident[MYSQL_TABLE_MAXLEN + MYSQL_DATABASE_MAXLEN + 2]; + read_table_identifier(db, sql, sql + len, ident, sizeof(ident)); + if (is_create_table_statement(router, sql, len)) { TABLE_CREATE *created = NULL; @@ -1077,7 +1116,7 @@ void handle_query_event(AVRO_INSTANCE *router, REP_HEADER *hdr, int *pending_tra } else { - created = table_create_alloc(sql, len, db); + created = table_create_alloc(ident, sql, len); } if (created && !save_and_replace_table_create(router, created)) @@ -1087,30 +1126,7 @@ void handle_query_event(AVRO_INSTANCE *router, REP_HEADER *hdr, int *pending_tra } else if (is_alter_table_statement(router, sql, len)) { - char ident[MYSQL_TABLE_MAXLEN + MYSQL_DATABASE_MAXLEN + 2]; - read_alter_identifier(sql, sql + len, ident, sizeof(ident)); - - bool combine = (strnlen(db, 1) && strchr(ident, '.') == NULL); - - size_t ident_len = strlen(ident) + 1; // + 1 for the NULL - - if (combine) - { - ident_len += (strlen(db) + 1); // + 1 for the "." - } - - char full_ident[ident_len]; - full_ident[0] = 0; // Set full_ident to "". - - if (combine) - { - strcat(full_ident, db); - strcat(full_ident, "."); - } - - strcat(full_ident, ident); - - TABLE_CREATE *created = hashtable_fetch(router->created_tables, full_ident); + TABLE_CREATE *created = hashtable_fetch(router->created_tables, ident); if (created) { @@ -1118,7 +1134,7 @@ void handle_query_event(AVRO_INSTANCE *router, REP_HEADER *hdr, int *pending_tra } else { - MXS_ERROR("Alter statement to a table with no create statement."); + MXS_ERROR("Alter statement to table '%s' has no preceding create statement.", ident); } } /* A transaction starts with this event */ diff --git a/server/modules/routing/avrorouter/avro_rbr.c b/server/modules/routing/avrorouter/avro_rbr.c index 7bd41b87a..8a55d3135 100644 --- a/server/modules/routing/avrorouter/avro_rbr.c +++ b/server/modules/routing/avrorouter/avro_rbr.c @@ -229,6 +229,7 @@ bool handle_row_event(AVRO_INSTANCE *router, REP_HEADER *hdr, uint8_t *ptr) { bool rval = false; uint8_t *start = ptr; + uint8_t *end = ptr + hdr->event_size - BINLOG_EVENT_HDR_LEN; uint8_t table_id_size = router->event_type_hdr_lens[hdr->event_type] == 6 ? 4 : 6; uint64_t table_id = 0; @@ -293,8 +294,9 @@ bool handle_row_event(AVRO_INSTANCE *router, REP_HEADER *hdr, uint8_t *ptr) snprintf(table_ident, sizeof(table_ident), "%s.%s", map->database, map->table); AVRO_TABLE* table = hashtable_fetch(router->open_tables, table_ident); TABLE_CREATE* create = map->table_create; + ss_dassert(hashtable_fetch(router->created_tables, table_ident) == create); - if (table && create && ncolumns == map->columns) + if (table && create && ncolumns == map->columns && create->columns == map->columns) { avro_value_t record; avro_generic_value_new(table->avro_writer_iface, &record); @@ -305,13 +307,12 @@ bool handle_row_event(AVRO_INSTANCE *router, REP_HEADER *hdr, uint8_t *ptr) int rows = 0; MXS_INFO("Row Event for '%s' at %lu", table_ident, router->current_pos); - while (ptr - start < hdr->event_size - BINLOG_EVENT_HDR_LEN) + while (ptr < end) { static uint64_t total_row_count = 1; MXS_INFO("Row %lu", total_row_count++); /** Add the current GTID and timestamp */ - uint8_t *end = ptr + hdr->event_size - BINLOG_EVENT_HDR_LEN; int event_type = get_event_type(hdr->event_type); prepare_record(router, hdr, event_type, &record); ptr = process_row_event_data(map, create, &record, ptr, col_present, end); @@ -353,6 +354,12 @@ bool handle_row_event(AVRO_INSTANCE *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) + { + MXS_ERROR("Table map event has a different column count for table " + "%s.%s than the CREATE TABLE statement. Possible " + "unsupported DDL detected.", map->database, map->table); + } else { MXS_ERROR("Row event and table map event have different column " @@ -526,7 +533,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value { int npresent = 0; avro_value_t field; - long ncolumns = MXS_MIN(map->columns, create->columns); + long ncolumns = map->columns; uint8_t *metadata = map->column_metadata; size_t metadata_offset = 0; @@ -576,7 +583,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value avro_value_set_string(&field, strval); sprintf(trace[i], "[%ld] ENUM: %lu bytes", i, bytes); ptr += bytes; - check_overflow(ptr < end); + check_overflow(ptr <= end); } else { @@ -612,7 +619,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value str[bytes] = '\0'; avro_value_set_string(&field, str); ptr += bytes; - check_overflow(ptr < end); + check_overflow(ptr <= end); } } else if (column_is_bit(map->column_types[i])) @@ -631,7 +638,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value avro_value_set_int(&field, value); sprintf(trace[i], "[%ld] BIT", i); ptr += bytes; - check_overflow(ptr < end); + check_overflow(ptr <= end); } else if (column_is_decimal(map->column_types[i])) { @@ -639,7 +646,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value ptr += unpack_decimal_field(ptr, metadata + metadata_offset, &f_value); avro_value_set_double(&field, f_value); sprintf(trace[i], "[%ld] DECIMAL", i); - check_overflow(ptr < end); + check_overflow(ptr <= end); } else if (column_is_variable_string(map->column_types[i])) { @@ -662,7 +669,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value buf[sz] = '\0'; ptr += sz; avro_value_set_string(&field, buf); - check_overflow(ptr < end); + check_overflow(ptr <= end); } else if (column_is_blob(map->column_types[i])) { @@ -681,7 +688,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value uint8_t nullvalue = 0; avro_value_set_bytes(&field, &nullvalue, 1); } - check_overflow(ptr < end); + check_overflow(ptr <= end); } else if (column_is_temporal(map->column_types[i])) { @@ -693,7 +700,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value 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); - check_overflow(ptr < end); + check_overflow(ptr <= end); } /** All numeric types (INT, LONG, FLOAT etc.) */ else @@ -704,7 +711,7 @@ uint8_t* process_row_event_data(TABLE_MAP *map, TABLE_CREATE *create, avro_value &metadata[metadata_offset], lval); set_numeric_field_value(&field, map->column_types[i], &metadata[metadata_offset], lval); sprintf(trace[i], "[%ld] %s", i, column_type_to_string(map->column_types[i])); - check_overflow(ptr < end); + check_overflow(ptr <= end); } ss_dassert(metadata_offset <= map->column_metadata_size); metadata_offset += get_metadata_len(map->column_types[i]); diff --git a/server/modules/routing/avrorouter/avro_schema.c b/server/modules/routing/avrorouter/avro_schema.c index e76301838..5ec5e4595 100644 --- a/server/modules/routing/avrorouter/avro_schema.c +++ b/server/modules/routing/avrorouter/avro_schema.c @@ -720,46 +720,28 @@ int resolve_table_version(const char* db, const char* table) /** * @brief Handle a query event which contains a CREATE TABLE statement - * @param sql Query SQL - * @param db Database where this query was executed + * + * @param ident Table identifier in database.table format + * @param sql The CREATE TABLE statement + * @param len Length of @c sql + * * @return New CREATE_TABLE object or NULL if an error occurred */ -TABLE_CREATE* table_create_alloc(const char* sql, int len, const char* db) +TABLE_CREATE* table_create_alloc(const char* ident, const char* sql, int len) { /** Extract the table definition so we can get the column names from it */ int stmt_len = 0; const char* statement_sql = get_table_definition(sql, len, &stmt_len); ss_dassert(statement_sql); + + char* tbl_start = strchr(ident, '.'); + ss_dassert(tbl_start); + *tbl_start++ = '\0'; + char table[MYSQL_TABLE_MAXLEN + 1]; char database[MYSQL_DATABASE_MAXLEN + 1]; - const char* err = NULL; - MXS_INFO("Create table: %.*s", len, sql); - - if (!statement_sql) - { - err = "table definition"; - } - else if (!get_table_name(sql, table)) - { - err = "table name"; - } - - if (get_database_name(sql, database)) - { - // The CREATE statement contains the database name - db = database; - } - else if (*db == '\0') - { - // No explicit or current database - err = "database name"; - } - - if (err) - { - MXS_ERROR("Malformed CREATE TABLE statement, could not extract %s: %.*s", err, len, sql); - return NULL; - } + strcpy(database, ident); + strcpy(table, tbl_start); int* lengths = NULL; char **names = NULL; @@ -773,13 +755,13 @@ TABLE_CREATE* table_create_alloc(const char* sql, int len, const char* db) { if ((rval = MXS_MALLOC(sizeof(TABLE_CREATE)))) { - rval->version = resolve_table_version(db, table); + 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(db); + rval->database = MXS_STRDUP(database); rval->table = MXS_STRDUP(table); } @@ -930,7 +912,6 @@ static void remove_extras(char* str) ss_dassert(strlen(str) == len); } - static void remove_backticks(char* src) { char* dest = src; @@ -1134,6 +1115,110 @@ static const char* get_tok(const char* sql, int* toklen, const char* end) return NULL; } +static void rskip_whitespace(const char* sql, const char** end) +{ + const char* ptr = *end; + + while (ptr > sql && isspace(*ptr)) + { + ptr--; + } + + *end = ptr; +} + +static void rskip_token(const char* sql, const char** end) +{ + const char* ptr = *end; + + while (ptr > sql && !isspace(*ptr)) + { + ptr--; + } + + *end = ptr; +} + +static bool get_placement_specifier(const char* sql, const char* end, const char** tgt, int* tgt_len) +{ + bool rval = false; + ss_dassert(end > sql); + end--; + + *tgt = NULL; + *tgt_len = 0; + + // Skip any trailing whitespace + rskip_whitespace(sql, &end); + + if (*end == '`') + { + // Identifier, possibly AFTER `column` + const char* id_end = end; + end--; + + while (end > sql && *end != '`') + { + end--; + } + + const char* id_start = end + 1; + ss_dassert(*end == '`' && *id_end == '`'); + + end--; + + rskip_whitespace(sql, &end); + rskip_token(sql, &end); + + // end points to the character _before_ the token + end++; + + if (strncasecmp(end, "AFTER", 5) == 0) + { + // This column comes after the specified column + rval = true; + *tgt = id_start; + *tgt_len = id_end - id_start; + } + } + else + { + // Something else, possibly FIRST or un-backtick'd AFTER + const char* id_end = end + 1; // Points to either a trailing space or one-after-the-end + rskip_token(sql, &end); + + // end points to the character _before_ the token + end++; + + if (strncasecmp(end, "FIRST", 5) == 0) + { + // Put this column first + rval = true; + } + else + { + const char* id_start = end + 1; + + // Skip the whitespace and until the start of the current token + rskip_whitespace(sql, &end); + rskip_token(sql, &end); + + // end points to the character _before_ the token + end++; + + if (strncasecmp(end, "AFTER", 5) == 0) + { + // This column comes after the specified column + rval = true; + *tgt = id_start; + *tgt_len = id_end - id_start; + } + } + } + + return rval; +} + static bool tok_eq(const char *a, const char *b, size_t len) { size_t i = 0; @@ -1150,15 +1235,130 @@ static bool tok_eq(const char *a, const char *b, size_t len) return true; } -void read_alter_identifier(const char *sql, const char *end, char *dest, int size) +static void skip_whitespace(const char** saved) { - int len = 0; - const char *tok = get_tok(sql, &len, end); // ALTER - if (tok && (tok = get_tok(tok + len, &len, end)) // TABLE - && (tok = get_tok(tok + len, &len, end))) // Table identifier + const char* ptr = *saved; + + while (*ptr && isspace(*ptr)) { - snprintf(dest, size, "%.*s", len, tok); - remove_backticks(dest); + ptr++; + } + + *saved = ptr; +} + +static void skip_token(const char** saved) +{ + const char* ptr = *saved; + + while (*ptr && !isspace(*ptr)) + { + ptr++; + } + + *saved = ptr; +} + +static void skip_non_backtick(const char** saved) +{ + const char* ptr = *saved; + + while (*ptr && *ptr != '`') + { + ptr++; + } + + *saved = ptr; +} + +const char* keywords[] = +{ + "CREATE", + "DROP", + "ALTER", + "IF", + "EXISTS", + "REPLACE", + "OR", + "TABLE", + "NOT", + NULL +}; + +static bool token_is_keyword(const char* tok, int len) +{ + for (int i = 0; keywords[i]; i++) + { + if (strncasecmp(keywords[i], tok, len) == 0) + { + return true; + } + } + + return false; +} + +void read_table_identifier(const char* db, const char *sql, const char *end, char *dest, int size) +{ + const char* start; + int len = 0; + bool is_keyword = true; + + while (is_keyword) + { + skip_whitespace(&sql); // Leading whitespace + + if (*sql == '`') + { + // Quoted identifier, not a keyword + is_keyword = false; + sql++; + start = sql; + skip_non_backtick(&sql); + len = sql - start; + sql++; + } + else + { + start = sql; + skip_token(&sql); + len = sql - start; + is_keyword = token_is_keyword(start, len); + } + } + + skip_whitespace(&sql); // Space after first identifier + + if (*sql != '.') + { + // No explicit database + snprintf(dest, size, "%s.%.*s", db, len, start); + } + else + { + // Explicit database, skip the period + sql++; + skip_whitespace(&sql); // Space after first identifier + + const char* id_start; + int id_len = 0; + + if (*sql == '`') + { + sql++; + id_start = sql; + skip_non_backtick(&sql); + id_len = sql - id_start; + sql++; + } + else + { + id_start = sql; + skip_token(&sql); + id_len = sql - id_start; + } + + snprintf(dest, size, "%.*s.%.*s", len, start, id_len, id_start); } } @@ -1192,6 +1392,14 @@ int get_column_index(TABLE_CREATE *create, const char *tok, int len) 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 (int x = 0; x < create->columns; x++) @@ -1206,6 +1414,35 @@ int get_column_index(TABLE_CREATE *create, const char *tok, int len) return idx; } +static bool not_column_operation(const char* tok, int len) +{ + const char* keywords[] = + { + "PRIMARY", + "UNIQUE", + "FULLTEXT", + "SPATIAL", + "PERIOD", + "PRIMARY", + "KEY", + "KEYS", + "INDEX", + "FOREIGN", + "CONSTRAINT", + NULL + }; + + for (int i = 0; keywords[i]; i++) + { + if (tok_eq(tok, keywords[i], strlen(keywords[i]))) + { + return true; + } + } + + return false; +} + bool table_create_alter(TABLE_CREATE *create, const char *sql, const char *end) { const char *tbl = strcasestr(sql, "table"), *def; @@ -1231,9 +1468,19 @@ bool table_create_alter(TABLE_CREATE *create, const char *sql, const char *end) if (tok) { - if (tok_eq(ptok, "add", plen) && tok_eq(tok, "column", len)) + if (not_column_operation(tok, len)) { + MXS_INFO("Statement doesn't affect columns, not processing: %s", sql); + return true; + } + else if (tok_eq(tok, "column", len)) + { + // Skip the optional COLUMN keyword 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); bool is_new = true; @@ -1264,10 +1511,8 @@ bool table_create_alter(TABLE_CREATE *create, const char *sql, const char *end) tok = get_next_def(tok, end); len = 0; } - else if (tok_eq(ptok, "drop", plen) && tok_eq(tok, "column", len)) + else if (tok_eq(ptok, "drop", plen)) { - tok = get_tok(tok + len, &len, end); - int idx = get_column_index(create, tok, len); if (idx != -1) @@ -1291,10 +1536,8 @@ bool table_create_alter(TABLE_CREATE *create, const char *sql, const char *end) tok = get_next_def(tok, end); len = 0; } - else if (tok_eq(ptok, "change", plen) && tok_eq(tok, "column", len)) + else if (tok_eq(ptok, "change", plen)) { - tok = get_tok(tok + len, &len, end); - int idx = get_column_index(create, tok, len); if (idx != -1 && (tok = get_tok(tok + len, &len, end))) diff --git a/server/modules/routing/avrorouter/avrorouter.h b/server/modules/routing/avrorouter/avrorouter.h index 27818068c..67029e1c5 100644 --- a/server/modules/routing/avrorouter/avrorouter.h +++ b/server/modules/routing/avrorouter/avrorouter.h @@ -310,12 +310,12 @@ extern void read_table_info(uint8_t *ptr, uint8_t post_header_len, uint64_t *tab char* dest, size_t len); extern TABLE_MAP *table_map_alloc(uint8_t *ptr, uint8_t hdr_len, TABLE_CREATE* create); extern void table_map_free(TABLE_MAP *map); -extern TABLE_CREATE* table_create_alloc(const char* sql, int len, const char* db); +extern TABLE_CREATE* table_create_alloc(const char* ident, const char* sql, int len); extern TABLE_CREATE* table_create_copy(AVRO_INSTANCE *router, const char* sql, size_t len, const char* db); extern void table_create_free(TABLE_CREATE* value); extern bool table_create_save(TABLE_CREATE *create, const char *filename); extern bool table_create_alter(TABLE_CREATE *create, const char *sql, const char *end); -extern void read_alter_identifier(const char *sql, const char *end, char *dest, int size); +extern void read_table_identifier(const char* db, const char *sql, const char *end, char *dest, int size); extern int avro_client_handle_request(AVRO_INSTANCE *, AVRO_CLIENT *, GWBUF *); extern void avro_client_rotate(AVRO_INSTANCE *router, AVRO_CLIENT *client, uint8_t *ptr); extern bool avro_open_binlog(const char *binlogdir, const char *file, int *fd); diff --git a/server/modules/routing/avrorouter/test/CMakeLists.txt b/server/modules/routing/avrorouter/test/CMakeLists.txt new file mode 100644 index 000000000..016461f38 --- /dev/null +++ b/server/modules/routing/avrorouter/test/CMakeLists.txt @@ -0,0 +1,3 @@ +add_executable(test_alter_parsing test_alter_parsing.c) +target_link_libraries(test_alter_parsing maxscale-common ${JANSSON_LIBRARIES} ${AVRO_LIBRARIES} maxavro sqlite3 lzma) +add_test(test_alter_parsing test_alter_parsing) \ No newline at end of file diff --git a/server/modules/routing/avrorouter/test/test_alter_parsing.c b/server/modules/routing/avrorouter/test/test_alter_parsing.c new file mode 100644 index 000000000..c622d6e75 --- /dev/null +++ b/server/modules/routing/avrorouter/test/test_alter_parsing.c @@ -0,0 +1,99 @@ +#include "../avro_schema.c" + +static struct +{ + const char* statement; + const char* target; + bool rval; +} data[] = +{ + {"/*!40000 ALTER TABLE `t1` DISABLE KEYS */", NULL, false}, + {"/*!40000 ALTER TABLE `t1` ENABLE KEYS */", NULL, false}, + {"ADD COLUMN `a` INT", NULL, false}, + {"ADD COLUMN `a`", NULL, false}, + {"ALTER TABLE `t1` ADD `account_id` INT", NULL, false}, + {"ALTER TABLE `t1` ADD `amount` INT", NULL, false}, + {"ALTER TABLE `t1` ADD `app_id` VARCHAR(64)", NULL, false}, + {"ALTER TABLE `t1` ADD `create_time` DATETIME", NULL, false}, + {"alter TABLE t1 add `end_time` varchar(10) DEFAULT NULL COMMENT 'this is a comment'", NULL, false}, + {"ALTER TABLE `t1` ADD `expire_time` DATETIME", NULL, false}, + {"ALTER TABLE `t1` ADD `id_a` VARCHAR(128)", NULL, false}, + {"ALTER TABLE `t1` ADD `id` BIGINT(20)", NULL, false}, + {"ALTER TABLE `t1` ADD `id` VARCHAR(64)", NULL, false}, + {"ALTER TABLE `t1` ADD `node_state` INT(4)", NULL, false}, + {"ALTER TABLE `t1` ADD `no` INT", NULL, false}, + {"ALTER TABLE `t1` ADD `order_id` INT", NULL, false}, + {"alter TABLE t1 add `start_time` varchar(10) DEFAULT NULL COMMENT 'this is a comment'", NULL, false}, + {"ALTER TABLE `t1` ADD `status` INT", NULL, false}, + {"ALTER TABLE `t1` ADD `task_id` BIGINT(20)", NULL, false}, + {"alter TABLE t1 add `undo` int(1) DEFAULT '0' COMMENT 'this is a comment'", NULL, false}, + {"alter table `t1` add unique (`a`,`id`)", NULL, false}, + {"alter table `t1` add unique (`a`)", NULL, false}, + {"alter table `t1` add UNIQUE(`a`)", NULL, false}, + {"ALTER TABLE `t1` ADD UNIQUE `idx_id` USING BTREE (`id`, `result`)", NULL, false}, + {"ALTER TABLE `t1` ADD `update_time` INT", NULL, false}, + {"ALTER TABLE `t1` ADD `username` VARCHAR(16)", NULL, false}, + {"ALTER TABLE `t1` AUTO_INCREMENT = 1", NULL, false}, + {"ALTER TABLE `t1` CHANGE `account_id` `account_id` BIGINT(20)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `amount` `amount` DECIMAL(32,2)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `app_id` `app_id` VARCHAR(64)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `business_id` `business_id` VARCHAR(128)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `business_id` `business_id` VARCHAR(64)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `business_unique_no` `business_unique_no` VARCHAR(64)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `expire_time` `expire_time` DATETIME", NULL, false}, + {"ALTER TABLE `t1` CHANGE `id_a` `id_a` VARCHAR(128)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `id` `id` BIGINT(20)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `node_state` `node_state` INT(4)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `order_id` `order_id` BIGINT(20)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `status` `status` INT(1)", NULL, false}, + {"ALTER TABLE `t1` CHANGE `update_time` `update_time` TIMESTAMP", NULL, false}, + {"ALTER TABLE `t1` CHANGE `username` `username` VARCHAR(16)", NULL, false}, + {"ALTER TABLE `t1` COMMENT = 'a comment'", NULL, false}, + {"alter table `t1` drop index a", NULL, false}, + {"alter table t1 drop index t1_idx", NULL, false}, + {"alter table t1 index(account_id, business_id)", NULL, false}, + {"ALTER TABLE `t1` MODIFY COLUMN `expire_time` DATETIME DEFAULT NULL COMMENT 'this is a comment' AFTER `update_time`", "update_time", true}, + {"ALTER TABLE `t1` MODIFY COLUMN `id_a` VARCHAR(128) CHARACTER SET utf8 COLLATE utf8_general_ci COMMENT 'this is a comment' AFTER `username`", "username", true}, + {"ALTER TABLE `t1` MODIFY COLUMN `number` VARCHAR(64) CHARACTER SET utf8 COLLATE utf8_general_ci DEFAULT NULL COMMENT 'this is a comment' AFTER `business_id`", "business_id", true}, + {"ALTER TABLE `t1` MODIFY COLUMN `task_id` BIGINT(20) DEFAULT NULL COMMENT 'this is a comment' AFTER `business_id`", "business_id", true}, + {"ALTER TABLE `t1` MODIFY COLUMN `username` VARCHAR(16) CHARACTER SET utf8 COLLATE utf8_general_ci NOT NULL COMMENT 'this is a comment' AFTER `business_id`", "business_id", true}, + {"ALTER TABLE `t1` RENAME `t2`", NULL, false}, + {"ALTER TABLE `db1`.`t1` ADD COLUMN `num` varchar(32) COMMENT 'this is a comment' AFTER `bank_name`", "bank_name", true}, + {"ALTER TABLE `db1`.`t1` ADD INDEX `idx_node_state` USING BTREE (`node_state`) comment ''", NULL, false}, + {"ALTER TABLE `db1`.`t1` CHANGE COLUMN `num` `code` varchar(32) DEFAULT NULL COMMENT 'this is a comment'", NULL, false}, + {"ALTER TABLE `db1`.`t1` DROP INDEX `a`, ADD INDEX `a` USING BTREE (`a`) comment ''", NULL, false}, + {"ALTER TABLE `db1`.`t1` DROP INDEX `a`, ADD INDEX `idx_a` USING BTREE (`a`) comment ''", NULL, false}, + {"ALTER TABLE `t1` CHANGE COLUMN `a` `c` INT AFTER `b`", "b", true}, + {"ALTER TABLE `t1` CHANGE COLUMN `a` `c` INT first", NULL, true}, + {"ALTER TABLE `t1` CHANGE COLUMN `a` `c` INT", NULL, false}, + {"ALTER TABLE `t1` MODIFY COLUMN `a` INT PRIMARY KEY", NULL, false}, + {NULL} +}; + +int main(int argc, char** argv) +{ + int rval = 0; + + for (int i = 0; data[i].statement; i++) + { + const char* target = NULL; + int len = 0; + const char* stmt = data[i].statement; + const char* end = data[i].statement + strlen(data[i].statement); + + if (get_placement_specifier(stmt, end, &target, &len) != data[i].rval) + { + const char* a = data[i].rval ? "true" : "false"; + const char* b = data[i].rval ? "false" : "true"; + printf("Expected '%s', got '%s' for '%s'\n", a, b, data[i].statement); + rval++; + } + else if (((bool)data[i].target != (bool)target) || (strncmp(target, data[i].target, len) != 0)) + { + printf("Expected '%s', got '%.*s' for '%s'\n", data[i].target, len, target, data[i].statement); + rval++; + } + } + + return rval; +} diff --git a/server/modules/routing/binlogrouter/blr_file.c b/server/modules/routing/binlogrouter/blr_file.c index d40b99a85..41b707aa7 100644 --- a/server/modules/routing/binlogrouter/blr_file.c +++ b/server/modules/routing/binlogrouter/blr_file.c @@ -521,9 +521,12 @@ blr_file_create(ROUTER_INSTANCE *router, char *orig_file) { close(router->binlog_fd); spinlock_acquire(&router->binlog_lock); - int len = strlen(file); - memmove(router->binlog_name, file, len); - router->binlog_name[len] = '\0'; + + /// Use an intermediate buffer in case the source and destination overlap + char new_binlog[strlen(file) + 1]; + strcpy(new_binlog, file); + strcpy(router->binlog_name, new_binlog); + router->binlog_fd = fd; /* Initial position after the magic number */ router->current_pos = BINLOG_MAGIC_SIZE;