- developers - lists.mariadb.org

[Maria-developers] MDEV-9504
by Jason Alpers 02 Apr '21

02 Apr '21

Hello, We're hitting the problem in MDEV-9504 ( https://jira.mariadb.org/browse/MDEV-9504) I see from the commit at https://github.com/MariaDB/server/commit/07b8aefe90ca830d2de068f2966cd2288b… that this change was backed out due to test failures. I ran through all the mysql-tests with and without the change and haven't been able to pinpoint a difference though. The reverting commit indicates the correct value for select_limit is only computed when the query plan changes. Any chance someone could provide more detail on what needs to be changed to avoid regression with this fix? If there are sample queries that regress with only the original MDEV-9504 changes, that would be helpful. Thanks in advance! Jase

1 0

Re: [Maria-developers] GSOC21: MDEV-16375 & MDEV-23143
by Hollow Man 02 Apr '21

02 Apr '21

Hi community! I've had my proposal shared with https://drive.google.com/file/d/1sv0qbqt9W-ob3GqxygWwRGurpRS1lCiv/view , hope to get some feedback from the community.   Songlin   ------------------ Original ------------------ From:  "Hollow Man"<hollowman(a)hollowman.ml>; Date:  Thu, Mar 11, 2021 00:17 AM To:  "maria-developers"<maria-developers(a)lists.launchpad.net>; Subject:  GSOC21: MDEV-16375 & MDEV-23143   Hi MariaDB community!    Glad to be here! My github account is @HollowMan6. Though I'm new to MariaDB community, I'm interested in MDEV-16375 & MDEV-23143: Function to normalize a json value & missing a JSON_EQUALS function for this year's GSOC project. Here are my first thoughts on these issues:    I have checked part of the codebase and I think the two issues can be merged into one. First we can create a function named JSON_NORMALIZE to normalize the json, which automatically parses the inputed json document, recursively sorts the keys (for objects) / sorts the numbers (for arrays), removes the spaces, and then return the json document string.    Then we create a function named JSON_EQUALS, which can be used to compare 2 json documents for equality realized by first seperately normalize the two json documents using JSON_NORMALIZE, then the 2 can be compared exactly as binary strings.    I have taken some inspirations from the Item_func_json_keys and json_scan_start for parsing json documents, and I think it's possible to sort the keys using std::map in STL for objects.    That's all for my ideas so far. Please correct me if I made some mistakes, and I'm going to work on my ideas later. Cheers! Hollow Man

3 4

Re: [Maria-developers] JSON_TABLE: On name resolution question
by Sergey Petrunia 02 Apr '21

02 Apr '21

On Thu, Apr 01, 2021 at 10:14:04PM -0700, Igor Babaev wrote: > Sergey, > > Can you explain this: > > MariaDB [test]> explain format=json select * from t1 as t2 where item_name > in (select item_name from t2 as s, json_table(t2.item_props,'$' columns( > color varchar(100) path '$.color')) as t) and t2.item_name='Jeans'; > +-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ > | EXPLAIN > > > > > > > | > +-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ > | { > "query_block": { > "select_id": 1, > "table": { > "table_name": "t2", > "access_type": "ALL", > "rows": 2, > "filtered": 100, > "attached_condition": "t2.item_name = 'Jeans'" > }, > "table": { > "table_name": "s", > "access_type": "ALL", > "rows": 2, > "filtered": 100, > "attached_condition": "s.item_name = 'Jeans'" > }, > "table": { > "table_name": "t", > "access_type": "ALL", > "rows": 40, > "filtered": 100, > "table_function": "json_table", > "first_match": "t2" > } > } > } | > +-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+ > How do get get s.item_name = 'Jeans' ? > Check out the optimizer trace: First, the subquery is converted into a semi-join: "transformation": { "select_id": 2, "from": "IN (SELECT)", "to": "semijoin", "converted_to_semi_join": true } then, there's a multiple equality: "transformation": "equality_propagation", "resulting_condition": "1 and multiple equal('Jeans', t2.item_name, s.item_name)" and then, after substituion: "attaching_conditions_to_tables": { "original_condition": "t2.item_name = 'Jeans' and s.item_name = 'Jeans'", BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 0

Re: [Maria-developers] 9abbb589e6d: MDEV-20220: Merge 5.7 P_S replication table 'replication_applier_status_by_worker
by Sergei Golubchik 01 Apr '21

01 Apr '21

Hi, Sujatha! thanks! ok to push. one question below On Apr 01, Sujatha wrote: > revision-id: 9abbb589e6d (mariadb-10.5.2-531-g9abbb589e6d) > parent(s): 04c7d5f1518 > author: Sujatha <sujatha.sivakumar(a)mariadb.com> > committer: Sujatha <sujatha.sivakumar(a)mariadb.com> > timestamp: 2021-03-29 22:57:26 +0530 > message: > > MDEV-20220: Merge 5.7 P_S replication table 'replication_applier_status_by_worker > > Step 3: > ====== > > Preserve worker pool information on either STOP SLAVE/Error. In case STOP > SLAVE is executed worker threads will be gone, hence worker threads will be > unavailable. Querying the table at this stage will give empty rows. To > address this case when worker threads are about to stop, due to an error or > forced stop, create a backup pool and preserve the data which is relevant to > populate performance schema table. Clear the backup pool upon slave start. > > > diff --git a/mysql-test/suite/perfschema/r/dml_handler.result b/mysql-test/suite/perfschema/r/dml_handler.result > index ce4ae6bf9a3..61bbba3189e 100644 > --- a/mysql-test/suite/perfschema/r/dml_handler.result > +++ b/mysql-test/suite/perfschema/r/dml_handler.result > @@ -12,78 +12,81 @@ Warning 1287 '<select expression> INTO <destination>;' is deprecated and will be > # For each table in the performance schema, attempt HANDLER...OPEN, > # which should fail with an error 1031, ER_ILLEGAL_HA. > # ... > ERROR HY000: Storage engine PERFORMANCE_SCHEMA of the table `performance_schema`.`replication_connection_configuration` doesn't have this option > +SELECT TABLE_NAME INTO @table_name FROM table_list WHERE id=57; > +HANDLER performance_schema.replication_applier_status_by_worker OPEN; > +ERROR HY000: Storage engine PERFORMANCE_SCHEMA of the table `performance_schema`.`replication_applier_status_by_worker` doesn't have this option > SELECT TABLE_NAME INTO @table_name FROM table_list WHERE id=56; > diff --git a/mysql-test/suite/perfschema/r/information_schema.result b/mysql-test/suite/perfschema/r/information_schema.result > index 3f4a8cf6336..c0aff9bb956 100644 > --- a/mysql-test/suite/perfschema/r/information_schema.result > +++ b/mysql-test/suite/perfschema/r/information_schema.result > @@ -59,6 +59,7 @@ performance_schema prepared_statements_instances def > performance_schema replication_applier_configuration def > performance_schema replication_applier_status def > performance_schema replication_applier_status_by_coordinator def > +performance_schema replication_applier_status_by_worker def > performance_schema replication_connection_configuration def > performance_schema rwlock_instances def > performance_schema session_account_connect_attrs def etc, etc. many tests showing that this commit adds a new performance_schema.replication_applier_status_by_worker table. but looking at the commit, I don't think it does. incorrectly split commits, perhaps? Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] 04c7d5f1518: MDEV-20220: Merge 5.7 P_S replication table 'replication_applier_status_by_worker
by Sergei Golubchik 01 Apr '21

01 Apr '21

Hi, Sujatha! thanks! ok to push On Apr 01, Sujatha wrote: > revision-id: 04c7d5f1518 (mariadb-10.5.2-530-g04c7d5f1518) > parent(s): a73681e1f03 > author: Sujatha <sujatha.sivakumar(a)mariadb.com> > committer: Sujatha <sujatha.sivakumar(a)mariadb.com> > timestamp: 2021-03-29 17:36:16 +0530 > message: > > MDEV-20220: Merge 5.7 P_S replication table 'replication_applier_status_by_worker > > Step2: > ===== > Add two extra columns mentioned below. > --------------------------------------------------------------------------- > |Column Name: | Description: | > |-------------------------------------------------------------------------| > | | | > |WORKER_IDLE_TIME | Total idle time in seconds that the worker | > | | thread has spent waiting for work from | > | | co-ordinator thread | > | | | > |LAST_TRANS_RETRY_COUNT | Total number of retries attempted by last | > | | transaction | > --------------------------------------------------------------------------- > Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] a73681e1f03: MDEV-20220: Merge 5.7 P_S replication table 'replication_applier_status_by_worker
by Sergei Golubchik 01 Apr '21

01 Apr '21

Hi, Sujatha! thanks! ok to push On Apr 01, Sujatha wrote: > revision-id: a73681e1f03 (mariadb-10.5.2-529-ga73681e1f03) > parent(s): bd43f39bd5a > author: Sujatha <sujatha.sivakumar(a)mariadb.com> > committer: Sujatha <sujatha.sivakumar(a)mariadb.com> > timestamp: 2021-03-29 17:27:54 +0530 > message: > > MDEV-20220: Merge 5.7 P_S replication table 'replication_applier_status_by_worker > > Step1: > ===== > Backport 'replication_applier_status_by_worker' from upstream. > > Iterate through rpl_parallel_thread_pool and display slave worker thread > specific information as part of 'replication_applier_status_by_worker' > table. > > --------------------------------------------------------------------------- > |Column Name: | Description: | > |-------------------------------------------------------------------------| > | | | > |CHANNEL_NAME | Name of replication channel through which the | > | | transaction is received. | > | | | > |THREAD_ID | Thread_Id as displayed in 'performance_schema. | > | | threads' table for thread with name | > | | 'thread/sql/rpl_parallel_thread' | > | | | > | | THREAD_ID will be NULL when worker threads are | > | | stopped due to an error/force stop | > | | | > |SERVICE_STATE | Thread is running or not | > | | | > |LAST_SEEN_TRANSACTION | Last GTID executed by worker | > | | | > |LAST_ERROR_NUMBER | Last Error that occured on a particular worker | > | | | > |LAST_ERROR_MESSAGE | Last error specific message | > | | | > |LAST_ERROR_TIMESTAMP | Time stamp of last error | > | | | > --------------------------------------------------------------------------- Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] b530cc73ddb: MDEV-22010: use executables MariaDB named in scripts
by Sergei Golubchik 01 Apr '21

01 Apr '21

Hi, Rucha! Looks almost perfect! a couple of comments about mysqld_multi.sh below On Apr 01, Rucha Deodhar wrote: > revision-id: b530cc73ddb (mariadb-10.5.2-386-gb530cc73ddb) > parent(s): de407e7cb4d > author: Rucha Deodhar <rucha.deodhar(a)mariadb.com> > committer: Rucha Deodhar <rucha.deodhar(a)mariadb.com> > timestamp: 2021-03-23 01:42:03 +0530 > message: > > MDEV-22010: use executables MariaDB named in scripts > > As a part of this MDEV following changes were made: > 1) Mariadb named executables used instead of mysql named executables in scripts > 2) renamed mysql-test-run and mysql-stress-test to mariadb-test-run and > mariadb-stress-test and created a symlink. > > diff --git a/scripts/mysqld_multi.sh b/scripts/mysqld_multi.sh > index 0b33f61710e..191caf22454 100644 > --- a/scripts/mysqld_multi.sh > +++ b/scripts/mysqld_multi.sh > @@ -353,7 +353,7 @@ sub start_mysqlds() > $tmp.= " $options[$j]"; > } > } > - if ($opt_verbose && $com =~ m/\/(safe_mysqld|mysqld_safe)$/ && !$info_sent) > + if ($opt_verbose && $com =~ m/\/(mariadb_safe|mariadbd-safe)$/ && !$info_sent) this doesn't make much sense. there was never a mariadb_safe tool, was it? if there was not, then you don't need to search for it, just write m/\/mariadbd-safe$/ although I'd rather written m[/mariadbd-safe$] - if / is not used as a delimiter, there's no need to escape it in a regex. > { > print "WARNING: $1 is being used to start mysqld. In this case you "; here it should, probably, say "to start mariadbd" ? > print "may need to pass\n\"ledir=...\" under groups [mysqldN] to "; > @@ -667,14 +667,14 @@ sub example > # > # 2.PID-FILE > # > -# If you are using mysqld_safe to start mysqld, make sure that every > -# MariaDB server has a separate pid-file. In order to use mysqld_safe > +# If you are using mariadbd-safe to start mariadbd, make sure that every > +# MariaDB server has a separate pid-file. In order to use mariadbd-safe > # via $my_progname, you need to use two options: > # > -# mysqld=/path/to/mysqld_safe > +# mysqld=/path/to/mariadbd-safe > # ledir=/path/to/mysqld-binary/ and here /path/to/mariadbd-binary/ ? > # > -# ledir (library executable directory), is an option that only mysqld_safe > +# ledir (library executable directory), is an option that only mariadbd-safe > # accepts, so you will get an error if you try to pass it to mysqld directly. > # For this reason you might want to use the above options within [mysqld#] > # group directly. > @@ -741,9 +741,9 @@ language = @datadir@/mysql/english > user = unix_user1 > > [mysqld3] > -mysqld = /path/to/mysqld_safe > +mysqld = /path/to/mariadbd-safe > ledir = /path/to/mysqld-binary/ and here > -mysqladmin = /path/to/mysqladmin > +mysqladmin = /path/to/mariadb-admin > socket = /tmp/mysql.sock3 > port = 3308 > pid-file = @localstatedir@3/hostname.pid3 Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

1 0

Re: [Maria-developers] JSON_TABLE: On CTE question
by Sergey Petrunia 01 Apr '21

01 Apr '21

Hello Igor, with recursive cte1 as ( select * from t1, json_table(t1.item_props,'$' columns( color varchar(100) path '$.color')) as t ), cte2 as ( select * from (select 1 as n from dual) x, cte1 union all select cte2.n+1, cte2.item_name, cte2.item_props, cte2.color from cte2, json_table(cte2.item_props,'$' columns( color varchar(100) path '$.color')) as t where t.item_name=cte2.item_name and cte2.n < 3 ) select * from cte2; ERROR 1054 (42S22): Unknown column 't.item_name' in 'where clause' with recursive cte1 as ( select * from t1, json_table(t1.item_props,'$' columns( color varchar(100) path '$.color')) as t ), cte2 as ( select * from (select 1 as n from dual) x, cte1 union all select cte2.n+1, cte2.item_name, cte2.item_props, cte2.color from cte2, cte1 as t where t.item_name=cte2.item_name and cte2.n < 3 ) select * from cte2; +------+-----------+-----------------------------------+-------+ | n | item_name | item_props | color | +------+-----------+-----------------------------------+-------+ | 1 | Laptop | {"color": "black", "price": 1000} | black | | 1 | Jeans | {"color": "blue", "price": 50} | blue | | 2 | Laptop | {"color": "black", "price": 1000} | black | | 2 | Jeans | {"color": "blue", "price": 50} | blue | | 3 | Laptop | {"color": "black", "price": 1000} | black | | 3 | Jeans | {"color": "blue", "price": 50} | blue | +------+-----------+-----------------------------------+-------+ Igor> It means that json table t does not inherit the column name item_name from cte2. I'm not sure what do you mean by "inherit"? json_table t has one column named "color". Could you elaborate? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 0

[Maria-developers] JSON_TABLE: On name resolution question
by Sergey Petrunia 01 Apr '21

01 Apr '21

Hello Igor, I was looking at the example you've posted earlier this week on Slack: Igor> @holyfoot, @spetrunia: here' serious problem with identification for JSON_TABLEs: create table t1 (item_name varchar(32), item_props varchar(1024)); insert into t1 values ('Laptop', '{"color": "black", "price": 1000}'); insert into t1 values ('Jeans', '{"color": "blue", "price": 50}'); create table t2 (item_name varchar(32), item_props varchar(1024)); insert into t2 values ('Laptop', '{"color": "black", "price": 1200}'); insert into t2 values ('Shirt', '{"color": "blue", "price": 20}'); MariaDB [test]> select * from t1 as t2 where item_name not in (select item_name from t2 as s, json_table(t2.item_props,'$' columns( color varchar(100) path '$.color')) as t); +-----------+--------------------------------+ | item_name | item_props | +-----------+--------------------------------+ | Jeans | {"color": "blue", "price": 50} | +-----------+--------------------------------+ Igor> t2.item_props should not be valid, because it refers to t2 as s. Let me format the query: select * from t1 as t2 where item_name not in (select item_name from t2 as s, json_table(t2.item_props, '$' columns( color varchar(100) path '$.color') ) as t ); My reply is: "t2.item_props" does NOT refer to (t2_as_s).item_props. It refers to the top level select's (t1_as_t2).item_props. Proof: let's rename the t1-as-t2 in the top-level select: select * from t1 as t2qqqqqqqqq where item_name not in (select item_name from t2 as s, json_table(t2.item_props, '$' columns( color varchar(100) path '$.color') ) as t ); ERROR 1054 (42S22): Unknown column 't2.item_props' in 'JSON_TABLE argument' Igor> This easily can be seen after adding MariaDB [test]> insert into t2 values ('Jeans', '{"color": "black", "price": 20}'); Query OK, 1 row affected (0.004 sec) MariaDB [test]> select * from t1 as t2 where item_name not in (select item_name from t2 as s, json_table(t2.item_props,'$' columns( color varchar(100) path '$.color')) as t); I get an empty set for this query. Is there any issue? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://petrunia.net

1 0

Re: [Maria-developers] f46382ba93c: Optimize usage of c_ptr(), c_ptr_quick() and String::alloc()
by Sergei Golubchik 01 Apr '21

01 Apr '21

Hi, Michael! On Mar 27, Michael Widenius wrote: > revision-id: f46382ba93c (mariadb-10.5.2-515-gf46382ba93c) > parent(s): c157c285db9 > 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 usage of c_ptr(), c_ptr_quick() and String::alloc() > > The proble was that hen using String::alloc() to allocate a string, "hen" ? after a minute of staring I realized that you probably meant "when". (also, "problem") > the String ensures that there is space for an extra NULL byte in the > buffer and if not, reallocates the string. This is a problem with the > String::set_int() that calls alloc(21), which forces > an extra malloc/free to happen. > > - We do not anymore re-allocate String if alloc() is called with the > Allocated_length. This reduces number of malloc() allocations, > especially one big re-allocation in Protocol::send_result_Set_metadata() > for almost every query that produced a result to the connnected client. > - Avoid extra mallocs when using LONGLONG_BUFFER_SIZE > This can now be done as alloc() doesn't increase buffers if new length is > not bigger than old one. > - c_ptr() is redesigned to be safer (but a bit longer) than before. > - Remove wrong usage of c_ptr_quick() > c_ptr_quick() where used in many cases to get the pointer to the used s/where/was/ > buffer, even when it didn't need to be \0 terminated. In this case > ptr() is a better substitute. > Another problem with c_ptr_quick() is that it didn't guarantee that > the string would be \0 terminated. > - item_val_str(), an API function not used currently by the server, > now always returns a null terminated string (before it didn't always > do that). > - Ensure that all String allocations uses STRING_PSI_MEMORY_KEY. The old > mixed usage of performance keys caused assert's when String buffers > where shrunk. > - Binary_string::shrink() is simplifed > > diff --git a/sql/item.cc b/sql/item.cc > index f6f3e2720fa..a8a43c6266a 100644 > --- a/sql/item.cc > +++ b/sql/item.cc > @@ -4703,7 +4703,7 @@ Item *Item_param::value_clone_item(THD *thd) > return 0; // Should create Item_decimal. See MDEV-11361. > case STRING_RESULT: > return new (mem_root) Item_string(thd, name, > - Lex_cstring(value.m_string.c_ptr_quick(), > + Lex_cstring(value.m_string.ptr(), Hmm, you said that LEX_CSTRING::str should always be \0-terminated. If that's the case, then c_ptr() would be correct here, not ptr() > value.m_string.length()), > value.m_string.charset(), > collation.derivation, > diff --git a/sql/partition_info.cc b/sql/partition_info.cc > index 871411cf6c4..b3e1bceec31 100644 > --- a/sql/partition_info.cc > +++ b/sql/partition_info.cc > @@ -126,7 +126,7 @@ partition_info *partition_info::get_clone(THD *thd) > /** > Mark named [sub]partition to be used/locked. > > - @param part_name Partition name to match. > + @param part_name Partition name to match. Must be \0 terminated! > @param length Partition name length. > > @return Success if partition found > @@ -172,9 +172,9 @@ bool partition_info::add_named_partition(const char *part_name, size_t length) > else > bitmap_set_bit(&read_partitions, part_def->part_id); > } > - DBUG_PRINT("info", ("Found partition %u is_subpart %d for name %s", > + DBUG_PRINT("info", ("Found partition %u is_subpart %d for name %.*s", > part_def->part_id, part_def->is_subpart, > - part_name)); > + length, part_name)); These two changes contradict each other. Either part_name must be \0-terminated, and then you don't need to specify a length in printf. Or it is not \0-terminated and you must specify a length. > DBUG_RETURN(false); > } > > diff --git a/sql/sql_string.cc b/sql/sql_string.cc > index f2a0f55aec8..95a57017c53 100644 > --- a/sql/sql_string.cc > +++ b/sql/sql_string.cc > @@ -118,9 +121,14 @@ bool Binary_string::realloc_raw(size_t alloc_length) > return FALSE; > } > > + > bool String::set_int(longlong num, bool unsigned_flag, CHARSET_INFO *cs) > { > - uint l=20*cs->mbmaxlen+1; > + /* > + This allocates a few bytes extra in the unlikely case that cs->mb_maxlen > + > 1, but we can live with that dunno, it seems that utf8 is the *likely* case and it's getting more and more likely with the time, > + */ > + uint l= LONGLONG_BUFFER_SIZE * cs->mbmaxlen; > int base= unsigned_flag ? 10 : -10; > > if (alloc(l)) > @@ -1254,21 +1262,16 @@ bool String::append_semi_hex(const char *s, uint len, CHARSET_INFO *cs) > // Shrink the buffer, but only if it is allocated on the heap. > void Binary_string::shrink(size_t arg_length) > { > - if (!is_alloced()) > - return; > - if (ALIGN_SIZE(arg_length + 1) < Alloced_length) > + if (is_alloced() && ALIGN_SIZE(arg_length + 1) < Alloced_length) > + { > + char *new_ptr; > + /* my_realloc() can't fail as new buffer is less than the original one */ > + if ((new_ptr= (char*) my_realloc(STRING_PSI_MEMORY_KEY, Ptr, arg_length, you don't need an if() because, as you wrote yourself "my_realloc() can't fail" here. > + MYF(thread_specific ? > + MY_THREAD_SPECIFIC : 0)))) > { > diff --git a/sql/sql_string.h b/sql/sql_string.h > index b3eca118b63..ba0cff68fb4 100644 > --- a/sql/sql_string.h > +++ b/sql/sql_string.h > @@ -600,25 +600,34 @@ class Binary_string: public Static_binary_string > > inline char *c_ptr() > { > - DBUG_ASSERT(!alloced || !Ptr || !Alloced_length || > - (Alloced_length >= (str_length + 1))); > - > - if (!Ptr || Ptr[str_length]) // Should be safe > - (void) realloc(str_length); > + if (unlikely(!Ptr)) > + return (char*) ""; > + /* > + Here we assume that any buffer used to initalize String has > + an end \0 or have at least an accessable character at end. > + This is to handle the case of String("Hello",5) efficently. > + */ > + if (unlikely(!alloced && !Ptr[str_length])) > + return Ptr; No, this is not good. Note the difference between String a("Hello", 5) and char hello[5]; String a(buf, 5); Your assumption should only apply to the first case, not to the second. In the first case alloced=Alloced_length=0, in the second case only alloced=0 and Alloced_length=5. So in the if() above you need to look at Alloced_length: if (!Alloced_length && !Ptr[str_length]) return Ptr; > + if (str_length < Alloced_length) > + { > + Ptr[str_length]=0; > + return Ptr; > + } > + (void) realloc(str_length+1); /* This will add end \0 */ > return Ptr; > } > + /* Use c_ptr() instead. This will be deleted soon, kept for compatiblity */ > inline char *c_ptr_quick() > { > - if (Ptr && str_length < Alloced_length) > - Ptr[str_length]=0; > - return Ptr; > + return c_ptr_safe(); 1. why not to remove it now? 2. it's strange that you write "use c_ptr() instead" but you actually use c_ptr_safe() instead. > } > inline char *c_ptr_safe() > { > if (Ptr && str_length < Alloced_length) > Ptr[str_length]=0; > else > - (void) realloc(str_length); > + (void) realloc(str_length + 1); > return Ptr; > } could you write a comment, explaining when one should use c_ptr() and when - c_ptr_safe() ? Regards, Sergei VP of MariaDB Server Engineering and security(a)mariadb.org

2 7