From 00ed7cc0bb54dd9cb405562496c3935f5ba396b7 Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 23 Nov 2007 17:51:12 +0400 Subject: BUG#31833 - ORDER BY leads to wrong result when ARCHIVE, BLOB and table cache is full After reading last record from freshly opened archive table (e.g. after flush table, or if there is no room in table cache), the table is reported as crashed. The problem was that azio wrongly invalidated azio_stream when it meets EOF. mysql-test/r/archive.result: A test case for BUG#31833. mysql-test/t/archive.test: A test case for BUG#31833. storage/archive/azio.c: After azread() successfuly read and inflated data, it calls check_header() function. According to the comment it is done to detect concatenated .az files. When we read last record, there are no more bytes left at the current offset, all further my_read() calls will return 0. In this case check_header() wrongly sets s->z_err to Z_ERRNO, indicating that azio_stream is broken. Following is original condition from gzio: len = (uInt)fread(s->inbuf + len, 1, Z_BUFSIZE >> len, s->file); if (len == 0 && ferror(s->file)) s->z_err = Z_ERRNO; As fread() returns 0 on both EOF and error, the condition states: Invalidate gzio_stream if we got an error from last fread(). Applied the same logic to azio. Note that a test case contains FLUSH TABLE t1 prior to SELECT. It is needed because azio doesn't flush buffers immediately. Thus we may azread() last record from in-memory buffer. When we read from in-memory buffer, EOF is detected by different branch of code in azread() and we never enter check_header() in this case. --- storage/archive/azio.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'storage') diff --git a/storage/archive/azio.c b/storage/archive/azio.c index 2cf0fe114d3..cada6c57918 100644 --- a/storage/archive/azio.c +++ b/storage/archive/azio.c @@ -262,7 +262,7 @@ void check_header(azio_stream *s) if (len) s->inbuf[0] = s->stream.next_in[0]; errno = 0; len = (uInt)my_read(s->file, (uchar *)s->inbuf + len, AZ_BUFSIZE_READ >> len, MYF(0)); - if (len == 0) s->z_err = Z_ERRNO; + if (len == (uInt)-1) s->z_err = Z_ERRNO; s->stream.avail_in += len; s->stream.next_in = s->inbuf; if (s->stream.avail_in < 2) { -- cgit v1.2.1 From 36c4fc22b2dc6b6e949628e25f15a866f2e0794d Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 27 Nov 2007 14:01:11 +0400 Subject: BUG#32050 - table logging gone wrong. INSERT/UPDATE against CSV table created by MySQL earlier than 5.1.23 with NULL-able column results in server crash in debug builds. Starting with 5.1.23 CSV doesn't permit creation of NULL-able columns, but it is still possible to get such table from older MySQL versions. Fixed by removing excessive DBUG_ASSERT(). scripts/mysql_system_tables_fix.sql: Starting with 5.1.23 CSV doesn't permit creation of NULL-able columns. Alter system CSV tables structure so that all columns are NOT NULL. storage/csv/ha_tina.cc: Starting with 5.1.23 CSV doesn't permit creation of NULL-able columns, but it is still possible to get such table from older MySQL versions. Removed excessive DBUG_ASSERT(). --- storage/csv/ha_tina.cc | 8 -------- 1 file changed, 8 deletions(-) (limited to 'storage') diff --git a/storage/csv/ha_tina.cc b/storage/csv/ha_tina.cc index 6a87b8ecc72..75c3f70dec4 100644 --- a/storage/csv/ha_tina.cc +++ b/storage/csv/ha_tina.cc @@ -472,14 +472,6 @@ int ha_tina::encode_quote(uchar *buf) const char *ptr; const char *end_ptr; const bool was_null= (*field)->is_null(); - - /* - CSV does not support nulls. ::create() prevents creation of a table - with nullable columns so if we encounter them here, there is a bug. - This may only occur if the frm was created by an older version of - mysqld which permitted table creation with nullable columns. - */ - DBUG_ASSERT(!(*field)->maybe_null()); /* assistance for backwards compatibility in production builds. -- cgit v1.2.1 From 874d13af25919b3418a65fddd08ad73b8c1972fc Mon Sep 17 00:00:00 2001 From: unknown Date: Fri, 7 Dec 2007 14:44:03 +0400 Subject: BUG#32817 - though CSV is marked as supported create table is rejected with error 1005. CSV doesn't support nullable fields. Report descriptive error if create table with nullable field is requested. mysql-test/r/csv.result: A test case for BUG#32817. mysql-test/t/csv.test: A test case for BUG#32817. storage/csv/ha_tina.cc: CSV doesn't support nullable fields. Report descriptive error if create table with nullable field is requested. --- storage/csv/ha_tina.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'storage') diff --git a/storage/csv/ha_tina.cc b/storage/csv/ha_tina.cc index 75c3f70dec4..b5ef4620108 100644 --- a/storage/csv/ha_tina.cc +++ b/storage/csv/ha_tina.cc @@ -1486,7 +1486,10 @@ int ha_tina::create(const char *name, TABLE *table_arg, for (Field **field= table_arg->s->field; *field; field++) { if ((*field)->real_maybe_null()) - DBUG_RETURN(-1); + { + my_error(ER_CHECK_NOT_IMPLEMENTED, MYF(0), "nullable columns"); + DBUG_RETURN(HA_ERR_UNSUPPORTED); + } } -- cgit v1.2.1 From 0899a87eef91702e4a6360b5b99856fa16ecb29d Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 11 Dec 2007 15:32:10 +0100 Subject: Bug#30273 - merge tables: Can't lock file (errno: 155) The patch for Bug 26379 (Combination of FLUSH TABLE and REPAIR TABLE corrupts a MERGE table) fixed this bug too. However it revealed a new bug that crashed the server. Flushing a merge table at the moment when it is between open and attach of children crashed the server. The flushing thread wants to abort locks on the flushed table. It calls ha_myisammrg::lock_count() and ha_myisammrg::store_lock() on the TABLE object of the other thread. Changed ha_myisammrg::lock_count() and ha_myisammrg::store_lock() to accept non-attached children. ha_myisammrg::lock_count() returns the number of MyISAM tables in the MERGE table so that the memory allocation done by get_lock_data() is done correctly, even if the children become attached before ha_myisammrg::store_lock() is called. ha_myisammrg::store_lock() will not return any lock if the children are not attached. This is however a change in the handler interface. lock_count() can now return a higher number than store_lock() stores locks. This is more safe than the reverse implementation would be. get_lock_data() in the SQL layer is adjusted accordingly. It sets MYSQL_LOCK::lock_count based on the number of locks returned by the handler::store_lock() calls, not based on the numbers returned by the handler::lock_count() calls. The latter are only used for allocation of memory now. No test case. The test suite cannot reliably run FLUSH between lock_count() and store_lock() of another thread. The bug report contains a program that can repeat the problem with some probability. include/myisammrg.h: Bug#30273 - merge tables: Can't lock file (errno: 155) Added mutex to struct st_myrg_info (MYRG_INFO). sql/handler.h: Bug#30273 - merge tables: Can't lock file (errno: 155) Extended comments for handler::lock_count() and handler::store_lock(). sql/lock.cc: Bug#30273 - merge tables: Can't lock file (errno: 155) Changed get_lock_data() so that the final lock_count is taken from the number of locks returned from handler::store_lock() instead of from handler::lock_count(). sql/sql_base.cc: Fixed a purecov comment. (unrelated to the rest of the changeset) storage/myisammrg/ha_myisammrg.cc: Bug#30273 - merge tables: Can't lock file (errno: 155) Changed ha_myisammrg::lock_count() and ha_myisammrg::store_lock() to accept non-attached children. Protected ha_myisammrg::store_lock() by MYRG_INFO::mutex. storage/myisammrg/myrg_close.c: Bug#30273 - merge tables: Can't lock file (errno: 155) Added MYRG_INFO::mutex destruction to myrg_parent_close(). storage/myisammrg/myrg_open.c: Bug#30273 - merge tables: Can't lock file (errno: 155) Added MYRG_INFO::mutex initialization to myrg_parent_open(). Protected myrg_attach_children() and myrg_detach_children() by MYRG_INFO::mutex. Fixed a purecov comment. (unrelated to the rest of the changeset) --- storage/myisammrg/ha_myisammrg.cc | 31 +++++++++++++++++++++++++++++-- storage/myisammrg/myrg_close.c | 1 + storage/myisammrg/myrg_open.c | 17 ++++++++++++++++- 3 files changed, 46 insertions(+), 3 deletions(-) (limited to 'storage') diff --git a/storage/myisammrg/ha_myisammrg.cc b/storage/myisammrg/ha_myisammrg.cc index f91b0dc7a92..4d3d8362a03 100644 --- a/storage/myisammrg/ha_myisammrg.cc +++ b/storage/myisammrg/ha_myisammrg.cc @@ -901,7 +901,14 @@ int ha_myisammrg::external_lock(THD *thd, int lock_type) uint ha_myisammrg::lock_count(void) const { - DBUG_ASSERT(this->file->children_attached); + /* + Return the real lock count even if the children are not attached. + This method is used for allocating memory. If we would return 0 + to another thread (e.g. doing FLUSH TABLE), and attach the children + before the other thread calls store_lock(), then we would return + more locks in store_lock() than we claimed by lock_count(). The + other tread would overrun its memory. + */ return file->tables; } @@ -911,7 +918,24 @@ THR_LOCK_DATA **ha_myisammrg::store_lock(THD *thd, enum thr_lock_type lock_type) { MYRG_TABLE *open_table; - DBUG_ASSERT(this->file->children_attached); + + /* + This method can be called while another thread is attaching the + children. If the processor reorders instructions or write to memory, + 'children_attached' could be set before 'open_tables' has all the + pointers to the children. Use of a mutex here and in + myrg_attach_children() forces consistent data. + */ + pthread_mutex_lock(&this->file->mutex); + + /* + When MERGE table is open, but not yet attached, other threads + could flush it, which means call mysql_lock_abort_for_thread() + on this threads TABLE. 'children_attached' is FALSE in this + situaton. Since the table is not locked, return no lock data. + */ + if (!this->file->children_attached) + goto end; /* purecov: tested */ for (open_table=file->open_tables ; open_table != file->end_table ; @@ -921,6 +945,9 @@ THR_LOCK_DATA **ha_myisammrg::store_lock(THD *thd, if (lock_type != TL_IGNORE && open_table->table->lock.type == TL_UNLOCK) open_table->table->lock.type=lock_type; } + + end: + pthread_mutex_unlock(&this->file->mutex); return to; } diff --git a/storage/myisammrg/myrg_close.c b/storage/myisammrg/myrg_close.c index b402a35a253..97216ed47fe 100644 --- a/storage/myisammrg/myrg_close.c +++ b/storage/myisammrg/myrg_close.c @@ -58,6 +58,7 @@ int myrg_close(MYRG_INFO *info) pthread_mutex_lock(&THR_LOCK_open); myrg_open_list=list_delete(myrg_open_list,&info->open_list); pthread_mutex_unlock(&THR_LOCK_open); + VOID(pthread_mutex_destroy(&info->mutex)); my_free((uchar*) info,MYF(0)); if (error) { diff --git a/storage/myisammrg/myrg_open.c b/storage/myisammrg/myrg_open.c index 6fb1888f84d..b5002116164 100644 --- a/storage/myisammrg/myrg_open.c +++ b/storage/myisammrg/myrg_open.c @@ -33,7 +33,7 @@ myrg_attach_children(). Please duplicate changes in these functions or make common sub-functions. */ -/* purecov: begin unused */ +/* purecov: begin deadcode */ /* not used in MySQL server */ MYRG_INFO *myrg_open(const char *name, int mode, int handle_locking) { @@ -171,6 +171,7 @@ MYRG_INFO *myrg_open(const char *name, int mode, int handle_locking) VOID(my_close(fd,MYF(0))); end_io_cache(&file); + VOID(pthread_mutex_init(&m_info->mutex, MY_MUTEX_INIT_FAST)); m_info->open_list.data=(void*) m_info; pthread_mutex_lock(&THR_LOCK_open); myrg_open_list=list_add(myrg_open_list,&m_info->open_list); @@ -328,6 +329,7 @@ MYRG_INFO *myrg_parent_open(const char *parent_name, end_io_cache(&file_cache); VOID(my_close(fd, MYF(0))); + VOID(pthread_mutex_init(&m_info->mutex, MY_MUTEX_INIT_FAST)); m_info->open_list.data= (void*) m_info; pthread_mutex_lock(&THR_LOCK_open); @@ -393,6 +395,14 @@ int myrg_attach_children(MYRG_INFO *m_info, int handle_locking, DBUG_ENTER("myrg_attach_children"); DBUG_PRINT("myrg", ("handle_locking: %d", handle_locking)); + /* + This function can be called while another thread is trying to abort + locks of this MERGE table. If the processor reorders instructions or + write to memory, 'children_attached' could be set before + 'open_tables' has all the pointers to the children. Use of a mutex + here and in ha_myisammrg::store_lock() forces consistent data. + */ + pthread_mutex_lock(&m_info->mutex); rc= 1; errpos= 0; file_offset= 0; @@ -464,6 +474,7 @@ int myrg_attach_children(MYRG_INFO *m_info, int handle_locking, m_info->keys= min_keys; m_info->last_used_table= m_info->open_tables; m_info->children_attached= TRUE; + pthread_mutex_unlock(&m_info->mutex); DBUG_RETURN(0); err: @@ -473,6 +484,7 @@ err: my_free((char*) m_info->rec_per_key_part, MYF(0)); m_info->rec_per_key_part= NULL; } + pthread_mutex_unlock(&m_info->mutex); my_errno= save_errno; DBUG_RETURN(1); } @@ -494,6 +506,8 @@ err: int myrg_detach_children(MYRG_INFO *m_info) { DBUG_ENTER("myrg_detach_children"); + /* For symmetry with myrg_attach_children() we use the mutex here. */ + pthread_mutex_lock(&m_info->mutex); if (m_info->tables) { /* Do not attach/detach an empty child list. */ @@ -504,6 +518,7 @@ int myrg_detach_children(MYRG_INFO *m_info) m_info->del= 0; m_info->data_file_length= 0; m_info->options= 0; + pthread_mutex_unlock(&m_info->mutex); DBUG_RETURN(0); } -- cgit v1.2.1