- developers - lists.mariadb.org

Re: [Maria-developers] 2be9b69f4ff: MDEV-23001 Precreate static Item_bool() to simplify code
by Sergei Golubchik 14 May '21

14 May '21

Hi, Monty! On Mar 25, Michael Widenius wrote: > revision-id: 2be9b69f4ff (mariadb-10.5.2-510-g2be9b69f4ff) > parent(s): cb545f11169 > author: Michael Widenius <michael.widenius(a)gmail.com> > committer: Michael Widenius <michael.widenius(a)gmail.com> > timestamp: 2021-03-24 14:05:15 +0200 > message: > > MDEV-23001 Precreate static Item_bool() to simplify code > > The following changes where done: > - Create global Item: Item_false and Item_true > - Replace all creation if 'FALSE' and 'TRUE' top level items used for > WHERE/HAVING/ON clauses to use Item_false and Item_true. > > The benefit are: > - No test needed if we where able to create the new item. > - Fixed possible errors if 'new' would have failed for the Item_bool's agree > - Less and faster code I don't quite agree with that, it's not less code: > 7 files changed, 65 insertions(+), 43 deletions(-) and I doubt that speedup can be measured in any benchmarks whatsoever. > diff --git a/sql/item.cc b/sql/item.cc > index 1a86b8b3114..0f160fa257b 100644 > --- a/sql/item.cc > +++ b/sql/item.cc > @@ -55,7 +55,8 @@ const char *item_empty_name=""; > const char *item_used_name= "\0"; > > static int save_field_in_field(Field *, bool *, Field *, bool); > - > +const Item_bool_static Item_false("FALSE", 0); > +const Item_bool_static Item_true("TRUE", 1); The main question - we have tons of modifiable Item members. How do you know none of them will modified in your Item_bool_static objects? > @@ -421,6 +421,9 @@ Item::Item(THD *thd): > /* Initially this item is not attached to any JOIN_TAB. */ > join_tab_idx= MAX_TABLES; > > + /* thd is NULL in case of static items like Item_true */ > + if (!thd) > + return; No, please, don't. thd can be NULL only twice, for statically initialized Item_true and Item_false. For that you removed a useful assert and added an if() that is completely unnecessary for billions of items created runtime. Better to have special contructor just for these two very special items. > /* Put item in free list so that we can free all items at end */ > next= thd->free_list; > thd->free_list= this; > @@ -3637,6 +3641,19 @@ void Item_int::print(String *str, enum_query_type query_type) > } > > > +/* > + This function is needed to ensure that Item_bool_static doesn't change > + the value of the member str_value. > +*/ > + > +void Item_bool::print(String *str, enum_query_type query_type) > +{ > + // my_charset_bin is good enough for numbers > + String tmp(value ? (char*) "1" : (char*) "0" , 1, &my_charset_bin); > + str->append(tmp); > +} This is not needed anymore. Item_bool inherits from Item_int, and Item_int::print is void Item_int::print(String *str, enum_query_type query_type) { StringBuffer<LONGLONG_BUFFER_SIZE> buf; // my_charset_bin is good enough for numbers buf.set_int(value, unsigned_flag, &my_charset_bin); str->append(buf); } You removed str_value from Item_int::print in July. > Item *Item_bool::neg_transformer(THD *thd) > { > value= !value; > diff --git a/sql/item.h b/sql/item.h > index a1c288ab1f0..1087c08869e 100644 > --- a/sql/item.h > +++ b/sql/item.h > @@ -1976,7 +1976,7 @@ class Item: public Value_source, > virtual bool limit_index_condition_pushdown_processor(void *arg) { return 0; } > virtual bool exists2in_processor(void *arg) { return 0; } > virtual bool find_selective_predicates_list_processor(void *arg) { return 0; } > - bool cleanup_is_expensive_cache_processor(void *arg) > + virtual bool cleanup_is_expensive_cache_processor(void *arg) You've added a new virtual method? First you try so hard to remove virtual methods, then you add a new one specifically for these two static items. Why does it matter what cleanup_is_expensive_cache_processor will do with them? You overwrite is_expensive() anyway. > { > is_expensive_cache= (int8)(-1); > return 0; > @@ -3220,9 +3220,11 @@ class Item_literal: public Item_basic_constant > Item_literal(THD *thd): Item_basic_constant(thd) > { } > Type type() const override { return CONST_ITEM; } > - bool check_partition_func_processor(void *) override { return false;} > + bool check_partition_func_processor(void *int_arg) override { return false;} > bool const_item() const override { return true; } > bool basic_const_item() const override { return true; } > + bool is_expensive() override { return false; } > + bool cleanup_is_expensive_cache_processor(void *arg) override { return 0; } > }; > > > diff --git a/sql/opt_index_cond_pushdown.cc b/sql/opt_index_cond_pushdown.cc > index 15bc2074e1f..e950ad1b7ca 100644 > --- a/sql/opt_index_cond_pushdown.cc > +++ b/sql/opt_index_cond_pushdown.cc > @@ -185,8 +185,8 @@ bool uses_index_fields_only(Item *item, TABLE *tbl, uint keyno, > static Item *make_cond_for_index(THD *thd, Item *cond, TABLE *table, uint keyno, > bool other_tbls_ok) > { > - if (!cond) > - return NULL; > + if (!cond || cond->basic_const_item()) > + return cond; The effect of this change is that you don't set cond->marker= ICP_COND_USES_INDEX_ONLY for basic_const_item's anymore. This looked suspicious at first. But then I noticed (and Sergey Petrunia confirmed) that ICP_COND_USES_INDEX_ONLY is not used at all. Since 5.3. So here I'd suggest to revert this your change and completely remove any ICP_COND_USES_INDEX_ONLY manipulations together with the ICP_COND_USES_INDEX_ONLY definition. > if (cond->type() == Item::COND_ITEM) > { > uint n_marked= 0; > diff --git a/sql/sql_parse.cc b/sql/sql_parse.cc > index 119c7360f07..e63e120a02b 100644 > --- a/sql/sql_parse.cc > +++ b/sql/sql_parse.cc > @@ -2395,6 +2395,7 @@ dispatch_command_return dispatch_command(enum enum_server_command command, THD * > thd->update_server_status(); > thd->protocol->end_statement(); > query_cache_end_of_result(thd); > + /* Check that the query didn't change const items */ what does that mean? Is it a TODO comment? Or it documents what happens below? > } > if (drop_more_results) > thd->server_status&= ~SERVER_MORE_RESULTS_EXISTS; > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index 93f5d3591ed..894d511adf4 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -2364,7 +2364,7 @@ int JOIN::optimize_stage2() > if (!conds && outer_join) > { > /* Handle the case where we have an OUTER JOIN without a WHERE */ > - conds= new (thd->mem_root) Item_bool(thd, true); // Always true > + conds= (Item*) &Item_true; Hmm, does casting const away even work in the Intel compiler? > } > > if (impossible_where) > @@ -22743,6 +22743,8 @@ make_cond_for_table_from_pred(THD *thd, Item *root_cond, Item *cond, > return new_cond; > } > } > + else if (cond->basic_const_item()) > + return cond; > > if (is_top_and_level && used_table == rand_table_bit && > (cond->used_tables() & ~OUTER_REF_TABLE_BIT) != rand_table_bit) this is rather fragile, isn't it? You skip item->set_join_tab_idx(join_tab_idx_arg), quite understandably. And currently, as far as I can see, item->get_join_tab_idx() is always used under if (!item->const_item()), so what you did is safe. But can it change in the future? Or is join_tab_idx something that conceptually doesn't make sense for const items? Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 2

Re: [Maria-developers] 9083627c50f: Make rename atomic/repeatable in MyISAM and Aria
by Sergei Golubchik 14 May '21

14 May '21

Hi, Michael! On May 04, Michael Widenius wrote: > revision-id: 9083627c50f (mariadb-10.5.2-562-g9083627c50f) > parent(s): 105cda3b1cb > author: Michael Widenius <michael.widenius(a)gmail.com> > committer: Michael Widenius <michael.widenius(a)gmail.com> > timestamp: 2021-03-25 12:06:34 +0200 > message: > > Make rename atomic/repeatable in MyISAM and Aria > > This is required to make Atomic RENAME TABLE work for these engines > > The requirement is that if we have a server crash in the middle of a > storage engine rename call, the upcoming ddl log recovery should be able > to finalize it by re-execute the rename. > > diff --git a/storage/maria/ma_rename.c b/storage/maria/ma_rename.c > index a4388596f6b..64dac32a51c 100644 > --- a/storage/maria/ma_rename.c > +++ b/storage/maria/ma_rename.c > @@ -106,28 +106,37 @@ int maria_rename(const char *old_name, const char *new_name) > _ma_reset_state(info); > maria_close(info); > > + /* > + This code is written so that it should be possible to re-run a > + failed rename (even if there is a server crash in between the > + renames) and complete it. > + */ > fn_format(from,old_name,"",MARIA_NAME_IEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT); > fn_format(to,new_name,"",MARIA_NAME_IEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT); > if (mysql_file_rename_with_symlink(key_file_kfile, from, to, > MYF(MY_WME | sync_dir))) > - DBUG_RETURN(my_errno); > + index_file_rename_error= my_errno; Shouldn't you continue only if index_file_rename_error == ENOENT ? also, I only very quickly looked at mysql_file_rename_with_symlink, but it didn't look atomic to me when symlinks are present. > fn_format(from,old_name,"",MARIA_NAME_DEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT); > fn_format(to,new_name,"",MARIA_NAME_DEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT); ... > diff --git a/storage/myisam/mi_rename.c b/storage/myisam/mi_rename.c > index 19df2e54213..13cf8c60897 100644 > --- a/storage/myisam/mi_rename.c > +++ b/storage/myisam/mi_rename.c > @@ -22,6 +22,7 @@ > int mi_rename(const char *old_name, const char *new_name) > { > char from[FN_REFLEN],to[FN_REFLEN]; > + int save_errno= 0; > DBUG_ENTER("mi_rename"); > > #ifdef EXTRA_DEBUG > @@ -32,10 +33,13 @@ int mi_rename(const char *old_name, const char *new_name) > fn_format(from,old_name,"",MI_NAME_IEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT); > fn_format(to,new_name,"",MI_NAME_IEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT); > if (mysql_file_rename_with_symlink(mi_key_file_kfile, from, to, MYF(MY_WME))) > - DBUG_RETURN(my_errno); > + save_errno= my_errno; > fn_format(from,old_name,"",MI_NAME_DEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT); > fn_format(to,new_name,"",MI_NAME_DEXT,MY_UNPACK_FILENAME|MY_APPEND_EXT); > - DBUG_RETURN(mysql_file_rename_with_symlink(mi_key_file_dfile, > - from, to, > - MYF(MY_WME)) ? my_errno : 0); > + if (mysql_file_rename_with_symlink(mi_key_file_dfile, > + from, to, > + MYF(MY_WME))) > + if (save_errno) > + save_errno= my_errno; > + DBUG_RETURN(save_errno); Why mi_rename isn't trying to undo a half-failed rename like ma_rename does? Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 32c73138ff4: Remove some usage of Check_level_instant_set and Sql_mode_save
by Sergei Golubchik 14 May '21

14 May '21

Hi, Monty! On Apr 19, Michael Widenius wrote: > revision-id: 32c73138ff4 (mariadb-10.5.2-559-g32c73138ff4) > parent(s): 07708eb9b28 > author: Michael Widenius <michael.widenius(a)gmail.com> > committer: Michael Widenius <michael.widenius(a)gmail.com> > timestamp: 2021-03-25 12:06:34 +0200 > message: > > Remove some usage of Check_level_instant_set and Sql_mode_save > > The reason for the removal are: > - Generates more code > - Storing and retreving THD > - Causes extra code and daata to be generated to handle possible throw > exceptions (which never happens in MariaDB code) > - Uses more stack space No, Monty. We've discussed it already, this makes code more complex, fragile and bug prone. And it does *not* generates extra code or data or stack - before writing this email I've compiled mariadbd both ways and compared the generated code for the function in question, it was *identical*. > Other things: > - Changed convert_const_to_int() to use item->save_in_field_no_warnings(), > which made the code shorter and simpler. > - Added thd as argument to store_key.copy() to make function simpler > - Added thd as argument to some subselect* constructor that inherites > from Item_subselect. these changes are good, thanks Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 6b06a99e4a4: Avoid creating the .frm file twice in some cases
by Sergei Golubchik 14 May '21

14 May '21

Hi, Monty! On Apr 19, Michael Widenius wrote: > revision-id: 6b06a99e4a4 (mariadb-10.5.2-557-g6b06a99e4a4) > parent(s): 9c1e36696f3 > author: Michael Widenius <michael.widenius(a)gmail.com> > committer: Michael Widenius <michael.widenius(a)gmail.com> > timestamp: 2021-03-25 12:06:34 +0200 > message: > > Avoid creating the .frm file twice in some cases > > diff --git a/sql/handler.cc b/sql/handler.cc > index c286aef3a9e..fe2fe6e9426 100644 > --- a/sql/handler.cc > +++ b/sql/handler.cc > @@ -5536,8 +5536,9 @@ int ha_create_table(THD *thd, const char *path, > > if (frm) > { > - bool write_frm_now= !create_info->db_type->discover_table && > - !create_info->tmp_table(); > + bool write_frm_now= (!create_info->db_type->discover_table && > + !create_info->tmp_table() && > + !create_info->frm_is_created); Is this the real problem? frm created twice? Why ha_create_table() is invoked twice at all? > > share.frm_image= frm; > > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index e00d3ee1ed2..73642a5bd97 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -547,8 +547,13 @@ uint build_table_filename(char *buff, size_t bufflen, const char *db, > > (void) tablename_to_filename(db, dbbuff, sizeof(dbbuff)); > > - /* Check if this is a temporary table name. Allow it if a corresponding .frm file exists */ > - if (is_prefix(table_name, tmp_file_prefix) && strlen(table_name) < NAME_CHAR_LEN && > + /* > + Check if this is a temporary table name. Allow it if a corresponding .frm > + file exists. > + */ > + if (!(flags & FN_IS_TMP) && > + is_prefix(table_name, tmp_file_prefix) && > + strlen(table_name) < NAME_CHAR_LEN && good idea! > check_if_frm_exists(tbbuff, dbbuff, table_name)) > flags|= FN_IS_TMP; > > @@ -9817,7 +9827,7 @@ do_continue:; > > if (table->s->tmp_table != NO_TMP_TABLE) > { > - /* Close lock if this is a transactional table */ > + /* Unlock lock if this is a transactional temporary table */ strictly speaking, you don't "unlock lock", you "unlock a resource" (e.g. a table) or you "release a lock" > if (thd->lock) > { > if (thd->locked_tables_mode != LTM_LOCK_TABLES && Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 9c1e36696f3: MDEV-20025: ADD_MONTHS() Oracle function
by Sergei Golubchik 14 May '21

14 May '21

Hi, Monty! On Apr 16, Michael Widenius wrote: > commit 9c1e36696f3 > Author: Michael Widenius <michael.widenius(a)gmail.com> > Date: Fri Feb 5 17:10:43 2021 +0200 > > diff --git a/mysql-test/suite/compat/oracle/r/func_add_months.result b/mysql-test/suite/compat/oracle/r/func_add_months.result > new file mode 100644 > index 00000000000..0e8ca1eef94 > --- /dev/null > +++ b/mysql-test/suite/compat/oracle/r/func_add_months.result > @@ -0,0 +1,85 @@ > +Test for ADD_MONTHS > +CREATE TABLE t1(c1 int, c2 datetime, c3 date, c4 time, c5 timestamp); > +INSERT INTO t1 VALUES (1, '2011-11-12 12:10:11', '2011-11-12', '12:10:11', '2011-11-12 12:10:11'); > +INSERT INTO t1 VALUES (2, '2021-11-12 00:23:12', '2021-11-12', '00:23:12', '2021-11-12 00:23:12'); > +INSERT INTO t1 VALUES (3, '2011-01-22 16:45:45', '2011-01-22', '16:45:45', '2011-01-22 16:45:45'); > +INSERT INTO t1 VALUES (4, '2031-05-12 04:11:34', '2031-05-12', '04:11:34', '2031-05-12 04:11:34'); > +INSERT INTO t1 VALUES (5, '2031-09-02 08:15:22', '2031-09-02', '08:15:22', '2031-09-02 08:15:22'); > +INSERT INTO t1 VALUES (6, '0000-09-02 00:00:00', '0000-09-02', '00:00:00', '1980-09-02 00:00:00'); > +INSERT INTO t1 VALUES (7, '9999-09-02', '9999-09-02', '00:00:00', '1980-09-02'); there's no last-day test here. Like ADD_MONTHS('2012-01-31', 1) = '2012-02-29' should be here, as Oracle manual specifically documents this case. Regards, Sergei

2 1

Re: [Maria-developers] fbe1dad15a1: Ensure that we do not allocate strings bigger than 4G in String objects.
by Sergei Golubchik 14 May '21

14 May '21

Hi, Monty! On Apr 16, Michael Widenius wrote: > revision-id: fbe1dad15a1 (mariadb-10.5.2-555-gfbe1dad15a1) > parent(s): 57c19902326 > author: Michael Widenius <michael.widenius(a)gmail.com> > committer: Michael Widenius <michael.widenius(a)gmail.com> > timestamp: 2021-03-25 12:06:34 +0200 > message: > > Ensure that we do not allocate strings bigger than 4G in String objects. > > This is needed as we are using uint32 for allocated and current length. > > diff --git a/sql/sql_string.cc b/sql/sql_string.cc > index 9c57bb22085..fedf5f4a48a 100644 > --- a/sql/sql_string.cc > +++ b/sql/sql_string.cc > @@ -37,6 +37,8 @@ bool Binary_string::real_alloc(size_t length) > DBUG_ASSERT(arg_length > length); > if (arg_length <= length) > return TRUE; /* Overflow */ > + if (arg_length >= UINT_MAX32) > + return FALSE; > str_length=0; > if (Alloced_length < arg_length) > { > @@ -45,7 +47,6 @@ bool Binary_string::real_alloc(size_t length) > arg_length,MYF(MY_WME | (thread_specific ? > MY_THREAD_SPECIFIC : 0))))) > return TRUE; > - DBUG_ASSERT(length < UINT_MAX32); > Alloced_length=(uint32) arg_length; > alloced=1; I liked assert more. It meant that we should never under any circumstances request more than 4G from Binary_string::real_alloc. You're saying that it's ok to do it and Binary_string::real_alloc should be ready to deal with it. I'd say the first approach was better, >4G should just never happen. Regards, Sergei

2 1

Re: [Maria-developers] c157c285db9: Optimize Sql_alloc
by Sergei Golubchik 14 May '21

14 May '21

Hi, Michael! On Mar 27, Michael Widenius wrote: > revision-id: c157c285db9 (mariadb-10.5.2-514-gc157c285db9) > parent(s): 16e38888c06 > author: Michael Widenius <michael.widenius(a)gmail.com> > committer: Michael Widenius <michael.widenius(a)gmail.com> > timestamp: 2021-03-24 14:31:53 +0200 > message: > > Optimize Sql_alloc > > - Remove 'dummy_for_valgrind' overrun marker as this doesn't help much. > The element also distorts the sizes of objects a bit, which makes it > harder to calculate gain in object sizes when doing size optimizations. > - Avoid one extra call indirection when using thd_get_current_thd(), which > is used by Sql_alloc. > > diff --git a/sql/mysqld.cc b/sql/mysqld.cc > index 4f522d96a9f..1ab52401705 100644 > --- a/sql/mysqld.cc > +++ b/sql/mysqld.cc > @@ -662,7 +662,15 @@ static std::atomic<char*> shutdown_user; > > static thread_local THD *THR_THD; > > +/** > + Get current THD object from thread local data > + > + @retval The THD object for the thread, NULL if not connection thread > +*/ > + > MYSQL_THD _current_thd() { return THR_THD; } > +THD *thd_get_current_thd() { return THR_THD; } I'd rather remove thd_get_current_thd() completely, why do we need two identical functions? Sql_alloc can use _current_thd() just fine, I've tried (also rocksdb uses thd_get_current_thd in one place, but it can use current_thd like the rest of the server code). Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 21eb8969ce9: Improved storage size for Item, Field and some other classes
by Sergei Golubchik 14 May '21

14 May '21

Hi, Monty! Looks ok, but again it doesn't seem you've squashed intermediate commits as you said you did. Bit fields and non-existent commits in columnstore - it's is clearly an intermediate work-in-progress state, all fixed in your later commits. On Sep 08, Michael Widenius wrote: > revision-id: 21eb8969ce9 (mariadb-10.5.2-270-g21eb8969ce9) > parent(s): c3ecf0d6243 > author: Michael Widenius <michael.widenius(a)gmail.com> > committer: Michael Widenius <michael.widenius(a)gmail.com> > timestamp: 2020-09-02 20:58:34 +0300 > message: > > Improved storage size for Item, Field and some other classes > > - Changed order of class fields to remove dead alignment space. > - Changed bool fields in Item to bit fields. > - Used packed enum's for some fields in common classes > - Removed not used Item::rsize. > - Changed some class variables from uint/int to smaller type int's. > - Ensured that field_index is uint16 in all classes and functions. Fixed > also that we proparly compare with NO_CACHED_FIELD_INDEX when checking > if variable is not set. > - Removed checking of highest bit of unireg_check (has not been used in > a long time) > - Fixed wrong arguments to make_cond_for_table() for join_tab_idx_arg > from false to 0. > > One of the result was reducing the size if class Item with ~24 bytes ... > @@ -929,18 +904,48 @@ class Item: public Value_source, > + > + /* > + str_values's main purpose is to be used to cache the value in > + save_in_field > + */ > + String str_value; > + > + LEX_CSTRING name; /* Name of item */ > + /* Original item name (if it was renamed)*/ > + const char *orig_name; > + > + uint32 /* All common bool variables for Item stored here */ > + maybe_null:1, /* If item may be null */ > + in_rollup:1, /* If used in GROUP BY list of a query with ROLLUP */ > + with_param:1, /* True if Item contains an SP parameter */ > + with_window_func:1, /* True if item contains a window func */ > + with_field:1, /* True if any item except Item_sum contains a field. > + Set during parsing. */ > + fixed:1; /* If item was fixed with fix_fields */ > + > + int16 marker; > + > diff --git a/storage/columnstore/columnstore b/storage/columnstore/columnstore > index b6b02ed516f..f606e76fb77 160000 > --- a/storage/columnstore/columnstore > +++ b/storage/columnstore/columnstore > @@ -1 +1 @@ > -Subproject commit b6b02ed516f92055127d416370799d91a82754ea > +Subproject commit f606e76fb779e40f3376693fff9969e4f2b7669a Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 3

Re: [Maria-developers] 4088269c578d: Improved storage size for Item, Field and some other classes
by Sergei Golubchik 14 May '21

14 May '21

Hi, Michael! On Dec 03, Michael Widenius wrote: > revision-id: 4088269c578d (mariadb-10.5.2-270-g4088269c578d) > parent(s): 994ea2af3973 > author: Michael Widenius <michael.widenius(a)gmail.com> > committer: Michael Widenius <michael.widenius(a)gmail.com> > timestamp: 2020-09-17 12:25:44 +0300 > message: > > Improved storage size for Item, Field and some other classes > > diff --git a/storage/columnstore/columnstore b/storage/columnstore/columnstore > index b6b02ed516f9..f606e76fb779 160000 > --- a/storage/columnstore/columnstore > +++ b/storage/columnstore/columnstore > @@ -1 +1 @@ > -Subproject commit b6b02ed516f92055127d416370799d91a82754ea > +Subproject commit f606e76fb779e40f3376693fff9969e4f2b7669a Again. You should not push invalid commit hashes into the main branch: storage/columnstore/columnstore $ git fetch origin storage/columnstore/columnstore $ git show f606e76fb779e40f3376693fff9969e4f2b7669a fatal: bad object f606e76fb779e40f3376693fff9969e4f2b7669a Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1

Re: [Maria-developers] 16e38888c06: Improved storage size for Item, Field and some other classes
by Sergei Golubchik 14 May '21

14 May '21

Hi, Michael! Did you consider trying -Wpadded? '-Wpadded' Warn if padding is included in a structure, either to align an element of the structure or to align the whole structure. Sometimes when this happens it is possible to rearrange the fields of the structure to reduce the padding and so make the structure smaller. Might reveal more useful cases where fields could be rearranged. On Mar 27, Michael Widenius wrote: > revision-id: 16e38888c06 (mariadb-10.5.2-513-g16e38888c06) > parent(s): 4cfb680b685 > author: Michael Widenius <michael.widenius(a)gmail.com> > committer: Michael Widenius <michael.widenius(a)gmail.com> > timestamp: 2021-03-24 14:31:53 +0200 > message: > > Improved storage size for Item, Field and some other classes > > - Changed order of class fields to remove dead alignment space. > - Changed bool fields in Item to bit fields. > - Used packed enum's for some fields in common classes > - Removed not used Item::rsize. > - Changed some class variables from uint/int to smaller type int's. > - Ensured that field_index is uint16 in all classes and functions. Fixed > also that we proparly compare with NO_CACHED_FIELD_INDEX when checking > if variable is not set. > - Removed checking of highest bit of unireg_check (has not been used in > a long time) > - Fixed wrong arguments to make_cond_for_table() for join_tab_idx_arg > from false to 0. > > One of the result was reducing the size if class Item with ~24 bytes Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 1