May 2019 - developers - lists.mariadb.org

Re: [Maria-developers] ef2519fee4e: MDEV-16546 System versioning setting to allow history modification
by Aleksey Midenkov 08 Jun '21

08 Jun '21

Hello, Sergei! On Fri, May 3, 2019 at 8:43 PM Sergei Golubchik <serg(a)mariadb.com> wrote: > > Hi, Aleksey! > > On May 03, Aleksey Midenkov wrote: > > revision-id: ef2519fee4e (versioning-1.0.5-17-gef2519fee4e) > > parent(s): 56145be2951 > > author: Aleksey Midenkov <midenok(a)gmail.com> > > committer: Aleksey Midenkov <midenok(a)gmail.com> > > timestamp: 2018-06-28 13:42:09 +0300 > > message: > > > > MDEV-16546 System versioning setting to allow history modification > > > > 1. Add server variable system_versioning_modify_history which will > > allow to set values for row_start, row_end in DML operations. > > > > 2. If secure_timestamp is YES or REPLICATION, > > system_versioning_modify_history does not have effect. If > > secure_timestamp is SUPER, system_versioning_modify_history requires > > special privilege (same as for setting current timestamp). > > I thought more about this idea. We don't really want to have the history > editable, do we? Well, I'm thinking about rollback table data to specific point in time. That could be a useful feature. > But it's needed for replication, to keep the master and > slave identical. That's what secure_timestamp is for. > > The idea was that this new variable, system_versioning_modify_history, > will be just a convenience feature, it will not allow history editing > any more than one can do without it. > > But now I suspect that even with secure_timestamp=NO one cannot truly > edit history. One can only insert new rows with arbitrary timestamps. > For example, to insert a row with row_start=1000 and row_end=2000, one > needs to do (if secure_timestamp=NO): > > set timestamp=1000; > insert row; > set timestamp=2000; > delete row; > > But I don't see how one can update or delete a history row with > secure_timestamp=NO. > > Now, with a SUPER privilege and secure_timestamp=NO or SUPER, one can > use the BINLOG command and truly edit the history arbitrarily, by faking > row events. I don't really get it why this is so important: since there is some limitation by configuration and privilege, we are just fine. Everything can be changed at filesystem level after all. > > The conclusion, I believe, is that system_versioning_modify_history > should allow INSERTs when secure_timestamp=NO, and it should allow > UPDATE/DELETE only for a SUPER user when secure_timestamp=NO or SUPER. I don't see a reason to argue on that. The only thing that is not clear, why we don't allow INSERTs when secure_timestamp=SUPER? > > The second thing I don't like at all, is when a table is created like > > CREATE TABLE t1 (a int) WITH SYSTEM VERSIONING > > with row_start/row_end implicit. You don't have it in the test, but > anyway one should be able to load history into such a table, while the > table does not have row_start and row_end columns. From the user point > of view these columns don't exist, they're pseudo-columns, like ROWID. > They just cannot be insertable-into, conceptually. But a user will want > to restore the history, right? I don't have a solution for this yet :( > Any ideas? We don't have to follow the conception if it doesn't help us. Since we have physical row_start/row_end, we don't have to pretend they don't exist. Who will win from that? > > See below a couple of minor comments about the patch itself. > > ... These are going to be fixed. > > Regards, > Sergei > Chief Architect MariaDB > and security(a)mariadb.org -- All the best, Aleksey Midenkov @midenok

2 15

[Maria-developers] Please review MDEV-18319 BIGINT UNSIGNED Performance issue
by Alexander Barkov 29 Oct '19

29 Oct '19

Hi Sergei, Please review a patch for MDEV-18319 https://github.com/MariaDB/server/commit/8a990ad17746927c6d395ec755a262eda5… Thanks

2 1

[Maria-developers] Windows line endings in MariaDB sources again (
by Otto Kekäläinen 24 Jul '19

24 Jul '19

Hello! While trying to release 10.0.26 for Debian and Ubuntu I noticed git chokes again on Windows line endings. As our .gitattributes states that by default git should clean up the line endings, why do we keep getting new Windows line endings in new code? mariadb-10.0$ cat .gitattributes # Set the default behavior, in case people don't have core.autocrlf set. * text=auto ... storage/connect/mysql-test/connect/std_data/*.txt -text mariadb-10.0.26$ find . | xargs file | grep CRLF ./storage/connect/mysql-test/connect/std_data/boyswin.txt: ASCII text, with CRLF line terminators ./storage/connect/mysql-test/connect/std_data/expenses.txt: ASCII text, with CRLF line terminators ./storage/connect/mysql-test/connect/std_data/emp.txt: ASCII text, with CRLF line terminators ./mysql-test/std_data/loaddata7.dat: ASCII text, with CRLF line terminators ./mysql-test/r/loadxml.result: ASCII text, with CRLF, LF line terminators ./mysql-test/r/perror-win.result: ASCII text, with CRLF, LF line terminators ./mysql-test/r/mysql_binary_mode.result: ASCII English text, with CRLF, LF line terminators ./mysql-test/r/func_regexp_pcre.result: UTF-8 Unicode C++ program text, with CRLF, CR, LF line terminators ./pcre/testdata/grepoutputN: ASCII text, with CRLF, CR, LF line terminators ./pcre/testdata/greppatN4: ASCII text, with CRLF line terminators This leads for me to: dpkg-source: info: building mariadb-10.0 using existing ./mariadb-10.0_10.0.26.orig.tar.gz dpkg-source: info: local changes detected, the modified files are: mariadb-10.0/storage/connect/mysql-test/connect/std_data/boyswin.txt mariadb-10.0/storage/connect/mysql-test/connect/std_data/emp.txt mariadb-10.0/storage/connect/mysql-test/connect/std_data/expenses.txt dpkg-source: error: aborting due to unexpected upstream changes, see /tmp/mariadb-10.0_10.0.26-1.diff.Yzwwhw Which means I have to do extra work with importing the upstream sources.. - Otto

2 4

[Maria-developers] GSoC [MDEV-6017]
by Alexey Mogilyovkin 10 Jun '19

10 Jun '19

Hello. This is the thread where I would like to publish my weekly reports every Monday. During preparatory weeks I set up development environment. I briefly examined testing framework, debugging process and now I am ready to start work. Work progress will be available on my github repository in branch "MDEV-6017" https://github.com/ZeroICQ/server/tree/MDEV-6017

3 5

Re: [Maria-developers] [Commits] b3473961810: MDEV-11094: Blackhole table updates on slave fail when row annotation is enabled
by Sergey Vojtovich 29 May '19

29 May '19

Hi Sujatha, On Wed, May 29, 2019 at 04:37:27PM +0530, sujatha wrote: > revision-id: b347396181018cedc946450cb49891f1a0aa4575 (mariadb-10.1.39-48-gb3473961810) > parent(s): 8358c6f03edb941be488f009c2bd0eb9df47e8c5 > author: Sujatha > committer: Sujatha > timestamp: 2019-05-29 15:18:52 +0530 > message: > > MDEV-11094: Blackhole table updates on slave fail when row annotation is enabled ...skip... > diff --git a/storage/blackhole/ha_blackhole.cc b/storage/blackhole/ha_blackhole.cc > index 01aaa9ea15f..43bcdc541a1 100644 > --- a/storage/blackhole/ha_blackhole.cc > +++ b/storage/blackhole/ha_blackhole.cc > @@ -25,6 +25,16 @@ > #include "ha_blackhole.h" > #include "sql_class.h" // THD, SYSTEM_THREAD_SLAVE_SQL > > +static bool is_row_based_replication(THD *thd) > +{ > + /* > + A row event which has its thd->query() == NULL or a row event which has > + replicate_annotate_row_events enabled. In the later case the thd->query() > + will be pointing to the query, received through replicated annotate event > + from master. > + */ > + return ((thd->query() == NULL) || thd->variables.binlog_annotate_row_events); Any reason to have so many redundant parenthesis? Why code comment rather than function comment? May be move "thd->system_thread == SYSTEM_THREAD_SLAVE_SQL" inside as well? Regards, Sergey

1 0

[Maria-developers] Proposal for automatic code formatting (clang-format)
by Eugene Kosov 27 May '19

27 May '19

Hi. I'm proposing to add some tool that automatically formats source code. Main purpose is overall productivity. It's possible to save time on manual formatting. Also, reading a uniformly formatted code is easier. Some new languages has one correct coding style and code formatters. Examples are Go (https://golang.org/cmd/gofmt/) and (to be done) Rust (https://github.com/rust-dev-tools/fmt-rfcs/blob/master/guide/guide.md) I suggest to read 'Motivation - why use a formatting tool' from second link. There is no single correct coding style in C and C++ worlds: different projects use different styles if any at all. But there is a tool which does formatting and supports different styles: clang-format. It's a binary available for every major platform. It's configurable: just put .clang-format file with formatting options in source directory and formatter will use it for current and nested directories. I actually do not propose here to use exactly clang-format but as I know it's the best of its kind. Here is a demo https://youtu.be/uZI_Qla4pNA?t=1893 Tool is configurable but it will not act exactly as you might expect and this is a pay for uniformity. So, let's add .clang-format to root directory, start using and polishing style. Here is some info on how to add it to some text editors https://clang.llvm.org/docs/ClangFormat.html I do not(!) suggesting to reformat whole project. At least for now. Here is my PR to add .clang-format files to server and InnoDB: https://github.com/MariaDB/server/pull/1270 I think that eventually .clang-format usage will lead to reformatting everything and having some project-wide precommit hook which will properly format your diffs. Do you have any objections or questions on having such a tool? Let's discuss, please. -- Eugene

1 0

Re: [Maria-developers] Regarding approach used for Early NULL filtering in the range optimizer(MDEV-15777)
by Sergey Petrunia 27 May '19

27 May '19

On Fri, May 24, 2019 at 04:39:51PM -0700, Igor Babaev wrote: > On 05/24/2019 01:41 PM, Sergey Petrunia wrote: > > On Fri, May 24, 2019 at 10:08:04AM -0700, Igor Babaev wrote: > > > On 05/24/2019 04:03 AM, Varun Gupta wrote: > > > > Hi Igor, > > > > After discussing with Sergey, we came up with these conclusions as to > > > > why we used the approach of going through all the keyuses in the KEYUSE > > > > array > > > > > > > > Cases to consider: > > > > we have an index on column a > > > > > > > > 1) a OP const > > > > where OP can be (</<=/=/between/ etc on which we do range analysis) . > > > > This case is already handled by the range optimizer. We do not create a > > > > NULL rejecting predicate for such conditions. > > > > > > > > 2) eq_join conditions (tbl1.a = tbl2.col2) > > > > This is the specific case we have tried to implement in MDEV-15777, we > > > > create NULL rejecting predicates for the keyuses for each table and then > > > > feed these predicates to range optimizer > > > There is no logical explanation for checking null rejection of fields used > > > in equalities with the help of array keyuse. > > The logic here is as follows: > > > > We already collect the set of KEYUSE::null_rejecting attributes. Its > > meaning is very close to what the patch needs. > The meaning of this argument is to be used a key value for ref access by an > index i1. > You need range access by a different index i2. There is no logical > connection > between these two indexes. The usage of i1 can be blocked by IGNORE clause > and this case > there won't be any keyuses for it. But the equality from which the > null-rejection can be > deduced is still there. You need range scan to reduce the number of records > produced > by the left operand of a join operation. And you should not care weather it > should be the > argument of any ref access. > Here is the situation: > The field of interest for range access is t1.a and you have two conditions > t1.a=t2.a with index i1 on t2.a > t1.b=t3.b with index i2 on t3.b > The use prohibited to use i1. t1.a apparently remains null-rejecting. Yet it > cannot be not detected > as such by a look through array of keyuses. I don't follow the logic. What MDEV-15777 does is to basically look at KEYUSE object, which represent tbl.key_col = other_column and feed this into the range optimizer: tbl.key_col IS NOT NULL If the query CAN use a certain index: - we will have a KEYUSE object for it - we will feed the IS NOT NULL into the range optimizer. if the query CANNOT use a certain index (because of IGNORE INDEX), then - we won't have KEYUSE object for that index (and so won't construct an IS NOT NULL condition for it) - range optimizer will not build ranges for it (so it doesn't really matter if IS NOT NULL was constructed or not) Can you provide a full example? > > > > Because of this, the part of the patch for MDEV-15777 which analyzes the KEYUSE > > array is very small: 122 lines including the comment (I counted > > make_null_rejecting_conds() and add_cond()). > > > > I think, the CPU overhead is small, too. > > > > > > 3) a < func(b,c) > > > > we do not handle this case, because: > > > > > > > > a) It is harder to process. We would need to process the whole WHERE > > > > clause to infer > > > > non-nullability of the condition. Example: > > > > > > > > (t1.f2+1 < t1.f1+t1.f3) OR ... OR t1.f1 is null > > > > > > > > here the left part of the OR condition doesn't allow f1 to be NULL, but > > > > the > > > > right condition allows it. We would also need to take into account tables > > > > inside outer joins. We would need to go through Item classes and add code > > > > which say "Item_xxx() will compute to false when its argument Y is null" > > > > (there is item->not_null_tables() currently, but not not_null_columns()). > > > Have you actually looked at the implementations of the virtual function > > > not_null_tables()? > > > Do you need me to provide the implementation of the virtual function > > > not_null_columns()? > > It will require multiple implementations. At the moment we have: > > > > nm mysqld --demangle | grep 'Item.*::not_null_tables' | wc -l > > 21 > > > > Another question: do you intend to collect not_null_columns() for all columns, > > or just columns that are a part of some index? > > > > What would a good data structure to store a set of tablename.column_name ? > It will be just a bitmap created for each table that can be used for many > other purposes. > If you want to filter out those that are not part of any index it can be > easily done > (even once for any table share) My point was that constructing such a bitmap is a lot heavier than the not_null_table_map(). table.h declares a field bitmap class: typedef Bitmap<MAX_FIELDS> Field_map; but it's huge: (gdb) p sizeof(Field_map) $52 = 544 and note that we will not be able to just put one Field_map into the TABLE object: If we think about how to compute not_null_columns() for Item_cond_and or Item_cond_or: - Item_cond_and will compute a union of all its arguments' not_null_columns(). - Item_cond_or will compute an intersection of all its arguments' not_null_columns(). - Other items have different logic. that is, we will need to keep multiple "set of fields in tables" objects around. Just one bitmap attached to the table will not suffice. This is doable, but it's going to be more code and have more CPU overhead. And I'm still not convinced why we should do this. It is not clear if this will fit the scope of MDEV-15777 and once we got it done, what's the use of it? Is this something other query optimizers do (so the users expect it?)? > > > > > > b) the conditions in form of arbitrary functions are not as frequently > > > > used as > > > > ref access conditions. > > > > > > > > to sum up a) and b) - doable but will require a lot of effort. > > > > > > > > Do you have any specific practically relevant example in mind that we > > > > should > > > > handle? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

Re: [Maria-developers] [Commits] 53d0e921794: MDEV-11094: Blackhole table updates on slave fail when row annotation is enabled
by andrei.elkin＠pp.inet.fi 27 May '19

27 May '19

Sujatha, howdy. Thank you for spelling it out for me! > Hello Andrei, ... >>> >>> Analysis: >>> ========= >>> Enabling "replicate_annotate_row_events" on slave, Tells the slave to write >>> annotate rows events received from the master to its own binary log. The >>> received annotate events are applied after the Gtid event as shown below. >>> thd->query() will be set to the actual query received from the master, through >>> annotate event. Annotate_rows event should not be deleted after the event is >>> applied as the thd->query will be used to generate new Annotate_rows event >>> during applying the subsequent Rows events. >> [ here ] >> The Table_map and Rows_log_event that follow do not touch thd->query(). > > The Table_map and Rows_log_event that follow "Annotate_rows" event do > make use of "thd->query()". Frankly I did not expect that. Perhaps it would have not been a bad idea to make Annotate_rows_log_event::do_apply_event() logging of itself, which still makes sense even if the slave applier will filter out its group's Rows_log_event:s so in this case the slave side event group would consists of mere Annotate. The above is a mere remark though, I have to accept what we have. >> >> I also suggest to check out mysqlbinlog run. There seems to be no test >> for that though, so one needs to be written. The goal would be to prove >> a created binlog output is as expected (e.g to include Annotate). > > > Sure, I will add a test case. Super! The patch is approved then. Cheers, Andrei

1 0

Re: [Maria-developers] [Commits] 42129dae63c: MDEV-18741: Optimizer trace: multi-part key ranges are printed incorrectly
by Sergey Petrunia 27 May '19

27 May '19

Hi Varun, One cosmetic comment below. Ok to push after it is addressed. On Fri, May 24, 2019 at 03:51:00PM +0530, Varun wrote: > revision-id: 42129dae63c4331b01f9a504c9f3ef1c0c1f857a (mariadb-10.4.4-126-g42129dae63c) > parent(s): 592fe954ef82be1bc08b29a8e54f7729eb1e1343 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2019-05-24 14:43:44 +0530 > message: > > MDEV-18741: Optimizer trace: multi-part key ranges are printed incorrectly > > Changed the function append_range_all_keyparts to use sel_arg_range_seq_init / sel_arg_range_seq_next to produce ranges. > Also adjusted to print format for the ranges, now the ranges are printed as: > (keypart1_min, keypart2_min,..) OP (keypart1_name,keypart2_name, ..) OP (keypart1_max,keypart2_max, ..) > > Also added more tests for range and index merge access for optimizer trace ... > diff --git a/sql/opt_range_mrr.cc b/sql/opt_range_mrr.cc > index 3a25da3edb2..5644e456dd4 100644 > --- a/sql/opt_range_mrr.cc > +++ b/sql/opt_range_mrr.cc > @@ -53,6 +53,8 @@ typedef struct st_sel_arg_range_seq > int i; /* Index of last used element in the above array */ > > bool at_start; /* TRUE <=> The traversal has just started */ > + /* TRUE if last checked tree->key can be used for ROR-scan */ > + bool is_ror_scan; > } SEL_ARG_RANGE_SEQ; > > This comment made sense at the previous location, but here it doesn't. Here the meaning is something like "Iteration functions will set this to FALSE" if ranges being traversed do not allow to construct a ROR-scan". BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] GSoC Introduction
by Rucha Deodhar 27 May '19

27 May '19

Hello, I am Rucha Deodhar. I am a GSoC student and will implement INSERT...RETURNING. In the first week I plan to extend the parser. Approximately the next 2 months, I plan to make changes the in the codes in insert.cc and other files related to the project, and remaining time for testing. As suggested, I am allotting 40% of the time for testing.This is my approximate plan, Regards, Rucha.

2 1

Re: [Maria-developers] Regarding approach used for Early NULL filtering in the range optimizer(MDEV-15777)
by Sergey Petrunia 24 May '19

24 May '19

On Fri, May 24, 2019 at 10:08:04AM -0700, Igor Babaev wrote: > On 05/24/2019 04:03 AM, Varun Gupta wrote: > > Hi Igor, > > After discussing with Sergey, we came up with these conclusions as to > > why we used the approach of going through all the keyuses in the KEYUSE > > array > > > > Cases to consider: > > we have an index on column a > > > > 1) a OP const > > where OP can be (</<=/=/between/ etc on which we do range analysis) . > > This case is already handled by the range optimizer. We do not create a > > NULL rejecting predicate for such conditions. > > > > 2) eq_join conditions (tbl1.a = tbl2.col2) > > This is the specific case we have tried to implement in MDEV-15777, we > > create NULL rejecting predicates for the keyuses for each table and then > > feed these predicates to range optimizer > There is no logical explanation for checking null rejection of fields used > in equalities with the help of array keyuse. The logic here is as follows: We already collect the set of KEYUSE::null_rejecting attributes. Its meaning is very close to what the patch needs. Because of this, the part of the patch for MDEV-15777 which analyzes the KEYUSE array is very small: 122 lines including the comment (I counted make_null_rejecting_conds() and add_cond()). I think, the CPU overhead is small, too. > > > > 3) a < func(b,c) > > we do not handle this case, because: > > > > a) It is harder to process. We would need to process the whole WHERE > > clause to infer > > non-nullability of the condition. Example: > > > > (t1.f2+1 < t1.f1+t1.f3) OR ... OR t1.f1 is null > > > > here the left part of the OR condition doesn't allow f1 to be NULL, but > > the > > right condition allows it. We would also need to take into account tables > > inside outer joins. We would need to go through Item classes and add code > > which say "Item_xxx() will compute to false when its argument Y is null" > > (there is item->not_null_tables() currently, but not not_null_columns()). > Have you actually looked at the implementations of the virtual function > not_null_tables()? > Do you need me to provide the implementation of the virtual function > not_null_columns()? It will require multiple implementations. At the moment we have: nm mysqld --demangle | grep 'Item.*::not_null_tables' | wc -l 21 Another question: do you intend to collect not_null_columns() for all columns, or just columns that are a part of some index? What would a good data structure to store a set of tablename.column_name ? > > > > b) the conditions in form of arbitrary functions are not as frequently > > used as > > ref access conditions. > > > > to sum up a) and b) - doable but will require a lot of effort. > > > > Do you have any specific practically relevant example in mind that we > > should > > handle? BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] Regarding approach used for Early NULL filtering in the range optimizer(MDEV-15777)
by Varun Gupta 24 May '19

24 May '19

Hi Igor, After discussing with Sergey, we came up with these conclusions as to why we used the approach of going through all the keyuses in the KEYUSE array Cases to consider: we have an index on column a 1) a OP const where OP can be (</<=/=/between/ etc on which we do range analysis) . This case is already handled by the range optimizer. We do not create a NULL rejecting predicate for such conditions. 2) eq_join conditions (tbl1.a = tbl2.col2) This is the specific case we have tried to implement in MDEV-15777, we create NULL rejecting predicates for the keyuses for each table and then feed these predicates to range optimizer 3) a < func(b,c) we do not handle this case, because: a) It is harder to process. We would need to process the whole WHERE clause to infer non-nullability of the condition. Example: (t1.f2+1 < t1.f1+t1.f3) OR ... OR t1.f1 is null here the left part of the OR condition doesn't allow f1 to be NULL, but the right condition allows it. We would also need to take into account tables inside outer joins. We would need to go through Item classes and add code which say "Item_xxx() will compute to false when its argument Y is null" (there is item->not_null_tables() currently, but not not_null_columns()). b) the conditions in form of arbitrary functions are not as frequently used as ref access conditions. to sum up a) and b) - doable but will require a lot of effort. Do you have any specific practically relevant example in mind that we should handle? Regards, Varun and Sergey

1 0

[Maria-developers] Review for: b4696ee97e684 : MDEV-15777: Support Early NULLs filtering-like restrictions in the range optimizer
by Sergey Petrunia 23 May '19

23 May '19

Hi Varun, Please find review input below. Only cosmetic changes are requested. I also assume that after this is pushed - we add a optimizer trace printout into 10.4 - the feature is enabled by default in 10.5 ? (do we need separate MDEVs for this?) One final question is the choice of name for @@optimizer_switch flag. Let me get back to you on this. > commit b4696ee97e6846ba49ef203cfc7189f50c1e53a7 > Author: Varun Gupta <varun.gupta(a)mariadb.com> > Date: Mon May 20 15:14:30 2019 +0530 > > MDEV-15777: Support Early NULLs filtering-like restrictions in the range optimizer > > For eqjoin conditions, we add a NOT NULL predicate so as to allow the range optimizer > to use the predicate and possibly create a range access on the given table. > > Example: > select * from t1,t2 where t1.a=t2.a; we have KEY(a) on t1 > we would inject a NOT NULL predicate t1.a IS NOT NULL for table t1 > this would allow the range optimizer to create ranges and we get a > range access on table t1, then we will be able > to do an early NULL filtering for ranges too. > > diff --git a/mysql-test/main/range.result b/mysql-test/main/range.result > index 32e0cf2868c..d75d6734277 100644 > --- a/mysql-test/main/range.result > +++ b/mysql-test/main/range.result > @@ -3024,3 +3024,151 @@ drop table t1; > # > # End of 10.2 tests > # > +# > +# MDEV-15777: Support Early NULLs filtering-like restrictions in the range optimizer > +# > +set @save_optimizer_switch= @@optimizer_switch; > +set @@optimizer_switch='null_rejecting_for_ranges=on'; > +create table ten(a int); > +insert into ten values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); > +create table one_k(a int); > +insert into one_k select A.a + B.a* 10 + C.a * 100 from ten A, ten B, ten C; Did we abandon the convention of naming the tables t1,t2,t3,... ? If not, please rename. > +create table t1 ( > +id int(10) unsigned NOT NULL, > +subset_id int(11) DEFAULT NULL, > +PRIMARY KEY (id), > +KEY t1_subset_id (subset_id)); ... > diff --git a/sql/opt_range.cc b/sql/opt_range.cc > index ec7b3dbbd7a..cb7800d1ba6 100644 > --- a/sql/opt_range.cc > +++ b/sql/opt_range.cc > @@ -2539,6 +2546,42 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use, > TRP_GROUP_MIN_MAX *group_trp; > double best_read_time= read_time; > > + /* > + For the range optimizer, null_rejecting_conds are just an extra condition > + on which one can use index to create ranges. > + > + Also here we try to allow consider range access for a column involed in a > + null rejecting predicate only if it is the first keypart of an index. > + Let try to make it clear with examples: > + > + Query 1 > + select * from t1,t2 where t1.a=2 and t1.b=t2.b; > + and we have a key(a,b) > + So lets consider range access for table t1: > + Null rejecting predicate : t1.b IS NOT NULL > + but the column t1.b is NOT the first keypart in the index, > + so we don't consider it while building ranges > + > + Query 2 > + select * from t1,t2 where t1.a=t2.a and t1.b=t2.b; > + and we have a key (a,b) > + > + So lets consider range access for table t1: > + Null rejecting predicate : t1.a IS NOT NULL AND t1.b IS NOT NULL > + so here t1.a is the first keypart in the index, so we consider > + the predicate for building the ranges. > + > + The first keypart prerequisite is made as we try to reuse range > + estimates (because they are more accurate) at different places > + and we needed to make sure that correct/accurate estimates are used there. > + */ > + > + if (null_rejecting_conds) > + not_null_cond_tree= null_rejecting_conds->get_mm_tree(&param, > + &null_rejecting_conds); Please fix indentation and fit within 80 chars. ^ And also use curly braces as the code inside if doesn't fit on one line. > + if (not_null_cond_tree) > + remove_nonrange_trees(&param, not_null_cond_tree); > + > if (cond) > { > if ((tree= cond->get_mm_tree(&param, &cond))) > @@ -2557,6 +2600,7 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use, > tree= NULL; > } > } > + tree= tree_and(&param, tree, not_null_cond_tree); > > /* > Try to construct a QUICK_GROUP_MIN_MAX_SELECT. > @@ -4422,6 +4466,128 @@ static bool create_partition_index_description(PART_PRUNE_PARAM *ppar) > return FALSE; > } > > +inline void add_cond(THD *thd, Item **e1, Item *e2) > +{ > + if (*e1) > + { > + if (!e2) > + return; > + Item *res; > + if ((res= new (thd->mem_root) Item_cond_and(thd, *e1, e2))) > + { > + res->fix_fields(thd, 0); > + res->update_used_tables(); > + *e1= res; > + } > + } > + else > + *e1= e2; > +} > + > +/* > + Create null rejecting conditions for a table, for all the equalites > + present in the WHERE clause of a query. > + > + SYNOPSIS > + make_null_rejecting_conds() > + @param TABLE - Keys of this table will participate in null > + rejecting conditions > + @param keyuse_array - array that has all the equalites of the > + WHERE clasuse The above doesn't match the function's definition. Out of date comment? > + > + DESCRIPTION > + This function creates null rejecting conditions for a table. These > + conditions are created to do range analysis on them , the conditions > + are of the form tbl.key.keypart IS NOT NULL. > + > + IMPLEMENTATION > + Lookup in the keyuse array to check if it has equalites that belong > + to the given table. If yes then find out if the conditions are null > + rejecting and accordingly create all the condition for the keys of a > + given table and AND them. > + > + > + RETURN > + NOT NULL - Found null rejecting conditions for the given table > + NULL - No null rejecting conditions for the given table The above doesn't match the function's definition. Out of date comment? > +*/ > + > +void make_null_rejecting_conds(THD *thd, JOIN_TAB *tab) > +{ > + KEY *keyinfo; > + Item *cond= NULL; > + KEYUSE* keyuse; > + TABLE *table= tab->table; > + key_map *const_keys= &tab->const_keys; ^ Does it make sense to introduce a variable if it is only used once? > + > + /* > + No need to add NOT NULL condition for materialized derived tables > + or materialized subqueries as we do not run the range optimizer > + on their conditions > + */ > + > + if (!optimizer_flag(thd, OPTIMIZER_SWITCH_NULL_REJECTING_FOR_RANGES)) > + return; > + > + if (tab->table->is_filled_at_execution() || > + (tab->table->pos_in_table_list->derived && > + tab->table->pos_in_table_list->is_materialized_derived())) > + return; > + > + Field_map not_null_keypart_map; > + > + /* > + The null rejecting conds added will be on the keypart of a key, so for > + that we need the table to atleast have a key. > + */ > + if (!table->s->keys || table->null_rejecting_conds || !tab->keyuse) > + return; > + > + for(keyuse= tab->keyuse; keyuse->table == table; keyuse++) > + { Please add space, "for ". > + /* > + No null rejecting conds for a hash key or full-text keys > + */ > + if (keyuse->key == MAX_KEY || keyuse->keypart == FT_KEYPART) > + continue; > + keyinfo= keyuse->table->key_info + keyuse->key; > + Field *field= keyinfo->key_part[keyuse->keypart].field; > + > + if (not_null_keypart_map.is_set(field->field_index)) > + continue; > + > + /* > + No need to add null-rejecting condition if we have a > + keyuse element as > + - table.key.keypart= const > + - (table.key.keypart= tbl.otherfield or table.key.keypart IS NULL) > + - table.key.keypart IS NOT NULLABLE > + */ > + > + if (keyuse->val->const_item() || > + !(keyuse->null_rejecting && field->maybe_null()) || > + keyuse->optimize & KEY_OPTIMIZE_REF_OR_NULL) > + continue; > + > + Item_field *field_item= new (thd->mem_root)Item_field(thd, field); > + Item* not_null_item= new (thd->mem_root)Item_func_isnotnull(thd, field_item); > + > + /* > + adding the key to const keys as we have the condition > + as key.keypart IS NOT NULL > + */ > + > + const_keys->set_bit(keyuse->key); > + not_null_item->fix_fields(thd, 0); ^^ please change 0 to &not_null_item. > + not_null_item->update_used_tables(); > + add_cond(thd, &cond, not_null_item); > + not_null_keypart_map.set_bit(field->field_index); > + } > + DBUG_EXECUTE("where", print_where(cond, "Early Null Filtering", > + QT_ORDINARY);); > + table->null_rejecting_conds= cond; > +} > + > > #ifndef DBUG_OFF > ... > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index 8cdcf3afc8b..140ceed5c2d 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -5510,15 +5512,30 @@ add_key_field(JOIN *join, > If the condition has form "tbl.keypart = othertbl.field" and > othertbl.field can be NULL, there will be no matches if othertbl.field > has NULL value. > - We use null_rejecting in add_not_null_conds() to add > - 'othertbl.field IS NOT NULL' to tab->select_cond. > + > + The field KEY_FIELD::null_rejecting is set to TRUE if either > + the left or the right hand side of the equality is NULLABLE > + > + null_rejecting is used > + - by add_not_null_conds(), to add "othertbl.field IS NOT NULL" to > + othertbl's tab->select_cond. (This is called "Early NULLs filtering") > + > + - by make_null_rejecting_conds(), to provide range optimizer with > + additional "tbl.keypart IS NOT NULL" condition. > */ > { > Item *real= (*value)->real_item(); > - if (((cond->functype() == Item_func::EQ_FUNC) || > - (cond->functype() == Item_func::MULT_EQUAL_FUNC)) && > - (real->type() == Item::FIELD_ITEM) && > - ((Item_field*)real)->field->maybe_null()) > + if (( > + (cond->functype() == Item_func::EQ_FUNC) || > + (cond->functype() == Item_func::MULT_EQUAL_FUNC) > + ) && > + ( > + ( > + (real->type() == Item::FIELD_ITEM) && > + ((Item_field*)real)->field->maybe_null() > + ) || > + (field->maybe_null()) > + )) > (*key_fields)->null_rejecting= true; > else > (*key_fields)->null_rejecting= false; > @@ -9982,8 +9999,12 @@ static bool create_ref_for_key(JOIN *join, JOIN_TAB *j, > uint maybe_null= MY_TEST(keyinfo->key_part[i].null_bit); > j->ref.items[i]=keyuse->val; // Save for cond removal > j->ref.cond_guards[i]= keyuse->cond_guard; > - if (keyuse->null_rejecting) > + Item *real= (keyuse->val)->real_item(); > + if (keyuse->null_rejecting && > + (real->type() == Item::FIELD_ITEM) && > + ((Item_field*)real)->field->maybe_null()) This code looks as if if keyuse->val->maybe_null() != keyuse->val->real_item()->maybe_null() do you have any specific case in mind or this is just to match the code in add_key_field? > j->ref.null_rejecting|= (key_part_map)1 << i; > + > keyuse_uses_no_tables= keyuse_uses_no_tables && !keyuse->used_tables; > /* > We don't want to compute heavy expressions in EXPLAIN, an example would BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

2 1

Re: [Maria-developers] [Commits] 53d0e921794: MDEV-11094: Blackhole table updates on slave fail when row annotation is enabled
by andrei.elkin＠pp.inet.fi 22 May '19

22 May '19

Sujatha, salute! Thanks for a detailed description! I would add up to it a minor piece > revision-id: 53d0e9217943719b806ef9fa1cac0a699df4839c (mariadb-10.1.39-31-g53d0e921794) > parent(s): 47637a3dd13d19e897a7cbfd1499f1bf3b2fdb2a > author: Sujatha > committer: Sujatha > timestamp: 2019-05-17 13:11:49 +0530 > message: > > MDEV-11094: Blackhole table updates on slave fail when row annotation is enabled > > Problem: > ======= > rpl_blackhole.test fails when executed with following options > mysqld=--binlog_annotate_row_events=1, mysqld=--replicate_annotate_row_events=1 > > Test output: > ------------ > worker[1] Using MTR_BUILD_THREAD 300, with reserved ports 16000..16019 > rpl.rpl_blackhole_bug 'mix' [ pass ] 791 > rpl.rpl_blackhole_bug 'row' [ fail ] > Replicate_Wild_Ignore_Table > Last_Errno 1032 > Last_Error Could not execute Update_rows_v1 event on table test.t1; Can't find > record in 't1', Error_code: 1032; handler error HA_ERR_END_OF_FILE; the event's > master log master-bin.000001, end_log_pos 1510 > > Analysis: > ========= > Enabling "replicate_annotate_row_events" on slave, Tells the slave to write > annotate rows events received from the master to its own binary log. The > received annotate events are applied after the Gtid event as shown below. > thd->query() will be set to the actual query received from the master, through > annotate event. Annotate_rows event should not be deleted after the event is > applied as the thd->query will be used to generate new Annotate_rows event > during applying the subsequent Rows events. [ here ] The Table_map and Rows_log_event that follow do not touch thd->query(). Notice that the being suggested sentence is sufficient to explain the not-NULL without the sentence below (fill free to remove then): > After the last Rows event has been > applied, the saved Annotate_rows event (if any) will be deleted. > > master-bin.000001 | 457 | Gtid | BEGIN GTID 0-1-2 > master-bin.000001 | 495 | Annotate_rows | insert into t values (10) > master-bin.000001 | 539 | Table_map | table_id: 19 (test.t) > master-bin.000001 | 579 | Write_rows_v1 | table_id: 19 flags: STMT_END_F > master-bin.000001 | 613 | Xid | COMMIT /* xid=7 */ > > In balckhole engine all the DML operations are noops as they donot store any > data. They simply return success without doing any operation. But the existing > strictly expects thd->query() to be 'NULL' to identify that row based > replication is in use. This assumption will fail when row annotations are > enabled as the query is not 'NULL'. Hence various row based operations like > 'update', 'delete', 'index lookup' will fail when row annotations are enabled. > > Fix: > === > Extend the row based replication check to include row annotations as well. > i.e Either the thd->query() is NULL or thd->query() points to query and row > annotations are in use. This works, but the check does not really refers to the Annotate. So in principle thd->query() may turn in not-NULL of different reasons. Besides the check is multiplied over few places (again hopefully covering all). We can do it simpler actually: Rows_log_event::do_apply_event() { ... do { /* in_use can have been set to NULL in close_tables_for_reopen */ THD* old_thd= table->in_use; if (!table->in_use) table->in_use= thd; --> add a check whether Annotate has been done and when so store/restore --> thd->query() like `old_thd` above/below, and pass it as NULL (thd->reset_query()) to the following: error= do_exec_row(rgi); if (error) DBUG_PRINT("info", ("error: %s", HA_ERR(error))); DBUG_ASSERT(error != HA_ERR_RECORD_DELETED); table->in_use = old_thd; I also suggest to check out mysqlbinlog run. There seems to be no test for that though, so one needs to be written. The goal would be to prove a created binlog output is as expected (e.g to include Annotate). Cheers, Andrei > > --- > mysql-test/suite/rpl/t/rpl_blackhole-master.opt | 1 + > mysql-test/suite/rpl/t/rpl_blackhole-slave.opt | 1 + > storage/blackhole/ha_blackhole.cc | 28 +++++++++++++++++++------ > 3 files changed, 24 insertions(+), 6 deletions(-) > > diff --git a/mysql-test/suite/rpl/t/rpl_blackhole-master.opt b/mysql-test/suite/rpl/t/rpl_blackhole-master.opt > new file mode 100644 > index 00000000000..1ad0b884c60 > --- /dev/null > +++ b/mysql-test/suite/rpl/t/rpl_blackhole-master.opt > @@ -0,0 +1 @@ > +--binlog_annotate_row_events > diff --git a/mysql-test/suite/rpl/t/rpl_blackhole-slave.opt b/mysql-test/suite/rpl/t/rpl_blackhole-slave.opt > new file mode 100644 > index 00000000000..7ac6a84faa7 > --- /dev/null > +++ b/mysql-test/suite/rpl/t/rpl_blackhole-slave.opt > @@ -0,0 +1 @@ > +--binlog_annotate_row_events --replicate_annotate_row_events > diff --git a/storage/blackhole/ha_blackhole.cc b/storage/blackhole/ha_blackhole.cc > index 01aaa9ea15f..43bcdc541a1 100644 > --- a/storage/blackhole/ha_blackhole.cc > +++ b/storage/blackhole/ha_blackhole.cc > @@ -25,6 +25,16 @@ > #include "ha_blackhole.h" > #include "sql_class.h" // THD, SYSTEM_THREAD_SLAVE_SQL > > +static bool is_row_based_replication(THD *thd) > +{ > + /* > + A row event which has its thd->query() == NULL or a row event which has > + replicate_annotate_row_events enabled. In the later case the thd->query() > + will be pointing to the query, received through replicated annotate event > + from master. > + */ > + return ((thd->query() == NULL) || thd->variables.binlog_annotate_row_events); > +} > /* Static declarations for handlerton */ > > static handler *blackhole_create_handler(handlerton *hton, > @@ -109,7 +119,8 @@ int ha_blackhole::update_row(const uchar *old_data, uchar *new_data) > { > DBUG_ENTER("ha_blackhole::update_row"); > THD *thd= ha_thd(); > - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) > + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && > + is_row_based_replication(thd)) > DBUG_RETURN(0); > DBUG_RETURN(HA_ERR_WRONG_COMMAND); > } > @@ -118,7 +129,8 @@ int ha_blackhole::delete_row(const uchar *buf) > { > DBUG_ENTER("ha_blackhole::delete_row"); > THD *thd= ha_thd(); > - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) > + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && > + is_row_based_replication(thd)) > DBUG_RETURN(0); > DBUG_RETURN(HA_ERR_WRONG_COMMAND); > } > @@ -135,7 +147,8 @@ int ha_blackhole::rnd_next(uchar *buf) > int rc; > DBUG_ENTER("ha_blackhole::rnd_next"); > THD *thd= ha_thd(); > - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) > + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && > + is_row_based_replication(thd)) > rc= 0; > else > rc= HA_ERR_END_OF_FILE; > @@ -220,7 +233,8 @@ int ha_blackhole::index_read_map(uchar * buf, const uchar * key, > int rc; > DBUG_ENTER("ha_blackhole::index_read"); > THD *thd= ha_thd(); > - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) > + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && > + is_row_based_replication(thd)) > rc= 0; > else > rc= HA_ERR_END_OF_FILE; > @@ -235,7 +249,8 @@ int ha_blackhole::index_read_idx_map(uchar * buf, uint idx, const uchar * key, > int rc; > DBUG_ENTER("ha_blackhole::index_read_idx"); > THD *thd= ha_thd(); > - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) > + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && > + is_row_based_replication(thd)) > rc= 0; > else > rc= HA_ERR_END_OF_FILE; > @@ -249,7 +264,8 @@ int ha_blackhole::index_read_last_map(uchar * buf, const uchar * key, > int rc; > DBUG_ENTER("ha_blackhole::index_read_last"); > THD *thd= ha_thd(); > - if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && thd->query() == NULL) > + if (thd->system_thread == SYSTEM_THREAD_SLAVE_SQL && > + is_row_based_replication(thd)) > rc= 0; > else > rc= HA_ERR_END_OF_FILE; > _______________________________________________ > commits mailing list > commits(a)mariadb.org > https://lists.askmonty.org/cgi-bin/mailman/listinfo/commits

1 0

Re: [Maria-developers] 9e176e81b72: MDEV-18531 : Use WolfSSL instead of YaSSL as "bundled" SSL/encryption library
by Sergei Golubchik 21 May '19

21 May '19

Hi, Vladislav! It's good to see how small the patch is, and that wolfssl is more OpenSSL compatible than YaSSL. Few comments below. On May 19, Vladislav Vaintroub wrote: > revision-id: 9e176e81b72 (mariadb-10.4.4-61-g9e176e81b72) > parent(s): ea679c88c32 > author: Vladislav Vaintroub <wlad(a)mariadb.com> > committer: Vladislav Vaintroub <wlad(a)mariadb.com> > timestamp: 2019-05-01 11:28:56 +0100 > message: > > MDEV-18531 : Use WolfSSL instead of YaSSL as "bundled" SSL/encryption library > > - Add new submodule for WolfSSL > - Build and use wolfssl and wolfcrypt instead of yassl/taocrypt > - Use HAVE_WOLFSSL instead of HAVE_YASSL > - Increase MY_AES_CTX_SIZE, to avoid compile time asserts in my_crypt.cc > (sizeof(EVP_CIPHER_CTX) is larger on WolfSSL) > > diff --git a/cmake/ssl.cmake b/cmake/ssl.cmake > index aa42333a5c8..79a531ec323 100644 > --- a/cmake/ssl.cmake > +++ b/cmake/ssl.cmake > @@ -48,29 +48,19 @@ MACRO (CHANGE_SSL_SETTINGS string) > ENDMACRO() > > MACRO (MYSQL_USE_BUNDLED_SSL) > - SET(INC_DIRS > - ${CMAKE_SOURCE_DIR}/extra/yassl/include > - ${CMAKE_SOURCE_DIR}/extra/yassl/taocrypt/include > + SET(INC_DIRS > + ${CMAKE_SOURCE_DIR}/extra/wolfssl/wolfssl > + ${CMAKE_SOURCE_DIR}/extra/wolfssl/wolfssl/wolfssl > + #${CMAKE_SOURCE_DIR}/extra/wolfssl/taocrypt/include don't you want to remove these (and many below) commented lines? > ) > - SET(SSL_LIBRARIES yassl taocrypt) > + SET(SSL_LIBRARIES wolfssl wolfcrypt) > SET(SSL_INCLUDE_DIRS ${INC_DIRS}) > - SET(SSL_INTERNAL_INCLUDE_DIRS ${CMAKE_SOURCE_DIR}/extra/yassl/taocrypt/mySTL) > + SET(SSL_DEFINES "-DHAVE_OPENSSL -DHAVE_WOLFSSL -DOPENSSL_ALL -DWOLFSSL_MYSQL_COMPATIBLE -DWC_NO_HARDEN") > - SET(SSL_DEFINES "-DHAVE_YASSL -DYASSL_PREFIX -DHAVE_OPENSSL -DMULTI_THREADED") > - SET(HAVE_ERR_remove_thread_state OFF CACHE INTERNAL "yassl doesn't have ERR_remove_thread_state") > - SET(HAVE_EncryptAes128Ctr OFF CACHE INTERNAL "yassl doesn't support AES-CTR") > - SET(HAVE_EncryptAes128Gcm OFF CACHE INTERNAL "yassl doesn't support AES-GCM") > + SET(HAVE_ERR_remove_thread_state ON CACHE INTERNAL "wolfssl doesn't have ERR_remove_thread_state") > + SET(HAVE_EncryptAes128Ctr ON CACHE INTERNAL "wolfssl does support AES-CTR") > + SET(HAVE_EncryptAes128Gcm OFF CACHE INTERNAL "wolfssl does not support AES-GCM") > CHANGE_SSL_SETTINGS("bundled") > - ADD_SUBDIRECTORY(extra/yassl) > - ADD_SUBDIRECTORY(extra/yassl/taocrypt) > - GET_TARGET_PROPERTY(src yassl SOURCES) > - FOREACH(file ${src}) > - SET(SSL_SOURCES ${SSL_SOURCES} ${CMAKE_SOURCE_DIR}/extra/yassl/${file}) > - ENDFOREACH() > - GET_TARGET_PROPERTY(src taocrypt SOURCES) > - FOREACH(file ${src}) > - SET(SSL_SOURCES ${SSL_SOURCES} > - ${CMAKE_SOURCE_DIR}/extra/yassl/taocrypt/${file}) > - ENDFOREACH() > + ADD_SUBDIRECTORY(extra/wolfssl) > MESSAGE_ONCE(SSL_LIBRARIES "SSL_LIBRARIES = ${SSL_LIBRARIES}") > ENDMACRO() > > diff --git a/extra/wolfssl/CMakeLists.txt b/extra/wolfssl/CMakeLists.txt > new file mode 100644 > index 00000000000..c1165ad6c72 > --- /dev/null > +++ b/extra/wolfssl/CMakeLists.txt > @@ -0,0 +1,119 @@ > +# CMakeLists.txt > +# > +# Copyright (C) 2006-2015 wolfSSL Inc. > +# > +# This file is part of wolfSSL. (formerly known as CyaSSL) > +# > +# wolfSSL is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 2 of the License, or > +# (at your option) any later version. > +# > +# wolfSSL is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program; if not, write to the Free Software > +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301, USA > + > +SET(WOLFSSL_SRCDIR ${CMAKE_CURRENT_SOURCE_DIR}/wolfssl/src) > +ADD_DEFINITIONS(${SSL_DEFINES}) > +ADD_DEFINITIONS( > + -DWOLFSSL_MYSQL_COMPATIBLE > + -DHAVE_ECC > + -DECC_TIMING_RESISTANT > + -DBUILDING_WOLFSSL > + -DHAVE_HASHDRBG > + -DWOLFSSL_AES_DIRECT > + -DWOLFSSL_SHA384 > + -DWOLFSSL_SHA512 > + -DWOLFSSL_SHA224 > + -DSESSION_CERT > + -DKEEP_OUR_CERT > + -DWOLFSSL_STATIC_RSA > + -DWC_RSA_BLINDING > + -DHAVE_TLS_EXTENSIONS > + -DHAVE_AES_ECB > + -DWOLFSSL_AES_COUNTER > + -DNO_WOLFSSL_STUB) > + > +SET(WOLFSSL_SOURCES > + ${WOLFSSL_SRCDIR}/crl.c > + ${WOLFSSL_SRCDIR}/internal.c > + ${WOLFSSL_SRCDIR}/keys.c > + ${WOLFSSL_SRCDIR}/tls.c > + ${WOLFSSL_SRCDIR}/wolfio.c > + ${WOLFSSL_SRCDIR}/ocsp.c > + ${WOLFSSL_SRCDIR}/ssl.c) > +ADD_DEFINITIONS(-DWOLFSSL_LIB) > +INCLUDE_DIRECTORIES(BEFORE ${CMAKE_CURRENT_SOURCE_DIR}/wolfssl) > +IF(MSVC) > + # size_t to long truncation warning > + ADD_DEFINITIONS(-wd4267) > +ENDIF() > + > + > +ADD_CONVENIENCE_LIBRARY(wolfssl ${WOLFSSL_SOURCES}) > + > +# Workaround linker crash with older Ubuntu binutils > +# e.g aborting at ../../bfd/merge.c line 873 in _bfd_merged_section_offset > +IF((CMAKE_SYSTEM_NAME MATCHES "Linux") AND (CMAKE_LINKER STREQUAL "/usr/bin/ld")) > + EXECUTE_PROCESS(COMMAND ${CMAKE_LINKER} -v OUTPUT_VARIABLE LINKER_VERSION_STR) > + STRING(REGEX MATCH "[0-9]+\\.[0-9]+" LINKER_VERSION > + "${LINKER_VERSION_STR}") > + IF(LINKER_VERSION AND (LINKER_VERSION VERSION_LESS 2.25)) > + STRING(REPLACE "-g " "-g1 " CMAKE_C_FLAGS_RELWITHDEBINFO > + ${CMAKE_C_FLAGS_RELWITHDEBINFO}) > + STRING(REPLACE "-g " "-g1 " CMAKE_C_FLAGS_DEBUG > + ${CMAKE_C_FLAGS_DEBUG}) > + STRING(REPLACE "-ggdb3 " " " CMAKE_C_FLAGS_RELWITHDEBINFO > + ${CMAKE_C_FLAGS_RELWITHDEBINFO}) > + STRING(REPLACE "-ggdb3 " " " CMAKE_C_FLAGS_DEBUG > + ${CMAKE_C_FLAGS_DEBUG}) > + ENDIF() > +ENDIF() I would've just disabled -g\w+ unconditionally, for a simpler CMakeLists.txt We aren't going to debug wolfssl anyway (and if I'd need to debug it, for some weird reason, I know how to enable debugging temporarily) > + > + > +SET(WOLFCRYPT_SRCDIR ${CMAKE_CURRENT_SOURCE_DIR}/wolfssl/wolfcrypt/src) > +SET(WOLFCRYPT_SOURCES > +${WOLFCRYPT_SRCDIR}/aes.c > +${WOLFCRYPT_SRCDIR}/arc4.c > +${WOLFCRYPT_SRCDIR}/asn.c > +#${WOLFCRYPT_SRCDIR}/blake2b.c > +#${WOLFCRYPT_SRCDIR}/camellia.c > +#${WOLFCRYPT_SRCDIR}/chacha.c > +${WOLFCRYPT_SRCDIR}/coding.c > +#${WOLFCRYPT_SRCDIR}/compress.c > +${WOLFCRYPT_SRCDIR}/des3.c > +${WOLFCRYPT_SRCDIR}/dh.c > +${WOLFCRYPT_SRCDIR}/dsa.c > +${WOLFCRYPT_SRCDIR}/ecc.c > +${WOLFCRYPT_SRCDIR}/error.c > +#${WOLFCRYPT_SRCDIR}/hc128.c > +${WOLFCRYPT_SRCDIR}/hmac.c > +${WOLFCRYPT_SRCDIR}/integer.c > +${WOLFCRYPT_SRCDIR}/logging.c > +#${WOLFCRYPT_SRCDIR}/md2.c > +${WOLFCRYPT_SRCDIR}/md4.c > +${WOLFCRYPT_SRCDIR}/md5.c > +${WOLFCRYPT_SRCDIR}/memory.c > +#${WOLFCRYPT_SRCDIR}/pkcs7.c > +${WOLFCRYPT_SRCDIR}/pkcs12.c > +#${WOLFCRYPT_SRCDIR}/poly1305.c > +${WOLFCRYPT_SRCDIR}/pwdbased.c > +${WOLFCRYPT_SRCDIR}/rabbit.c > +${WOLFCRYPT_SRCDIR}/random.c > +#${WOLFCRYPT_SRCDIR}/ripemd.c > +${WOLFCRYPT_SRCDIR}/rsa.c > +${WOLFCRYPT_SRCDIR}/sha.c > +${WOLFCRYPT_SRCDIR}/sha256.c > +${WOLFCRYPT_SRCDIR}/sha512.c > +#${WOLFCRYPT_SRCDIR}/tfm.c > +${WOLFCRYPT_SRCDIR}/wc_port.c > +${WOLFCRYPT_SRCDIR}/wc_encrypt.c > +${WOLFCRYPT_SRCDIR}/hash.c > +${WOLFCRYPT_SRCDIR}/wolfmath.c) > +ADD_CONVENIENCE_LIBRARY(wolfcrypt ${WOLFCRYPT_SOURCES}) > + > diff --git a/extra/wolfssl/wolfssl b/extra/wolfssl/wolfssl > new file mode 160000 > index 00000000000..dc313ccf6ee > --- /dev/null > +++ b/extra/wolfssl/wolfssl > @@ -0,0 +1 @@ > +Subproject commit dc313ccf6ee0e99fb5a13793b82e412f396e11c6 I think we should only use wolfssl (any external project, actually) only at release points, not at intermediate commits. 4.0.0-stable should be ok, I guess. And, generaly, pefer to use system globally installed library, if available. Although in case of wolfssl (and libmarias3) I would not bother. > diff --git a/include/ssl_compat.h b/include/ssl_compat.h > index c94b9671d5f..105405efff2 100644 > --- a/include/ssl_compat.h > +++ b/include/ssl_compat.h > @@ -17,9 +17,9 @@ > #include <openssl/opensslv.h> > > /* OpenSSL version specific definitions */ > -#if !defined(HAVE_YASSL) && defined(OPENSSL_VERSION_NUMBER) > +#if defined(OPENSSL_VERSION_NUMBER) > > -#if OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER) > +#if (OPENSSL_VERSION_NUMBER >= 0x10002000L && !defined(LIBRESSL_VERSION_NUMBER)) || defined (HAVE_WOLFSSL) What's OPENSSL_VERSION_NUMBER in wolfssl? > #define HAVE_X509_check_host 1 May be move it to ssl.cmake? Where other such defines are. > #endif > > diff --git a/include/sslopt-case.h b/include/sslopt-case.h > index 4a8c65948cb..b129a97cc76 100644 > --- a/include/sslopt-case.h > +++ b/include/sslopt-case.h > @@ -29,8 +29,8 @@ > One can disable SSL later by using --skip-ssl or --ssl=0 > */ > opt_use_ssl= 1; > - /* crl has no effect in yaSSL */ > -#ifdef HAVE_YASSL > +#ifdef HAVE_WOLFSSL > + /* CRL does not work with WolfSSL */ Still? > opt_ssl_crl= NULL; > opt_ssl_crlpath= NULL; > #endif > diff --git a/mysys_ssl/my_crypt.cc b/mysys_ssl/my_crypt.cc > index 2d6f5188034..1ce0c6bbc03 100644 > --- a/mysys_ssl/my_crypt.cc > +++ b/mysys_ssl/my_crypt.cc > @@ -18,14 +18,10 @@ > #include <my_global.h> > #include <string.h> > > -#ifdef HAVE_YASSL > -#include "yassl.cc" delete yassl.cc file, perhaps? > -#else > #include <openssl/evp.h> > #include <openssl/aes.h> > #include <openssl/err.h> > #include <openssl/rand.h> > -#endif > > #include <my_crypt.h> > #include <ssl_compat.h> > @@ -70,8 +66,24 @@ class MyCTX > } > virtual int finish(uchar *dst, uint *dlen) > { > +#ifdef HAVE_WOLFSSL > + /* > + Bug in WolfSSL - sometimes EVP_CipherFinal_ex > + returns success without setting destination length > + when it should return error. > + We catch it by presetting invalid value for length, > + and checking if it has changed after the call. > + > + See https://github.com/wolfSSL/wolfssl/issues/2092 > + */ > + *dlen= UINT_MAX; > +#endif I suppose you can remove it. This is fixed as of 4.0.0-stable tag. > if (!EVP_CipherFinal_ex(ctx, dst, (int*)dlen)) > return MY_AES_BAD_DATA; > +#ifdef HAVE_WOLFSSL > + if (*dlen == UINT_MAX) > + return MY_AES_BAD_DATA; > +#endif > return MY_AES_OK; > } > }; > diff --git a/plugin/file_key_management/parser.cc b/plugin/file_key_management/parser.cc > index 13a9dfa0cb6..27277f62994 100644 > --- a/plugin/file_key_management/parser.cc > +++ b/plugin/file_key_management/parser.cc > @@ -103,7 +103,6 @@ openssl enc -aes-256-cbc -md sha1 -k "secret" -in keys.txt -out keys.enc > EVP_BytesToKey(EVP_aes_256_cbc(), EVP_sha1(), salt, > secret, strlen(secret), 1, key, iv); > > - but alas! we want to support yassl too does wolfssl have an equivalent of EVP_BytesToKey? if not, the comment still holds. > */ > > void Parser::bytes_to_key(const unsigned char *salt, const char *input, > diff --git a/sql/mysqld.cc b/sql/mysqld.cc > index bfa03aa57c1..8c3a59c55d1 100644 > --- a/sql/mysqld.cc > +++ b/sql/mysqld.cc > @@ -7317,13 +7315,13 @@ static int show_ssl_get_verify_mode(THD *thd, SHOW_VAR *var, char *buff, > { > var->type= SHOW_LONG; > var->value= buff; > -#ifndef HAVE_YASSL > +#ifndef HAVE_WOLFSSL > if( thd->net.vio && thd->net.vio->ssl_arg ) > *((long *)buff)= (long)SSL_get_verify_mode((SSL*)thd->net.vio->ssl_arg); no SSL_get_verify_mode or an analog in wolfssl? I'm asking, as I see it has gone a long way towards OpenSSL compatibility > else > *((long *)buff)= 0; > #else > - *((long *)buff) = 0; > + *((long *)buff)= 0; > #endif > return 0; > } Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

Re: [Maria-developers] ddec45aa50e: MDEV-16932: ASAN heap-use-after-free in my_charlen_utf8 / my_well_formed_char_length_utf8 on 2nd execution of SP with ALTER trying to add bad CHECK
by Sergei Golubchik 20 May '19

20 May '19

Hi, Oleksandr! On May 20, Oleksandr Byelkin wrote: > Alter table will trigger recompilation of the SP (as I remember all SPs) I've run this test, it complains about duplicate constraint name. > > What about this: > > > > CREATE TABLE t1 (a INT); > > CREATE PROCEDURE sp() ALTER TABLE t1 ADD CONSTRAINT CHECK (b > 0); > > --error ER_BAD_FIELD_ERROR > > CALL sp; > > alter table t1 add column b int, add constraint check (b < 10); > > CALL sp; > > > > ? > > second CALL will not regenerate the name of the constraint. > > Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] ddec45aa50e: MDEV-16932: ASAN heap-use-after-free in my_charlen_utf8 / my_well_formed_char_length_utf8 on 2nd execution of SP with ALTER trying to add bad CHECK
by Sergei Golubchik 20 May '19

20 May '19

Hi, Oleksandr! On May 20, Oleksandr Byelkin wrote: > On Mon, May 20, 2019 at 3:11 PM Sergei Golubchik <serg(a)mariadb.org> wrote: > > On May 20, Oleksandr Byelkin wrote: > > > revision-id: ddec45aa50e (mariadb-10.2.24-11-gddec45aa50e) > > > parent(s): 50999738eae > > > author: Oleksandr Byelkin <sanja(a)mariadb.com> > > > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > > > timestamp: 2019-05-14 14:01:15 +0200 > > > message: > > > > > > MDEV-16932: ASAN heap-use-after-free in my_charlen_utf8 / > > > my_well_formed_char_length_utf8 on 2nd execution of SP with ALTER > > > trying to add bad CHECK > > > > > > In case of error the SP can be executed without re-comilation and so > > > will reuse constructed constaint name, so the name should be allocated > > > in the statement memory. > > > > Is that right? You're saying that the "unique constraint name" will be > > generated once and reused? > > > > But on the second SP invocation is might no longer be unique. I'd think > > that `ALTER TABLE t1 ADD CONSTRAINT CHECK (x > 0)` should add a new > > constraint, not "add a new contraint with the name that used to be > > unique when the SP was invoked for the first time". > > With the second call we have 2 possibility: > > 1) first execution failed and so we can reuse the name > 2) first execution succeed and we will re-compile SP or re-prepare PS and > so will generate new name (that is why we mostly had no problems) What about this: CREATE TABLE t1 (a INT); CREATE PROCEDURE sp() ALTER TABLE t1 ADD CONSTRAINT CHECK (b > 0); --error ER_BAD_FIELD_ERROR CALL sp; alter table t1 add column b int, add constraint check (b < 10); CALL sp; ? second CALL will not regenerate the name of the constraint. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] ddec45aa50e: MDEV-16932: ASAN heap-use-after-free in my_charlen_utf8 / my_well_formed_char_length_utf8 on 2nd execution of SP with ALTER trying to add bad CHECK
by Sergei Golubchik 20 May '19

20 May '19

Hi, Oleksandr! On May 20, Oleksandr Byelkin wrote: > revision-id: ddec45aa50e (mariadb-10.2.24-11-gddec45aa50e) > parent(s): 50999738eae > author: Oleksandr Byelkin <sanja(a)mariadb.com> > committer: Oleksandr Byelkin <sanja(a)mariadb.com> > timestamp: 2019-05-14 14:01:15 +0200 > message: > > MDEV-16932: ASAN heap-use-after-free in my_charlen_utf8 / > my_well_formed_char_length_utf8 on 2nd execution of SP with ALTER > trying to add bad CHECK > > In case of error the SP can be executed without re-comilation and so > will reuse constructed constaint name, so the name should be allocated > in the statement memory. Is that right? You're saying that the "unique constraint name" will be generated once and reused? But on the second SP invocation is might no longer be unique. I'd think that `ALTER TABLE t1 ADD CONSTRAINT CHECK (x > 0)` should add a new constraint, not "add a new contraint with the name that used to be unique when the SP was invoked for the first time". > diff --git a/sql/sql_table.cc b/sql/sql_table.cc > index 3df877792cc..ecd5de3a09d 100644 > --- a/sql/sql_table.cc > +++ b/sql/sql_table.cc > @@ -4185,9 +4185,15 @@ mysql_prepare_create_table(THD *thd, HA_CREATE_INFO *create_info, > while ((check= c_it++)) > { > if (!check->name.length) > + { > + Query_arena backup; > + Query_arena *arena= thd->activate_stmt_arena_if_needed(&backup); > make_unique_constraint_name(thd, &check->name, > &alter_info->check_constraint_list, > &nr); > + if (arena) > + thd->restore_active_arena(arena, &backup); > + } > { > /* Check that there's no repeating constraint names. */ > List_iterator_fast<Virtual_column_info> Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

1 0

Re: [Maria-developers] [Commits] 66349551bf3: MDEV-16214: Incorrect plan taken by the optimizer , uses INDEX instead of ref access with ORDER BY
by Sergey Petrunia 19 May '19

19 May '19

Hi Varun, The patch is mostly fine. Please find some input on the comments / coding style below. Ok to push after it is addressed. On Fri, May 17, 2019 at 01:44:36PM +0530, Varun wrote: > revision-id: 66349551bf3a266fbefaebfe044abd05108723c2 (mariadb-10.3.12-205-g66349551bf3) > parent(s): 3d56adbfac394b2b3ffd22a89fe7c2978ed9a505 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2019-05-17 13:44:05 +0530 > message: > > MDEV-16214: Incorrect plan taken by the optimizer , uses INDEX instead of ref access with ORDER BY > > The issue in this case is that we take in account the estimates from quick keys instead of rec_per_key. > The estimates for quick keys are better than rec_per_key only if we have ref(const), so we need to check > that all keyparts in the ref key are of the type ref(const). > > --- > mysql-test/main/order_by.result | 57 +++++++++++++++++++++++++++++++++++++++++ > mysql-test/main/order_by.test | 37 ++++++++++++++++++++++++++ > sql/sql_select.cc | 42 ++++++++++++++++++++---------- > 3 files changed, 122 insertions(+), 14 deletions(-) ... > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index f36a68bc7ae..8cdcf3afc8b 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -9986,31 +9986,35 @@ static bool create_ref_for_key(JOIN *join, JOIN_TAB *j, > j->ref.null_rejecting|= (key_part_map)1 << i; > keyuse_uses_no_tables= keyuse_uses_no_tables && !keyuse->used_tables; > /* > - Todo: we should remove this check for thd->lex->describe on the next > - line. With SHOW EXPLAIN code, EXPLAIN printout code no longer depends > - on it. However, removing the check caused change in lots of query > - plans! Does the optimizer depend on the contents of > - table_ref->key_copy ? If yes, do we produce incorrect EXPLAINs? > + We don't want to compute heavy expressions in EXPLAIN, an example would > + select * from t1 where t1.key=(select thats very heavy); > + > + (select thats very heavy) => is a constant here > + eg: (select avg(order_cost) from orders) => constant but expensive > */ > if (!keyuse->val->used_tables() && !thd->lex->describe) > { // Compare against constant > - store_key_item tmp(thd, > + store_key_item tmp(thd, > keyinfo->key_part[i].field, > key_buff + maybe_null, > maybe_null ? key_buff : 0, > keyinfo->key_part[i].length, > keyuse->val, > FALSE); > - if (unlikely(thd->is_fatal_error)) > - DBUG_RETURN(TRUE); > - tmp.copy(); > + if (unlikely(thd->is_fatal_error)) > + DBUG_RETURN(TRUE); > + tmp.copy(); > j->ref.const_ref_part_map |= key_part_map(1) << i ; > } > else > - *ref_key++= get_store_key(thd, > - keyuse,join->const_table_map, > - &keyinfo->key_part[i], > - key_buff, maybe_null); > + { > + *ref_key++= get_store_key(thd, > + keyuse,join->const_table_map, > + &keyinfo->key_part[i], > + key_buff, maybe_null); > + if (!keyuse->val->used_tables()) > + j->ref.const_ref_part_map |= key_part_map(1) << i ; > + } > /* > Remember if we are going to use REF_OR_NULL > But only if field _really_ can be null i.e. we force JT_REF > @@ -25256,6 +25260,8 @@ bool JOIN_TAB::save_explain_data(Explain_table_access *eta, > { > if (!(eta->ref_list.append_str(thd->mem_root, "const"))) > return 1; > + if (thd->lex->describe) > + key_ref++; This is very cryptic. Please provide an explanation, something like: create_ref_for_key() handles keypart=const equalities as follows: - non-EXPLAIN execution will copy the "const" to lookup tuple immediately and will not add an element to ref.key_copy - EXPLAIN will put an element into ref.key_copy. Since we've just printed "const" for it, we should skip it here. > } > else > { > @@ -26921,7 +26927,15 @@ test_if_cheaper_ordering(const JOIN_TAB *tab, ORDER *order, TABLE *table, > */ > if (ref_key >= 0 && ref_key != MAX_KEY && tab->type == JT_REF) > { > - if (table->quick_keys.is_set(ref_key)) > + /* > + For all the parts of the ref key we check if all of them belong > + to the type ref(const). This is done because if all parts of the ref > + key are of type ref(const), then we are sure that the estimates > + provides by quick keys is better than that provide by rec_per_key. The above is hard to read and has typos. Here's a shorter variant: If ref access uses keypart=const for all its key parts, and quick select uses the same # of key parts, then they are equivalent. Reuse #rows estimate from quick select as it is more precise. > + */ > + if (tab->ref.const_ref_part_map == make_prev_keypart_map(tab->ref.key_parts) && > + table->quick_keys.is_set(ref_key) && > + table->quick_key_parts[ref_key] == tab->ref.key_parts) Please fix indetation; One of the lines is too long. > refkey_rows_estimate= table->quick_rows[ref_key]; > else > { BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0

[Maria-developers] FYI: usage of F1-F12 keys in mysql client
by Aleksey Midenkov 17 May '19

17 May '19

By adding something like this in your .inputrc: $if MYSQL "\033OP": "use information_schema; show tables;\n" "\033OQ": "use test;\n" "\033OR": "select * from " "\033OS": "show columns from " "\033[15~": "\033[17~": "\033[18~": "\033[19~": "\033[20~": "\033[21~": "\033[23~": "\033[24~": $endif you may have F1-F12 keys utilized. Different terminals have different codes (.inputrc doesn't support capability names unfortunately), so you'll have to find out your own codes via tack utiity. With strong will and enough patience it is possible to add Ctrl+, Alt+, Shift+ and their combinations as well. This works only with MDEV-17609 patch. -- All the best, Aleksey Midenkov @midenok

1 0

Re: [Maria-developers] MDEV-19284-19285v3
by Eugene Kosov 14 May '19

14 May '19

Hi, Alexander! Patch looks good to me. 13.05.2019, 17:03, "Alexander Barkov" <bar(a)mariadb.com>: > Hi Eugene, > > Please review the next round of the patch. > > Are any changes in tests still needed? > > Thanks! -- Eugene

1 0

Re: [Maria-developers] 7198e7c71fc: MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING
by Sergei Golubchik 13 May '19

13 May '19

Hi, Aleksey! On Dec 23, Aleksey Midenkov wrote: > revision-id: 7198e7c71fc (versioning-1.0.6-82-g7198e7c71fc) > parent(s): 6be155757b7 > author: Aleksey Midenkov <midenok(a)gmail.com> > committer: Aleksey Midenkov <midenok(a)gmail.com> > timestamp: 2018-12-20 13:25:00 +0300 > message: > > MDEV-15458 Segfault in heap_scan() upon UPDATE after ADD SYSTEM VERSIONING > > * HA_EXTRA_REMEMBER_POS, HA_EXTRA_RESTORE_POS for HEAP storage engine > * Versioning tests support As far as I can see, there are two approaches to fixing this bug. And if we go the way of HA_EXTRA_REMEMBER_POS/HA_EXTRA_RESTORE_POS then your fix is good. But I'd still like to understand whether write_row is supposed to disturb the scan. Why should heap_write() overwrite info->current_ptr=pos and info->current_hash_ptr? Inserting a row doesn't happen at the "current position", so I don't quite understand why it modifies current_ptr at all. There is no functionality like "continue the scan from the last inserted row", right? It just doesn't make sense to me. Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

[Maria-developers] Welcome to GSoC 2019
by Vicențiu Ciorbaru 10 May '19

10 May '19

Hi! As you may have noticed a few days ago, you have received an email from Google that you have been accepted as part of Google Summer of Code. Congratulations on the effort to learn about the program, get involved and submit succesful proposals. The fun part begins now! If you have not done so already, please reach out to your mentor and seek further advice about your project. Each one of your projects can be implemented in multiple ways, some more complex than others. I strongly advise you to create a concrete plan based off of your proposal with your mentor and aim to stick to it. Your mentor can guide you to ensure you have a succesful GSoC. Remember that you are here to learn about Open Source, what it means to be part of an Open Source community, to interact with the community, all while becoming better programmers. Please take advantage of this opportunity and keep your technical discussions public, on Zulip https://mariadb.zulipchat.com, and on our developer mailing list https://launchpad.net/~maria-developers. By keeping your discussions public, you get the benefit of not only having your mentor see your questions but all of the MariaDB community. You never know when someone might lend a hand in case your mentor is not yet online. :) The mantra of our community should be that we are always welcoming to new people. Do not be afraid to ask questions. There are no bad or stupid or silly questions. The only key requirement is that you first make an effort to find the answer yourself and only afterwards seek outside help. If you can show a bit of involvement, the community will show you the same. I recommend that you agree with your mentor on some form of weekly progress reporting, via either a blog-post, an email, or something similar that can be publicly shared. Hope all of you will have a great GSoC! Vicențiu MariaDB Foundation GSoC 2019 Admin

1 0

Re: [Maria-developers] a4be420c920: MDEV-19235 MariaDB Server compiled for 128 Indexes crashes at startup
by Sergei Golubchik 09 May '19

09 May '19

Hi, Vladislav! Thanks. That's pretty much the solution I had in mind too :) A couple of comments on details below. I've only commented on sql_bitmap.h, everything else is fine. On May 08, Vladislav Vaintroub wrote: > revision-id: a4be420c920 (mariadb-10.4.4-25-ga4be420c920) > parent(s): ee4a2fef181 > author: Vladislav Vaintroub <wlad(a)mariadb.com> > committer: Vladislav Vaintroub <wlad(a)mariadb.com> > timestamp: 2019-04-17 17:17:06 +0100 > message: > > MDEV-19235 MariaDB Server compiled for 128 Indexes crashes at startup > > With MAX_INDEXIES=64(default), key_map=Bitmap<64> is just a wrapper around > ulonglong and thus "trivial" (can be bzero-ed, or memcpy-ed, and stays > valid still) > > With MAX_INDEXES=128, key_map = Bitmap<128> is not a "trivial" type > anymore. The implementation uses MY_BITMAP, and MY_BITMAP contains pointers > which make Bitmap invalid, when it is memcpy-ed/bzero-ed. > > The problem in 10.4 is that there are many new key_map members, inside TABLE > or KEY, and those are often memcopied and bzeroed > > The fix makes Bitmap "trivial", by inlining most of MY_BITMAP functionality. > pointers/heap allocations are not used anymore. > diff --git a/sql/sql_bitmap.h b/sql/sql_bitmap.h > index 705a8d169e1..74866336a1a 100644 > --- a/sql/sql_bitmap.h > +++ b/sql/sql_bitmap.h > @@ -25,73 +25,184 @@ > > #include <my_sys.h> > #include <my_bitmap.h> > +#include <my_bit.h> > > template <uint width> class Bitmap > { > - MY_BITMAP map; > +public: > uint32 buffer[(width+31)/32]; Why is it public? > public: > - Bitmap() { init(); } > + Bitmap():buffer() What does that do? > + { > + } > - Bitmap(const Bitmap& from) { *this=from; } > - explicit Bitmap(uint prefix_to_set) { init(prefix_to_set); } > + explicit Bitmap(uint prefix_to_set):buffer() > + { > + set_prefix(prefix_to_set); > + } > - void init() { my_bitmap_init(&map, buffer, width, 0); } > - void init(uint prefix_to_set) { init(); set_prefix(prefix_to_set); } > uint length() const { return width; } > - Bitmap& operator=(const Bitmap& map2) > - { > + void init() > + { > + clear_all(); > + } > + void init(uint prefix_to_set) > + { > init(); > - memcpy(buffer, map2.buffer, sizeof(buffer)); > - return *this; > + set_prefix(prefix_to_set); I wonder whether init() method is still needed. You've removed it from the constructor. And if there's any other code that uses it, I'd rather it'd be using clear_all(). > } > - void set_bit(uint n) { bitmap_set_bit(&map, n); } > + void set_bit(uint n) > + { > + DBUG_ASSERT(n < width); > + ((uchar*)buffer)[n / 8] |= (1 << (n & 7)); > + } > - void clear_bit(uint n) { bitmap_clear_bit(&map, n); } > + void clear_bit(uint n) > + { > + DBUG_ASSERT(n < width); > + ((uchar*)buffer)[n / 8] &= ~(1 << (n & 7)); > + } > - void set_prefix(uint n) { bitmap_set_prefix(&map, n); } > + void set_prefix(uint prefix_size) > + { > + set_if_smaller(prefix_size, width); > + uint prefix_bytes, prefix_bits, d; > + uchar* m = (uchar*)buffer; > + > + if ((prefix_bytes = prefix_size / 8)) > + memset(m, 0xff, prefix_bytes); > + m += prefix_bytes; > + if ((prefix_bits = prefix_size & 7)) > + { > + *(m++) = (1 << prefix_bits) - 1; > + // As the prefix bits are set, lets count this byte too as a prefix byte. > + prefix_bytes++; > + } > + if ((d = (width + 7) / 8 - prefix_bytes)) > + memset(m, 0, d); > + } > - void set_all() { bitmap_set_all(&map); } > + void set_all() > + { > + set_prefix(width); may be better to optimize it? memset(buffer, 0xff, sizeof(buffer) - 1); ((uchar*)buffer)[sizeof(buffer)-1] = last_byte_mask(width); > + } > - void clear_all() { bitmap_clear_all(&map); } > + void clear_all() > + { > + memset(buffer, 0x00, sizeof(buffer)); > + } > - void intersect(Bitmap& map2) { bitmap_intersect(&map, &map2.map); } > + void intersect(Bitmap & map2) > + { > + for (uint i = 0; i < array_elements(buffer); i++) > + buffer[i] &= map2.buffer[i]; > + } > - void intersect(ulonglong map2buff) > - { > - // Use a spearate temporary buffer, as bitmap_init() clears all the bits. > - ulonglong buf2; > - MY_BITMAP map2; > - > - my_bitmap_init(&map2, (uint32 *) &buf2, sizeof(ulonglong) * 8, 0); > - > - // Store the original bits. > - if (sizeof(ulonglong) >= 8) > - { > - int8store(const_cast<uchar *>(static_cast<uchar *> > - (static_cast<void *>(&buf2))), > - map2buff); > - } > - else > - { > - DBUG_ASSERT(sizeof(buffer) >= 4); > - int4store(const_cast<uchar *>(static_cast<uchar *> > - (static_cast<void *>(&buf2))), > - static_cast<uint32>(map2buff)); > - } > - bitmap_intersect(&map, &map2); > - } > + void intersect_and_pad(ulonglong map2buff, bool pad_with_ones) make it private? > + { > + compile_time_assert(sizeof(ulonglong) == 8); > + uint32 tmp[2]; > + int8store(tmp, map2buff); > + if (array_elements(buffer) > 2 && pad_with_ones) > + set_all(); really? I'd expect that if pad_with_ones==true, you simply don't need to do anything to the rest of the buffer. > + > + buffer[0] &= tmp[0]; > + if (array_elements(buffer) > 1) > + buffer[1] &= tmp[1]; > + > + if (array_elements(buffer) > 2 && !pad_with_ones) > + memset((char*)buffer + 8, 0 , sizeof(buffer) - 8); > + } > + void intersect(ulonglong map2buff) > + { > + intersect_and_pad(map2buff, 0); > + } > /* Use highest bit for all bits above sizeof(ulonglong)*8. */ > void intersect_extended(ulonglong map2buff) > { > - intersect(map2buff); > - if (map.n_bits > sizeof(ulonglong) * 8) > - bitmap_set_above(&map, sizeof(ulonglong), > - MY_TEST(map2buff & (1LL << (sizeof(ulonglong) * 8 - 1)))); > + intersect_and_pad(map2buff, (map2buff & (1ULL << 63))); > } > - void subtract(Bitmap& map2) { bitmap_subtract(&map, &map2.map); } > + void subtract(Bitmap & map2) > + { > + for (size_t i = 0; i < array_elements(buffer); i++) > + buffer[i] &= ~(map2.buffer[i]); > + } > - void merge(Bitmap& map2) { bitmap_union(&map, &map2.map); } > + void merge(Bitmap & map2) > + { > + for (size_t i = 0; i < array_elements(buffer); i++) > + buffer[i] |= map2.buffer[i]; > + } > - bool is_set(uint n) const { return bitmap_is_set(&map, n); } > + bool is_set(uint n) const > + { > + DBUG_ASSERT(n < width); > + return ((uchar*)buffer)[n / 8] & (1 << (n & 7)); > + } > - bool is_prefix(uint n) const { return bitmap_is_prefix(&map, n); } > + bool is_prefix(uint prefix_size) const > + { > + uint prefix_mask = last_byte_mask(prefix_size); > + uchar* m = (uchar*)buffer; > + uchar* end_prefix = m + (prefix_size - 1) / 8; > + uchar* end; > + DBUG_ASSERT(prefix_size <= width); > + > + /* Empty prefix is always true */ > + if (!prefix_size) > + return true; > + > + while (m < end_prefix) > + if (*m++ != 0xff) > + return false; > + > + end = ((uchar*)buffer) + (width + 7) / 8 - 1; > + if (m == end) > + return ((*m & last_byte_mask(width)) == prefix_mask); > + > + if (*m != prefix_mask) > + return false; > + > + while (++m < end) > + if (*m != 0) > + return false; > + return ((*m & last_byte_mask(width)) == 0); > + } > - bool is_clear_all() const { return bitmap_is_clear_all(&map); } > + bool is_clear_all() const > + { > + for (size_t i= 0; i < array_elements(buffer); i++) > + if (buffer[i]) > + return false; > + return true; > + } > - bool is_set_all() const { return bitmap_is_set_all(&map); } > + bool is_set_all() const > + { > + if (width == sizeof(buffer) * 8) > + { > + for (size_t i = 0; i < array_elements(buffer); i++) > + if (buffer[i] != 0xFFFFFFFFU) > + return false; > + return true; > + } > + else > + return is_prefix(width); this can also be optimized to not use complex is_prefix(), but I don't think is_set_all() is used as often as set_all(), so it shouldn't matter as much. > + } > - bool is_subset(const Bitmap& map2) const { return bitmap_is_subset(&map, &map2.map); } > + bool is_subset(const Bitmap & map2) const > + { > + for (size_t i= 0; i < array_elements(buffer); i++) > + if (buffer[i] & ~(map2.buffer[i])) > + return false; > + return true; > + } > - bool is_overlapping(const Bitmap& map2) const { return bitmap_is_overlapping(&map, &map2.map); } > + bool is_overlapping(const Bitmap & map2) const > + { > + for (size_t i = 0; i < array_elements(buffer); i++) > + if (buffer[i] & map2.buffer[i]) > + return true; > + return false; > + } > - bool operator==(const Bitmap& map2) const { return bitmap_cmp(&map, &map2.map); } > + bool operator==(const Bitmap & map2) const > + { > + return memcmp(buffer, map2.buffer, sizeof(buffer)) == 0; > + } > - bool operator!=(const Bitmap& map2) const { return !(*this == map2); } > + bool operator!=(const Bitmap & map2) const > + { > + return !(*this == map2); > + } > char *print(char *buf) const > { > char *s=buf; Regards, Sergei Chief Architect MariaDB and security(a)mariadb.org

2 2

Re: [Maria-developers] [Commits] 0ff8b73b60f: MDEV-15777:Support Early NULLs filtering-like restrictions in the range optimizer
by Sergey Petrunia 06 May '19

06 May '19

Hi Varun, Please finally find the review below. The patch is almost there I think, but there are some minor adjustments that still need to be made. > revision-id: 0ff8b73b60fae40556f91d3959f20bf3cf182b28 (mariadb-10.3.0-947-g0ff8b73b60f) > parent(s): 15419a558370aeed9521b498c34d50f20a8d47a5 > author: Varun Gupta > committer: Varun Gupta > timestamp: 2018-05-15 13:53:11 +0530 > message: > > MDEV-15777:Support Early NULLs filtering-like restrictions in the range optimizer > > --- > mysql-test/main/mdev15777.result | 455 +++++++++++++++++++++++++++++++++++++++ > mysql-test/main/mdev15777.test | 138 ++++++++++++ > sql/opt_range.cc | 125 ++++++++++- > sql/opt_range.h | 2 + > sql/sql_select.cc | 22 +- > sql/table.cc | 1 + > sql/table.h | 6 + > 7 files changed, 745 insertions(+), 4 deletions(-) > > diff --git a/mysql-test/main/mdev15777.result b/mysql-test/main/mdev15777.result > new file mode 100644 > index 00000000000..549ac08b91f > --- /dev/null > +++ b/mysql-test/main/mdev15777.result > @@ -0,0 +1,455 @@ > +# Test added to check that null filtering is used by range optimizer > +create table ten(a int); > +insert into ten values (0),(1),(2),(3),(4),(5),(6),(7),(8),(9); > +create table one_k(a int); > +insert into one_k select A.a + B.a* 10 + C.a * 100 from ten A, ten B, ten C; > +create table one_m(a int); > +insert into one_m select A.a + B.a* 1000 from one_k A, one_k B; The testcases are extremely confusing. * Do you really need a table with one millon rows to demonstrate the issue? * Why use examples with subqueries (which are converted into join anyway)? - if we want to verify that this optimization works together with subquery-to-semi-join conversion, let's start with a join example, and then construct a subquery which is converted to the query. * The testcases produce a lot of EXPLAIN outputs, and there is no explanation provided what one should expect. Please create a testcase: - of reasonable size (I think tables of 10-20K rows should suffice) - start from a join query - Like in other testcases, tables should be named, t1,t2, ... - I assume the query plan should be: range access on the first table (constructed from a NOT NULL predicate), ref access for the second one. - The testcase should have '--echo #' comments explaining what is relevant in the query plan > +delete from one_m where a=0 limit 1; > +create table t1 ( > +id int(10) unsigned NOT NULL AUTO_INCREMENT, > +filler varchar(100), > +subset_id int(11) DEFAULT NULL, > +PRIMARY KEY (id), > +KEY t1_subset_id (subset_id) > +); > +create table t1_subsets ( ... > diff --git a/sql/opt_range.cc b/sql/opt_range.cc > index 8422917065f..ae06b899ac9 100644 > --- a/sql/opt_range.cc > +++ b/sql/opt_range.cc > @@ -155,6 +155,7 @@ class SEL_IMERGE; > #define CLONE_KEY1_MAYBE 1 > #define CLONE_KEY2_MAYBE 2 > #define swap_clone_flag(A) ((A & 1) << 1) | ((A & 2) >> 1) > +#define FT_KEYPART (MAX_FIELDS+10) > > > /* > @@ -2400,6 +2401,7 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use, > { > uint idx; > double scan_time; > + Item *null_rejecting_conds= NULL; > DBUG_ENTER("SQL_SELECT::test_quick_select"); > DBUG_PRINT("enter",("keys_to_use: %lu prev_tables: %lu const_tables: %lu", > (ulong) keys_to_use.to_ulonglong(), (ulong) prev_tables, > @@ -2423,6 +2425,7 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use, > read_time= (double) records + scan_time + 1; // Force to use index > > possible_keys.clear_all(); > + null_rejecting_conds= head->null_rejecting_conds; > > DBUG_PRINT("info",("Time to scan table: %g", read_time)); > > @@ -2431,7 +2434,7 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use, > { > uchar buff[STACK_BUFF_ALLOC]; > MEM_ROOT alloc; > - SEL_TREE *tree= NULL; > + SEL_TREE *tree= NULL, *not_null_cond_tree= NULL; > KEY_PART *key_parts; > KEY *key_info; > PARAM param; > @@ -2540,6 +2543,12 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use, > TRP_GROUP_MIN_MAX *group_trp; > double best_read_time= read_time; > Please add a comment saying that as far as range optimizer is concerned, null_rejecting_conds is just an extra condition on this table. (That is, we don't care if it has NOT NULL conditions or something else). > + if (null_rejecting_conds) > + { > + not_null_cond_tree= null_rejecting_conds->get_mm_tree(&param, > + &null_rejecting_conds); > + } > + > if (cond) > { > if ((tree= cond->get_mm_tree(&param, &cond))) > @@ -2558,6 +2567,13 @@ int SQL_SELECT::test_quick_select(THD *thd, key_map keys_to_use, > tree= NULL; > } > } > + if (not_null_cond_tree) > + { > + if (!tree) > + tree= not_null_cond_tree; > + else > + tree= tree_and(&param, tree, not_null_cond_tree); This if-else logic is redundant, as it's already present in tree_and(). Can just have this: tree= tree_and(tree, not_null_cond_tree); > + } > > /* > Try to construct a QUICK_GROUP_MIN_MAX_SELECT. > @@ -14643,6 +14659,113 @@ void QUICK_GROUP_MIN_MAX_SELECT::add_keys_and_lengths(String *key_names, > add_key_and_length(key_names, used_lengths, &first); > } > > +inline void add_cond(THD *thd, Item **e1, Item *e2) > +{ > + if (*e1) > + { > + if (!e2) > + return; > + Item *res; > + if ((res= new (thd->mem_root) Item_cond_and(thd, *e1, e2))) > + { > + res->fix_fields(thd, 0); > + res->update_used_tables(); > + *e1= res; > + } > + } > + else > + *e1= e2; > +} > + > +/* > + Create null rejecting conditions for a table, for all the equalites > + present in the WHERE clause of a query. > + > + SYNOPSIS > + make_null_rejecting_conds() > + @param TABLE - Keys of this table will participate in null > + rejecting conditions > + @param keyuse_array - array that has all the equalites of the > + WHERE clasuse > + > + DESCRIPTION > + This function creates null rejecting conditions for a table. These > + conditions are created to do range analysis on them , the conditions > + are of the form tbl.key.keypart IS NOT NULL. > + > + IMPLEMENTATION > + Lookup in the keyuse array to check if it has equalites that belong > + to the given table. If yes then find out if the conditions are null > + rejecting and accordingly create all the condition for the keys of a > + given table and AND them. > + > + > + RETURN > + NOT NULL - Found null rejecting conditions for the given table > + NULL - No null rejecting conditions for the given table > +*/ > + > +void make_null_rejecting_conds(THD *thd, TABLE *table, > + DYNAMIC_ARRAY *keyuse_array, key_map *const_keys) ^ Please fix identation. > +{ > + KEY *keyinfo; > + Item *cond= NULL; > + KEYUSE* keyuse; > + > + /* > + The null rejecting conds added will be on the keypart of a key, so for > + that we need the table to atleast have a key. > + */ please fix typos/wording in the above comment. > + if (!table->s->keys) > + return ; > + if (table->null_rejecting_conds) > + return; > + > + for(uint i=0; i < keyuse_array->elements; i++) > + { This loop does excessive work. Check out how best_access_path() examines the KEYUSE array. Key points - the array is sorted by table, index_nr, key_part_nr. - JOIN_TAB::keyuse points to the first element that refers to the table described by the JOIN_TAB - The array may have "duplicates", especially when multiple equalities are present: KEYUSE(t.keyXpartY = other_table.colX) KEYUSE(t.keyXpartY = other_table.colY) KEYUSE(t.keyXpartY = some_expr) ... Since the elements are sorted by keyXpartY, it is trival to avoid generating multiple "t.keyXpartY IS NOT NULL" for consecutive KEYUSEs. I would go even further: the same "t.keyXpartY" can be part of multiple indexes (and so its KEYUSE objects will not be adjacent). Can we use a table's column bitmap to avoid generating duplicate IS NOT NULL objects? > + keyuse= (KEYUSE*)dynamic_array_ptr(keyuse_array, i); > + if (keyuse->table == table) > + { > + /* > + No null rejecting conds for a hash key orr full-text keys > + */ please fix typos/wording in the above comment. > + if (keyuse->key == MAX_KEY || keyuse->keypart == FT_KEYPART) > + continue; > + keyinfo= keyuse->table->key_info+keyuse->key; > + Field *field= keyinfo->key_part[keyuse->keypart].field; > + > + /* > + No need to add null-rejecting condition if we have a > + keyuse element as > + - table.key.keypart= const > + - (table.key.keypart= tbl.otherfield or table.key.keypart IS NULL) > + - table.key.keypart IS NOT NULLABLE > + */ > + > + if (keyuse->val->const_item() > + || !(keyuse->null_rejecting && field->maybe_null()) Please remove the brackets: !(A && B) -> !A || !B > + || keyuse->optimize & KEY_OPTIMIZE_REF_OR_NULL) Please also put the '||' at the end of the lines, like it is done in the rest of the code. > + continue; > + > + Item_field *field_item= new (thd->mem_root)Item_field(thd, field); > + Item* not_null_item= new (thd->mem_root)Item_func_isnotnull(thd, > + field_item); Please add error handling (just return from the function if we failed to allocate any of the above) > + > + /* > + adding the key to const keys as we have the condition > + as key.keypart IS NOT NULL > + */ > + > + const_keys->set_bit(keyuse->key); > + not_null_item->fix_fields(thd, 0); > + not_null_item->update_used_tables(); > + add_cond(thd, &cond, not_null_item); > + } > + } > + table->null_rejecting_conds= cond; > + return; Please remove the above 'return;' as it is redundant and makes one suspect a merge error > +} > + > > #ifndef DBUG_OFF > > diff --git a/sql/opt_range.h b/sql/opt_range.h > index bd85a12d4a1..894d46a892c 100644 > --- a/sql/opt_range.h > +++ b/sql/opt_range.h > @@ -1728,6 +1728,8 @@ bool calculate_cond_selectivity_for_table(THD *thd, TABLE *table, Item **cond); > bool prune_partitions(THD *thd, TABLE *table, Item *pprune_cond); > #endif > void store_key_image_to_rec(Field *field, uchar *ptr, uint len); > +void make_null_rejecting_conds(THD *thd, TABLE *table, > + DYNAMIC_ARRAY *keyuse_array, key_map *const_keys); Please fix identation. > > extern String null_string; > > diff --git a/sql/sql_select.cc b/sql/sql_select.cc > index f658b78c33c..4aceb333340 100644 > --- a/sql/sql_select.cc > +++ b/sql/sql_select.cc > @@ -4805,6 +4805,9 @@ make_join_statistics(JOIN *join, List<TABLE_LIST> &tables_list, > add_group_and_distinct_keys(join, s); > > s->table->cond_selectivity= 1.0; > + > + make_null_rejecting_conds(join->thd, s->table, > + keyuse_array, &s->const_keys); > > /* > Perform range analysis if there are keys it could use (1). > @@ -5352,15 +5356,24 @@ add_key_field(JOIN *join, > If the condition has form "tbl.keypart = othertbl.field" and > othertbl.field can be NULL, there will be no matches if othertbl.field > has NULL value. > + > + The field KEY_FIELD::null_rejecting is set to TRUE if we have both > + the left and right hand side of the equality are NULLABLE > + This is NOT what is happening. null_rejecting is set to true, if *either* left or right-hand side of the equality is nullable. Please fix the comment. Please also fix the comment in KEY_FIELD::null_rejecting to reflect that. > We use null_rejecting in add_not_null_conds() to add > 'othertbl.field IS NOT NULL' to tab->select_cond. > + > + We use null_rejecting in make_null_rejecting_conds() to add > + tbl.keypart IS NOT NULL so we can do range analysis on this condition > + Please join the above two sentences together: null_rejecting is used - by add_not_null_conds(), to add "othertbl.field IS NOT NULL" to othertbl's tab->select_cond. (This is called "Early NULLs filtering") - by make_null_rejecting_conds(), to provide range optimizer with additional "tbl.keypart IS NOT NULL" condition. > */ > { > Item *real= (*value)->real_item(); > if (((cond->functype() == Item_func::EQ_FUNC) || > (cond->functype() == Item_func::MULT_EQUAL_FUNC)) && > - (real->type() == Item::FIELD_ITEM) && > + (((real->type() == Item::FIELD_ITEM) && > ((Item_field*)real)->field->maybe_null()) > + ||(field->maybe_null()))) ^Please fix the identation. > (*key_fields)->null_rejecting= true; > else > (*key_fields)->null_rejecting= false; > @@ -9813,7 +9826,10 @@ static bool create_ref_for_key(JOIN *join, JOIN_TAB *j, > uint maybe_null= MY_TEST(keyinfo->key_part[i].null_bit); > j->ref.items[i]=keyuse->val; // Save for cond removal > j->ref.cond_guards[i]= keyuse->cond_guard; > - if (keyuse->null_rejecting) > + Item *real= (keyuse->val)->real_item(); > + if (keyuse->null_rejecting && > + (real->type() == Item::FIELD_ITEM) && > + ((Item_field*)real)->field->maybe_null()) ^ Please fix identation > j->ref.null_rejecting|= (key_part_map)1 << i; > keyuse_uses_no_tables= keyuse_uses_no_tables && !keyuse->used_tables; > /* > @@ -18538,7 +18554,7 @@ free_tmp_table(THD *thd, TABLE *entry) > DBUG_ASSERT(entry->pos_in_table_list->table == entry); > entry->pos_in_table_list->table= NULL; > } > - > + entry->null_rejecting_conds= NULL; What is this for? As far as I understand we are freeing the TABLE object > free_root(&own_root, MYF(0)); /* the table is allocated in its own root */ > thd_proc_info(thd, save_proc_info); > > diff --git a/sql/table.cc b/sql/table.cc > index a6e445d0d2e..097fee465de 100644 > --- a/sql/table.cc > +++ b/sql/table.cc > @@ -4597,6 +4597,7 @@ void TABLE::init(THD *thd, TABLE_LIST *tl) > created= TRUE; > cond_selectivity= 1.0; > cond_selectivity_sampling_explain= NULL; > + null_rejecting_conds= NULL; > #ifdef HAVE_REPLICATION > /* used in RBR Triggers */ > master_had_triggers= 0; > diff --git a/sql/table.h b/sql/table.h > index 6fd3f219914..50d47899b49 100644 > --- a/sql/table.h > +++ b/sql/table.h > @@ -1356,6 +1356,12 @@ struct TABLE > SplM_opt_info *spl_opt_info; > key_map keys_usable_for_splitting; > > + /* > + Null rejecting conds added for all tables so we can do range analysis > + on these conditions > + */ "for all tables"? Please change the comment to something like: "NULL-rejecting conditions for this table. They are created from eq_ref access, and used by the range optimizer". > + Item* null_rejecting_conds; > + > void init(THD *thd, TABLE_LIST *tl); > bool fill_item_list(List<Item> *item_list) const; > void reset_item_list(List<Item> *item_list, uint skip) const; BR Sergei -- Sergei Petrunia, Software Developer MariaDB Corporation | Skype: sergefp | Blog: http://s.petrunia.net/blog

1 0