- developers - lists.mariadb.org

Re: [Maria-developers] [Commits] 7931ca4: MDEV-9024 Build fails with VS2015
by Sergei Golubchik 05 Nov '15

05 Nov '15

Hi, Wlad! Mostly ok, please see below On Nov 03, wlad(a)mariadb.com wrote: > revision-id: 7931ca4f89ad5cd38ea3dc54f1363b7de2fd3414 (mariadb-10.0.22-5-g7931ca4) > parent(s): fa1438cbf4307731a54ea4137d5f7d4b744cdfbc > committer: Vladislav Vaintroub > timestamp: 2015-11-03 11:32:00 +0100 > message: > > MDEV-9024 Build fails with VS2015 > > Fix build failures caused by new C runtime library > - isnan, snprintf, struct timespec are now defined, attempt to redefine them leads > - P_tmpdir, tzname are no more defined > - lfind() and lsearch() in lf_hash.c had to be renamed, declaration > conflicts with some C runtime functions with the same name declared > in a header included by stdlib.h > > Also fix couple of annoying warnings : > - remove #define NOMINMAX from config.h to avoid "redefined" compiler > warnings(NOMINMAX is already in compile flags) > > - disable incremental linker in Debug as well (feature not used much > and compiler crashes often) > > > Also simplify package building with Wix, require Wix 3.9 or later > (VS2015 is not compatible with old Wix 3.5/3.6) > > diff --git a/cmake/os/Windows.cmake b/cmake/os/Windows.cmake > index 4c01bab..fd6e739 100644 > --- a/cmake/os/Windows.cmake > +++ b/cmake/os/Windows.cmake > @@ -181,14 +184,13 @@ CHECK_SYMBOL_REPLACEMENT(S_IROTH _S_IREAD sys/stat.h) > CHECK_SYMBOL_REPLACEMENT(S_IFIFO _S_IFIFO sys/stat.h) > CHECK_SYMBOL_REPLACEMENT(SIGQUIT SIGTERM signal.h) > CHECK_SYMBOL_REPLACEMENT(SIGPIPE SIGINT signal.h) > -CHECK_SYMBOL_REPLACEMENT(isnan _isnan float.h) How can this be wrong? It checks whetherone should use _isnan instead of isnan. That seems like the kind of check that works both with old and new VS. > CHECK_SYMBOL_REPLACEMENT(finite _finite float.h) > diff --git a/config.h.cmake b/config.h.cmake > index 46eed79..a1d505f 100644 > --- a/config.h.cmake > +++ b/config.h.cmake > > -/* We don't want the min/max macros */ > -#ifdef __WIN__ > -#define NOMINMAX > -#endif I'd generally prefer to define symbols in headers, instead of putting everything into ever-growing compiler command line Besides, the minimal fix for the redefinition warning is -#define NOMINMAX +#define NOMINMAX 1 because -DNOMINMAX defines NOMINMAX to be 1, not empty. > /* > MySQL features > */ > diff --git a/mysys/lf_hash.c b/mysys/lf_hash.c > index aa96ca9..fa92971 100644 > --- a/mysys/lf_hash.c > +++ b/mysys/lf_hash.c > @@ -43,7 +43,7 @@ const int LF_HASH_OVERHEAD= sizeof(LF_SLIST); > > /* > a structure to pass the context (pointers two the three successive elements > - in a list) from lfind to linsert/ldelete > + in a list) from my_lfind to my_linsert/my_ldelete > */ > typedef struct { > intptr volatile *prev; > @@ -70,7 +70,7 @@ typedef struct { > cursor is positioned in either case > pins[0..2] are used, they are NOT removed on return > */ > -static int lfind(LF_SLIST * volatile *head, CHARSET_INFO *cs, uint32 hashnr, > +static int my_lfind(LF_SLIST * volatile *head, CHARSET_INFO *cs, uint32 hashnr, er... I don't like that. lfind/lsearch are internal static functions in lf_hash.c, they shouldn't need or use my_ prefix. Two alternatives: 1. #define lfind remap_lfind_for_win #include <my_global.h> #include <my_sys.h> #undef lfind 2. -static int lfind(LF_SLIST * volatile *head, CHARSET_INFO *cs, uint32 hashnr, +static int l_find(LF_SLIST * volatile *head, CHARSET_INFO *cs, uint32 hashnr, > const uchar *key, uint keylen, CURSOR *cursor, LF_PINS *pins) > { > uint32 cur_hashnr; > diff --git a/win/packaging/CMakeLists.txt b/win/packaging/CMakeLists.txt > index 3b9b72f..0535a48 100644 > --- a/win/packaging/CMakeLists.txt > +++ b/win/packaging/CMakeLists.txt > SET(CPACK_WIX_UPGRADE_CODE "49EB7A6A-1CEF-4A1E-9E89-B9A4993963E3") > - SET(CPACK_WIX_PACKAGE_NAME "MariaDB @MAJOR_VERSION@.@MINOR_VERSION@") > + SET(CPACK_WIX_PACKAGE_NAME "MariaDB ${MAJOR_VERSION}.${MINOR_VERSION}") > ELSE() > SET(CPACK_WIX_UPGRADE_CODE "2331E7BD-EE58-431B-9E18-B2B918BCEB1B") > - SET(CPACK_WIX_PACKAGE_NAME "MariaDB @MAJOR_VERSION@.@MINOR_VERSION@ (x64)") > + SET(CPACK_WIX_PACKAGE_NAME "MariaDB ${MAJOR_VERSION}.${MINOR_VERSION} (x64)") Weird. I was sure I've fixed this months ago :-[] Regards, Sergei

1 0

Re: [Maria-developers] [Commits] 01bd373: MDEV-8437 - plugin variables conflict with bootstrap
by Sergei Golubchik 05 Nov '15

05 Nov '15

Hi, Sergey! On Nov 03, Sergey Vojtovich wrote: > revision-id: 01bd3737777f8f0cf2e1737d01a48033a7615b2a (mariadb-10.1.8-23-g01bd373) > parent(s): dc40d737d4af3cb0e272c58507ad54e4eb24be28 > committer: Sergey Vojtovich > timestamp: 2015-11-03 14:02:49 +0400 > message: > > MDEV-8437 - plugin variables conflict with bootstrap ok to push > There is no strong need to change password column: the only side effect is > that 4.0 -> 10.1 upgrades may get root/debian-sys-maint passwords stored in old > format. This should be perfectly acceptable, since all password at this point > are stored in old format. Regards, Sergei

2 3

Re: [Maria-developers] [Commits] 178cddd: MDEV-9082 - Debian: mysql_install_db is called on upgrade
by Sergei Golubchik 05 Nov '15

05 Nov '15

Hi, Sergey! On Nov 05, Sergey Vojtovich wrote: > revision-id: 178cddd664cb4ce2c6ccbe76c33e4fd2c1fc1b3d (mariadb-10.1.8-25-g178cddd) > parent(s): 2ef562d2c74297b0b92c4322697dad98767cc873 > committer: Sergey Vojtovich > timestamp: 2015-11-05 13:57:24 +0400 > message: > > MDEV-9082 - Debian: mysql_install_db is called on upgrade looks ok to me Regards, Sergei

1 0

Re: [Maria-developers] [Commits] 2ef562d: MDEV-9080 - Debian: incorrect empty password check in postinst
by Sergei Golubchik 05 Nov '15

05 Nov '15

Hi, Sergey! ok to push On Nov 04, Sergey Vojtovich wrote: > revision-id: 2ef562d2c74297b0b92c4322697dad98767cc873 (mariadb-10.1.8-24-g2ef562d) > parent(s): 01bd3737777f8f0cf2e1737d01a48033a7615b2a > committer: Sergey Vojtovich > timestamp: 2015-11-04 20:37:16 +0400 > message: > > MDEV-9080 - Debian: incorrect empty password check in postinst > > There was code that was supposed to "catch upgrades from previous versions where > the root password wasn't set". But it is wrong in many regards: > - it is supposed to be executed against running server, but at this point server > should be down, which makes this code no-op > - if the above is fixed, root password will be requested twice (initial root > password request + this one) > - it asks for a password only once, while "initial root password request" asks > twice (password + password verification) > - it may give false positive if unix socket based authentication is in effect > > Removed this code since it didn't work for quite a while (at least since > mysql-5.1) and nobody cared about it. Regards, Sergei

1 0

Re: [Maria-developers] [Commits] dc40d73: MDEV-8437 - plugin variables conflict with bootstrap
by Sergei Golubchik 05 Nov '15

05 Nov '15

Hi, Sergey! On Nov 03, Sergey Vojtovich wrote: > revision-id: dc40d737d4af3cb0e272c58507ad54e4eb24be28 (mariadb-10.1.8-22-gdc40d73) > parent(s): a574407444fc3ea93ca88afa93dc18154251bf74 > committer: Sergey Vojtovich > timestamp: 2015-11-03 12:08:23 +0400 > message: > > MDEV-8437 - plugin variables conflict with bootstrap ok to push > Removed redundant attempt to create mysql.plugin table: > - original code was supposed to INSTALL some plugins: > INSERT INTO plugin VALUES ('innodb', 'ha_innodb.so'), > ('federated', 'ha_federated.so'), ('blackhole', 'ha_blackhole.so'), > ('archive', 'ha_archive.so'); I didn't see any INSERT's in the postinst script. > - original code was supposed to fail if mysql.plugin exists: > The query sequence is supposed to be aborted if the CREATE TABLE fails due > to an already existent table in which case the admin might already have > chosen to remove one or more plugins. > - mysql.plugin must've been created by preceeding mysql_install_db anyway > Regards, Sergei

2 1

Re: [Maria-developers] [Commits] 4685c90: mariadb@.service.in should be configured for all systemd platforms.
by Sergey Vojtovich 05 Nov '15

05 Nov '15

Hi Nirbhay, taking into account that CPackRPM won't be able to install it, what's the purpose of having this configured for all systemd platforms? Regards, Sergey On Tue, Nov 03, 2015 at 05:38:12PM -0500, Nirbhay Choubey wrote: > revision-id: 4685c903bc822596bcf12cbfdb5bcc4460ed0105 > parent(s): 95289e5b665d6a3e37f938d0db978d0d0166e493 > committer: Nirbhay Choubey > branch nick: 10.1 > timestamp: 2015-11-03 17:38:11 -0500 > message: > > mariadb@.service.in should be configured for all systemd platforms. > > --- > support-files/CMakeLists.txt | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/support-files/CMakeLists.txt b/support-files/CMakeLists.txt > index 2ab4350..2929db1 100644 > --- a/support-files/CMakeLists.txt > +++ b/support-files/CMakeLists.txt > @@ -79,9 +79,9 @@ IF(UNIX) > IF(HAVE_SYSTEMD) > CONFIGURE_FILE(mariadb.service.in > ${CMAKE_CURRENT_BINARY_DIR}/mariadb.service @ONLY) > + CONFIGURE_FILE(mariadb@.service.in > + ${CMAKE_CURRENT_BINARY_DIR}/mariadb@.service @ONLY) > IF(NOT RPM) > - CONFIGURE_FILE(mariadb@.service.in > - ${CMAKE_CURRENT_BINARY_DIR}/mariadb@.service @ONLY) > INSTALL(FILES use_galera_new_cluster.conf > ${CMAKE_CURRENT_BINARY_DIR}/mariadb.service > ${CMAKE_CURRENT_BINARY_DIR}/mariadb@.service > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

2 2

[Maria-developers] write_set.diff review
by Sergei Golubchik 04 Nov '15

04 Nov '15

Hi, Summary: 1. Three questions (are you sure, what guarantee, why) 2. A couple of suggestions (spilt a commit, move function call out of DBUG_RETURN) 3. Two suggestions for unrelated optimizations (don't allocate unneded bitmaps, don't modify read_set) > commit 2734eca05d499bcbcaf428d04d4f1acbb298589f > Author: Monty <monty(a)mariadb.org> > Date: Mon Nov 2 11:52:06 2015 +0200 > > diff --git a/sql/table.h b/sql/table.h > index b150e4f..6ecfdef 100644 > --- a/sql/table.h > +++ b/sql/table.h > @@ -1066,9 +1066,11 @@ struct TABLE > String alias; /* alias or table name */ > uchar *null_flags; > MY_BITMAP def_read_set, def_write_set, def_vcol_set, tmp_set; > + MY_BITMAP def_rpl_write_set; I would rather not allocate more bitmaps directly in the TABLE. They are not always needed, and only increase the TABLE size. The same is true for def_vcol_set, in fact, it's *even more true* for vcols. Instead I'd allocate them in the TABLE's memroot as needed. > MY_BITMAP eq_join_set; /* used to mark equi-joined fields */ > MY_BITMAP cond_set; /* used to mark fields from sargable conditions*/ > - MY_BITMAP *read_set, *write_set, *vcol_set; /* Active column sets */ > + /* Active column sets */ > + MY_BITMAP *read_set, *write_set, *vcol_set, *rpl_write_set; > /* > The ID of the query that opened and is using this table. Has different > meanings depending on the table type. > diff --git a/sql/table.cc b/sql/table.cc > index 76cb4b8..8816ee0 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -2884,20 +2884,28 @@ enum open_frm_error open_table_from_share(THD *thd, TABLE_SHARE *share, > /* Allocate bitmaps */ > > bitmap_size= share->column_bitmap_size; > - if (!(bitmaps= (uchar*) alloc_root(&outparam->mem_root, bitmap_size*6))) > + if (!(bitmaps= (uchar*) alloc_root(&outparam->mem_root, bitmap_size*7))) > goto err; > my_bitmap_init(&outparam->def_read_set, > (my_bitmap_map*) bitmaps, share->fields, FALSE); > my_bitmap_init(&outparam->def_write_set, > - (my_bitmap_map*) (bitmaps+bitmap_size), share->fields, FALSE); > + (my_bitmap_map*) (bitmaps+bitmap_size), share->fields, > + FALSE); > my_bitmap_init(&outparam->def_vcol_set, > - (my_bitmap_map*) (bitmaps+bitmap_size*2), share->fields, FALSE); > + (my_bitmap_map*) (bitmaps+bitmap_size*2), share->fields, > + FALSE); This is totally wasted memory if the table has no virtual columns. (that's what I meant by *even more true* - rpl bitmap and eq_join_set, cond_set bitmaps may or may not be used for this table, one never knows. But for vcols it's always known in advance whether the table has them or not). Still, I'd allocate other bitmaps on demand too. > my_bitmap_init(&outparam->tmp_set, > - (my_bitmap_map*) (bitmaps+bitmap_size*3), share->fields, FALSE); > + (my_bitmap_map*) (bitmaps+bitmap_size*3), share->fields, > + FALSE); > my_bitmap_init(&outparam->eq_join_set, > - (my_bitmap_map*) (bitmaps+bitmap_size*4), share->fields, FALSE); > + (my_bitmap_map*) (bitmaps+bitmap_size*4), share->fields, > + FALSE); > my_bitmap_init(&outparam->cond_set, > - (my_bitmap_map*) (bitmaps+bitmap_size*5), share->fields, FALSE); > + (my_bitmap_map*) (bitmaps+bitmap_size*5), share->fields, > + FALSE); > + my_bitmap_init(&outparam->def_rpl_write_set, > + (my_bitmap_map*) (bitmaps+bitmap_size*6), share->fields, > + FALSE); > outparam->default_column_bitmaps(); > > outparam->cond_selectivity= 1.0; > @@ -5986,36 +5997,48 @@ void TABLE::mark_columns_needed_for_insert() > we only want to log a PK and we needed other fields for > execution). > */ > + > void TABLE::mark_columns_per_binlog_row_image() > { > + THD *thd= in_use; > DBUG_ENTER("mark_columns_per_binlog_row_image"); > DBUG_ASSERT(read_set->bitmap); > DBUG_ASSERT(write_set->bitmap); > > - THD *thd= current_thd; Hmm. Are you sure that TABLE::in_use is *always* set at this point? You've basically replaced - THD *thd=current_thd; - if (in_use && in_use->is_current_stmt_binlog_format_row) + THD *thd=in_use; + if (thd->is_current_stmt_binlog_format_row) And, btw, where's your MDEV for caching rbr checks per statement? > + /* If not using row format */ > + rpl_write_set= write_set; > > /** > If in RBR we may need to mark some extra columns, > depending on the binlog-row-image command line argument. > */ > if ((WSREP_EMULATE_BINLOG(thd) || mysql_bin_log.is_open()) && > - in_use && > - in_use->is_current_stmt_binlog_format_row() && > + thd->is_current_stmt_binlog_format_row() && > !ha_check_storage_engine_flag(s->db_type(), HTON_NO_BINLOG_ROW_OPT)) > { > /* if there is no PK, then mark all columns for the BI. */ > if (s->primary_key >= MAX_KEY) > + { > bitmap_set_all(read_set); > + rpl_write_set= read_set; > + } > + else > + { > switch (thd->variables.binlog_row_image) { > case BINLOG_ROW_IMAGE_FULL: > - if (s->primary_key < MAX_KEY) > bitmap_set_all(read_set); > - bitmap_set_all(write_set); > + /* Set of columns that should be written (all) */ > + rpl_write_set= read_set; > break; > case BINLOG_ROW_IMAGE_NOBLOB: > - /* for every field that is not set, mark it unless it is a blob */ > + /* Only write changed columns + not blobs */ > + rpl_write_set= &def_rpl_write_set; > + bitmap_copy(rpl_write_set, write_set); > + > + /* > + for every field that is not set, mark it unless it is a blob or > + part of a primary key > + */ > for (Field **ptr=field ; *ptr ; ptr++) > { > Field *my_field= *ptr; > @@ -6027,23 +6050,24 @@ void TABLE::mark_columns_per_binlog_row_image() > If set in the AI, then the blob is really needed, there is > nothing we can do about it. > */ > - if ((s->primary_key < MAX_KEY) && > - ((my_field->flags & PRI_KEY_FLAG) || > - (my_field->type() != MYSQL_TYPE_BLOB))) > + if ((my_field->flags & PRI_KEY_FLAG) || > + (my_field->type() != MYSQL_TYPE_BLOB)) > + { > bitmap_set_bit(read_set, my_field->field_index); > - > - if (my_field->type() != MYSQL_TYPE_BLOB) > - bitmap_set_bit(write_set, my_field->field_index); > + bitmap_set_bit(rpl_write_set, my_field->field_index); > } > + } > break; > case BINLOG_ROW_IMAGE_MINIMAL: > - /* mark the primary key if available in the read_set */ > - if (s->primary_key < MAX_KEY) > + /* mark the primary key */ > mark_columns_used_by_index_no_reset(s->primary_key, read_set); > + /* Only write columns that have changed */ > + rpl_write_set= write_set; Where's the guarantee that the primary key is in the write set? > break; > > default: > DBUG_ASSERT(FALSE); > + } > } > file->column_bitmaps_signal(); > } > diff --git a/sql/opt_range.cc b/sql/opt_range.cc > index 860426c..6201065 100644 > --- a/sql/opt_range.cc > +++ b/sql/opt_range.cc > @@ -11140,26 +11138,21 @@ int QUICK_RANGE_SELECT::reset() > int QUICK_RANGE_SELECT::get_next() > { > range_id_t dummy; > + int result; > + DBUG_ENTER("QUICK_RANGE_SELECT::get_next"); > + > + if (!in_ror_merged_scan) > + DBUG_RETURN(file->multi_range_read_next(&dummy)); please don't put function calls inside DBUG_RETURN, always use a variable to store the return value first > + > MY_BITMAP * const save_read_set= head->read_set; > MY_BITMAP * const save_write_set= head->write_set; > - > - DBUG_ENTER("QUICK_RANGE_SELECT::get_next"); > - if (in_ror_merged_scan) > - { > /* > We don't need to signal the bitmap change as the bitmap is always the > same for this head->file > */ > head->column_bitmaps_set_no_signal(&column_bitmap, &column_bitmap); > - } > - > - int result= file->multi_range_read_next(&dummy); > + result= file->multi_range_read_next(&dummy); > - > - if (in_ror_merged_scan) > - { > - /* Restore bitmaps set on entry */ > head->column_bitmaps_set_no_signal(save_read_set, save_write_set); > - } > DBUG_RETURN(result); > } Ok, all changes in opt_range.cc are unrelated to your rpl_write_set patch, please commit them separately > diff --git a/sql/rpl_record.cc b/sql/rpl_record.cc > index ee222f0..208b2b6 100644 > --- a/sql/rpl_record.cc > +++ b/sql/rpl_record.cc > @@ -434,7 +433,7 @@ unpack_row(rpl_group_info *rgi, > *master_reclength = table->s->reclength; > } > > - DBUG_RETURN(error); > + DBUG_RETURN(0); > } > > /** also unrelated to rpl_write_set. If you'd like you can put it in the same commit with opt_range.cc changes, with the sybject "cleanups" or something > diff --git a/sql/sql_class.cc b/sql/sql_class.cc > index 5054357..b238e89 100644 > --- a/sql/sql_class.cc > +++ b/sql/sql_class.cc > @@ -6259,11 +6248,11 @@ int THD::binlog_delete_row(TABLE* table, bool is_trans, > DBUG_ASSERT(is_current_stmt_binlog_format_row() && > ((WSREP(this) && wsrep_emulate_bin_log) || mysql_bin_log.is_open())); > /** > - Save a reference to the original read and write set bitmaps. > - We will need this to restore the bitmaps at the end. > + Save a reference to the original read bitmaps > + We will need this to restore the bitmaps at the end as > + binlog_prepare_row_images() may changetable->read_set. > */ This doesn't make much sense. Here, the code is like that THD::binlog_delete_row: save table->read_set THD::binlog_prepare_row_images calculate table->tmp_set table->read_set=table->tmp_set pack_row(table->read_set) for every set bit in the set_arg call field->pack() restore table->read_set So, two thoughts here: 1. tmp_set must be a strict subset of read_set! other fields may not be present in the record[0] and there're no row reads between binlog_prepare_row_images and field->pack 2. One can pass table->tmp_set directly into pack_row() as an argument which means there's no need for binlog_prepare_row_images to modify table->read_set at all > MY_BITMAP *old_read_set= table->read_set; > - MY_BITMAP *old_write_set= table->write_set; > > /** > This will remove spurious fields required during execution but > diff --git a/sql/sql_insert.cc b/sql/sql_insert.cc > index 488826a..0661e70 100644 > --- a/sql/sql_insert.cc > +++ b/sql/sql_insert.cc > @@ -2867,5 +2867,7 @@ pthread_handler_t handle_delayed_insert(void *arg) > /* Tell client that the thread is initialized */ > mysql_cond_signal(&di->cond_client); > > + di->table->mark_columns_needed_for_insert(); why? > + > /* Now wait until we get an insert or lock to handle */ > /* We will not abort as long as a client thread uses this thread */ Regards, Sergei

1 0

[Maria-developers] Python Storage Engine
by Kim Ebert 02 Nov '15

02 Nov '15

Greetings All, I wanted to share the beginnings of a project. This is a Python Storage Engine. The idea is to plumb in the necessary C to Python calls to allow a storage engine to be written in Python instead of C or C++. This may be valuable to those who want to prototype a database engine in Python, or if the performance is sufficient for the use case, for production. I haven't taken things to far yet, but I think it gives a rough idea of how to move forward with adding the other calls. In my particular case I'm using it to prototype connecting to a search API, like Solr, to provide an SQL like interface. I did have a quick question. I want to license it as a more permissive license, but since I'm using the example engine, does that mean that I need to use the GPL license? Or can I go with the more permissive MIT-new license? https://github.com/perfectsearch/mariadb-plugin-python-engine Thoughts and/or feedback are/is appreciated. Thanks, Kim

3 2

Re: [Maria-developers] MDEV-8692 prefschema test failures on ARM (on Debian build system)
by Sergei Golubchik 28 Oct '15

28 Oct '15

Hi, Alexander! On Oct 23, Alexander Barkov wrote: > Hi, > > - the variant with MY_PERF_CYCLES generally works, but does not fix the > test failure. What do you mean by that? How does it "work" if it does not fix the test failure? > - the variant with MY_CLOCK_REALTIME_FOR_CYCLES works fine > and fixed the problem. > > But it's not really "CYCLES". Perhaps this is not a problem, > because on some other platforms we also fallback cycles to nanoseconds, That commit 49667f0 falls back from cycles to nanoseconds explicitly. So it's ok not to support cycles, as far as I understand, and there's no need to do nanoseconds in place of cycles like your MY_CLOCK_REALTIME_FOR_CYCLES. By the way, why 49667f0 didn't fix the problem? What happens on ARMv7 - no 'cycles' timer at all in perfschema, or there is one and it works incorrectly? > diff --git a/mysql-test/suite/perfschema/t/func_file_io.test b/mysql-test/suite/perfschema/t/func_file_io.test > index 2055fa9..5894cb0 100644 > --- a/mysql-test/suite/perfschema/t/func_file_io.test > +++ b/mysql-test/suite/perfschema/t/func_file_io.test > @@ -43,6 +43,11 @@ TRUNCATE TABLE performance_schema.events_waits_current; > > SELECT * FROM t1 WHERE id = 1; > > +--vertical_results > +SELECT * FROM performance_schema.events_waits_history_long > +WHERE EVENT_NAME = 'wait/io/file/myisam/dfile'; > +--horizontal_results forgotten debug output? > + > SET @before_count = (SELECT SUM(TIMER_WAIT) > FROM performance_schema.events_waits_history_long > WHERE (EVENT_NAME = 'wait/io/file/myisam/dfile') > diff --git a/mysys/my_rdtsc.c b/mysys/my_rdtsc.c > index 028c7f8..a5ed808 100644 > --- a/mysys/my_rdtsc.c > +++ b/mysys/my_rdtsc.c > @@ -533,6 +590,10 @@ void my_timer_init(MY_TIMER_INFO *mti) > mti->cycles.routine= MY_TIMER_ROUTINE_ASM_GCC_SPARC32; > #elif defined(__sgi) && defined(HAVE_CLOCK_GETTIME) && defined(CLOCK_SGI_CYCLE) > mti->cycles.routine= MY_TIMER_ROUTINE_SGI_CYCLE; > +#elif defined(MY_CLOCK_REALTIME_FOR_CYCLES) > + mti->cycles.routine= MY_TIMER_ROUTINE_CLOCK_GETTIME; > +#elif defined(MY_PERF_CYCLES) > + mti->cycles.routine= MY_TIMER_ROUTINE_SGI_CYCLE; I'd say that if we'll have it pushed, you'll need to add a dedicated MY_TIMER_ROUTINE_xxx for this, not reuse MY_TIMER_ROUTINE_SGI_CYCLE. > #elif defined(HAVE_SYS_TIMES_H) && defined(HAVE_GETHRTIME) > mti->cycles.routine= MY_TIMER_ROUTINE_GETHRTIME; > #else Regards, Sergei

1 0

Re: [Maria-developers] [Commits] 72bb358: MDEV-7930 Assertion `table_share->tmp_table != NO_TMP_TABLE || m_lock_type != 2' failed in handler::ha_index_read_map
by Sergey Petrunia 27 Oct '15

27 Oct '15

Hi Sanja, Please find some cosmetic feedback below. Ok to push after it has been addressed. On Mon, Oct 26, 2015 at 05:00:34PM +0100, sanja(a)mariadb.com wrote: > revision-id: 72bb358c2ba98e6e0d4a15b92af514a45751fe8d (mariadb-10.0.21-44-g72bb358) > parent(s): 18f7dfed179204dcfc02a27790e22bb9cc4e2e32 > committer: Oleksandr Byelkin > timestamp: 2015-10-26 16:59:40 +0100 > message: > > MDEV-7930 Assertion `table_share->tmp_table != NO_TMP_TABLE || m_lock_type != 2' failed in handler::ha_index_read_map > > In optimizing aggregate function do not try to touch tables from outer > query. > ... > diff --git a/sql/opt_sum.cc b/sql/opt_sum.cc > index fc3ce09..ad29229 100644 > --- a/sql/opt_sum.cc > +++ b/sql/opt_sum.cc > @@ -341,7 +341,8 @@ int opt_sum_query(THD *thd, > there are no outer joins. > */ > if (!conds && !((Item_sum_count*) item)->get_arg(0)->maybe_null && > - !outer_tables && maybe_exact_count) > + !outer_tables && maybe_exact_count && > + ((item->used_tables() & OUTER_REF_TABLE_BIT) == 0)) > { > if (!is_exact_count) > { > @@ -369,7 +370,8 @@ int opt_sum_query(THD *thd, > indexes to find the key. > */ > Item *expr=item_sum->get_arg(0); > - if (expr->real_item()->type() == Item::FIELD_ITEM) > + if (((expr->used_tables() & OUTER_REF_TABLE_BIT) == 0) && > + expr->real_item()->type() == Item::FIELD_ITEM) > { > uchar key_buff[MAX_KEY_LENGTH]; > TABLE_REF ref; > @@ -394,16 +396,20 @@ int opt_sum_query(THD *thd, > const_result= 0; > break; > } > + /* > + Constant table is already read and the only row is the max > + and the min > + */ I think this comment is not relevant anymore? It's part of the first variant of the fix, which is no longer in the patch. Please remove it. > if (!(error= table->file->ha_index_init((uint) ref.key, 1))) > - error= (is_max ? > + error= (is_max ? > get_index_max_value(table, &ref, range_fl) : > get_index_min_value(table, &ref, item_field, range_fl, > prefix_len)); > > /* Verify that the read tuple indeed matches the search key */ > - if (!error && reckey_in_range(is_max, &ref, item_field->field, > - conds, range_fl, prefix_len)) > - error= HA_ERR_KEY_NOT_FOUND; > + if (!error && reckey_in_range(is_max, &ref, item_field->field, > + conds, range_fl, prefix_len)) > + error= HA_ERR_KEY_NOT_FOUND; These are just endspace changes. Can you remove them? > table->disable_keyread(); > table->file->ha_index_end(); > if (error) > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits -- BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0