[Maria-developers] Patch for MDEV-12874 - UPDATE statements with the same source and target
Hello, Here is a patch for MDEV-12874 (on branch 10.2-ext). Can anyone review it ? Alexander, during my work on this MDEV, I've found a new issue with update statement and Oracle compatibility. create table t1 (c1 integer, c2 integer, c3 integer); insert into t1(c1,c2,c3) values (1,1,1); update t1 set c1 = c1+1, c2 = c1+1, c3 = c2+1; select * from t1; Mariadb : +------+------+------+ | c1 | c2 | c3 | +------+------+------+ | 2 | 3 | 4 | +------+------+------+ 1 row in set (0.00 sec) Oracle : DVTORA> C1 C2 C3 ---------- ---------- ---------- 2 2 2 Can you open a new MDEV for this ? Best regards, Jérôme.
Hello Alexander, As I thought, there is the same issue with select. The following procedure produce a different result between Mariadb and Oracle. set sql_mode=oracle; use test; DELIMITER $$ CREATE or replace procedure psselerr_field(res OUT VARCHAR) AS b1 INT; BEGIN b1:=10; select 1,cast(b1 as varchar(10)) into b1, res from dual; return ; END; $$ DELIMITER ; call psselerr_field (@res); select @res; Mariadb : res = 1 Oracle: res = 10 Best regards, Jérôme.
-----Message d'origine----- De : jerome brauge Envoyé : vendredi 21 juillet 2017 17:04 À : maria-developers@lists.launchpad.net Cc : Alexander Barkov; 'Sergei Golubchik' Objet : Patch for MDEV-12874 - UPDATE statements with the same source and target
Hello,
Here is a patch for MDEV-12874 (on branch 10.2-ext). Can anyone review it ?
Alexander, during my work on this MDEV, I've found a new issue with update statement and Oracle compatibility.
create table t1 (c1 integer, c2 integer, c3 integer); insert into t1(c1,c2,c3) values (1,1,1); update t1 set c1 = c1+1, c2 = c1+1, c3 = c2+1; select * from t1;
Mariadb : +------+------+------+ | c1 | c2 | c3 | +------+------+------+ | 2 | 3 | 4 | +------+------+------+ 1 row in set (0.00 sec)
Oracle : DVTORA> C1 C2 C3 ---------- ---------- ---------- 2 2 2
Can you open a new MDEV for this ?
Best regards, Jérôme.
Hello Jerome, On 07/31/2017 04:28 PM, jerome brauge wrote:
Hello Alexander, As I thought, there is the same issue with select. The following procedure produce a different result between Mariadb and Oracle.
Thanks for reporting this. I filed a new issue for this: https://jira.mariadb.org/browse/MDEV-13418
set sql_mode=oracle; use test; DELIMITER $$ CREATE or replace procedure psselerr_field(res OUT VARCHAR) AS b1 INT; BEGIN b1:=10; select 1,cast(b1 as varchar(10)) into b1, res from dual; return ; END; $$ DELIMITER ; call psselerr_field (@res); select @res;
Mariadb : res = 1 Oracle: res = 10
Best regards, Jérôme.
-----Message d'origine----- De : jerome brauge Envoyé : vendredi 21 juillet 2017 17:04 À : maria-developers@lists.launchpad.net Cc : Alexander Barkov; 'Sergei Golubchik' Objet : Patch for MDEV-12874 - UPDATE statements with the same source and target
Hello,
Here is a patch for MDEV-12874 (on branch 10.2-ext). Can anyone review it ?
Alexander, during my work on this MDEV, I've found a new issue with update statement and Oracle compatibility.
create table t1 (c1 integer, c2 integer, c3 integer); insert into t1(c1,c2,c3) values (1,1,1); update t1 set c1 = c1+1, c2 = c1+1, c3 = c2+1; select * from t1;
Mariadb : +------+------+------+ | c1 | c2 | c3 | +------+------+------+ | 2 | 3 | 4 | +------+------+------+ 1 row in set (0.00 sec)
Oracle : DVTORA> C1 C2 C3 ---------- ---------- ---------- 2 2 2
Can you open a new MDEV for this ?
Best regards, Jérôme.
On 08/01/2017 09:29 AM, Alexander Barkov wrote:
Hello Jerome,
On 07/31/2017 04:28 PM, jerome brauge wrote:
Hello Alexander, As I thought, there is the same issue with select. The following procedure produce a different result between Mariadb and Oracle.
Thanks for reporting this. I filed a new issue for this:
Made a few updates in MDEV-13418 after reading the SQL standard. Please have a look.
set sql_mode=oracle; use test; DELIMITER $$ CREATE or replace procedure psselerr_field(res OUT VARCHAR) AS b1 INT; BEGIN b1:=10; select 1,cast(b1 as varchar(10)) into b1, res from dual; return ; END; $$ DELIMITER ; call psselerr_field (@res); select @res;
Mariadb : res = 1 Oracle: res = 10
Best regards, Jérôme.
-----Message d'origine----- De : jerome brauge Envoyé : vendredi 21 juillet 2017 17:04 À : maria-developers@lists.launchpad.net Cc : Alexander Barkov; 'Sergei Golubchik' Objet : Patch for MDEV-12874 - UPDATE statements with the same source and target
Hello,
Here is a patch for MDEV-12874 (on branch 10.2-ext). Can anyone review it ?
Alexander, during my work on this MDEV, I've found a new issue with update statement and Oracle compatibility.
create table t1 (c1 integer, c2 integer, c3 integer); insert into t1(c1,c2,c3) values (1,1,1); update t1 set c1 = c1+1, c2 = c1+1, c3 = c2+1; select * from t1;
Mariadb : +------+------+------+ | c1 | c2 | c3 | +------+------+------+ | 2 | 3 | 4 | +------+------+------+ 1 row in set (0.00 sec)
Oracle : DVTORA> C1 C2 C3 ---------- ---------- ---------- 2 2 2
Can you open a new MDEV for this ?
Best regards, Jérôme.
Hello Jerome, On 07/21/2017 07:04 PM, jerome brauge wrote:
Hello,
Here is a patch for MDEV-12874 (on branch 10.2-ext). Can anyone review it ?
Let's ask Sergei to review your patch when he's back from vacation.
Alexander, during my work on this MDEV, I've found a new issue with update statement and Oracle compatibility.
Thanks for noticing this. I filed a bug report: https://jira.mariadb.org/browse/MDEV-13417
create table t1 (c1 integer, c2 integer, c3 integer); insert into t1(c1,c2,c3) values (1,1,1); update t1 set c1 = c1+1, c2 = c1+1, c3 = c2+1; select * from t1;
Mariadb : +------+------+------+ | c1 | c2 | c3 | +------+------+------+ | 2 | 3 | 4 | +------+------+------+ 1 row in set (0.00 sec)
Oracle : DVTORA> C1 C2 C3 ---------- ---------- ---------- 2 2 2
Can you open a new MDEV for this ?
Best regards, Jérôme.
Hi, jerome! On Jul 21, jerome brauge wrote:
Hello,
Here is a patch for MDEV-12874 (on branch 10.2-ext). Can anyone review it ?
I'll send comments about the code later, don't want to mix small details and the big question. So, the big one: Why did you introduce completely new code for handling updates with the same table? mysql_update has logic when one searches and updates the same index (UPDATE idx=idx*2 WHERE idx > 5). That's used_key_is_modified variable. I'd thought you could reuse that. You might still need your tempfile_newdata, but even then it'll be much less code and simpler implementation. Would that work? Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hi, Sergei Here is a version that reuses used logic when one searches and updates the same index. The principal impact is that we can't use the can_compare optimization because we have to store all new values for all keys of tempfile. But this correct a bug in my previous implementation (on update through a view). This simplifies the code a bit, but not much (no needs of tempfile_key). I don't see how to do this more simpler. If you have better ideas, I listen your suggestions. Thank you very much. Jérôme.
-----Message d'origine----- De : Sergei Golubchik [mailto:serg@mariadb.org] Envoyé : vendredi 18 août 2017 01:24 À : jerome brauge Cc : maria-developers@lists.launchpad.net; Alexander Barkov Objet : Re: Patch for MDEV-12874 - UPDATE statements with the same source and target
Hi, jerome!
On Jul 21, jerome brauge wrote:
Hello,
Here is a patch for MDEV-12874 (on branch 10.2-ext). Can anyone review it ?
I'll send comments about the code later, don't want to mix small details and the big question. So, the big one:
Why did you introduce completely new code for handling updates with the same table? mysql_update has logic when one searches and updates the same index (UPDATE idx=idx*2 WHERE idx > 5). That's used_key_is_modified variable. I'd thought you could reuse that. You might still need your tempfile_newdata, but even then it'll be much less code and simpler implementation. Would that work?
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hi, Jérôme! Sorry, it took a while, there were releases in the plan :( On Aug 18, jerome brauge wrote:
Hi, Sergei Here is a version that reuses used logic when one searches and updates the same index. The principal impact is that we can't use the can_compare optimization because we have to store all new values for all keys of tempfile.
I don't quite understand that. You have to store new values in the tempfile, yes, but why does it mean that can_compare cannot be used? But here's a bug in your implementation, you cannot simply write table->record[0] to a temp file. This will not write blobs, because they're stored out of record. The correct solution is not to use an IO_CACHE for records, but to create a temporary table. It's create_tmp_table() function, and, for example, multi_update::initialize_tables() uses it. Hmm, I wonder, whether we could just reuse multi_update. Could be rather simple solution, if it'll work.
But this correct a bug in my previous implementation (on update through a view). This simplifies the code a bit, but not much (no needs of tempfile_key). I don't see how to do this more simpler. If you have better ideas, I listen your suggestions.
The way I see it now, there are essentially two distinct features. Using the same table in the WHERE clase. And in the SET clause. When the table is only present in the WHERE clause, we could use pretty much the same code as for used_key_is_modified (an additional loop over records before the main update loop, and a tempfile). When the table is only present in the SET clause, we don't need used_key_is_modified, but we need your tempfile_newdata and an additional loop after the main update loop. In the worst case you have three loops now (although this could, probably, be reduced to two). But I would think that this use case isn't particularly common and it is not worth the troubles distinguishing these use cases yet. So, there's no need to redo anything in this regard. Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hello Sergei, I agree, I can probably use can_compare_record in my second loop. The use of a temporary table only to handle the case of blobs, seems to me too heavy. If I add a check to exclude update of blob column (or switch to multi update) , is my patch be acceptable ? I will try to understand multi_update code to see if I can use it (but my skills are a bit limited) Many thanks. Jérôme.
-----Message d'origine----- De : Sergei Golubchik [mailto:serg@mariadb.org] Envoyé : mardi 29 août 2017 14:01 À : jerome brauge Cc : maria-developers@lists.launchpad.net; Alexander Barkov Objet : Re: Patch for MDEV-12874 - UPDATE statements with the same source and target
Hi, Jérôme!
Sorry, it took a while, there were releases in the plan :(
On Aug 18, jerome brauge wrote:
Hi, Sergei Here is a version that reuses used logic when one searches and updates the same index. The principal impact is that we can't use the can_compare optimization because we have to store all new values for all keys of tempfile.
I don't quite understand that. You have to store new values in the tempfile, yes, but why does it mean that can_compare cannot be used?
But here's a bug in your implementation, you cannot simply write table->record[0] to a temp file. This will not write blobs, because they're stored out of record.
The correct solution is not to use an IO_CACHE for records, but to create a temporary table. It's create_tmp_table() function, and, for example, multi_update::initialize_tables() uses it.
Hmm, I wonder, whether we could just reuse multi_update. Could be rather simple solution, if it'll work.
But this correct a bug in my previous implementation (on update through a view). This simplifies the code a bit, but not much (no needs of tempfile_key). I don't see how to do this more simpler. If you have better ideas, I listen your suggestions.
The way I see it now, there are essentially two distinct features. Using the same table in the WHERE clase. And in the SET clause.
When the table is only present in the WHERE clause, we could use pretty much the same code as for used_key_is_modified (an additional loop over records before the main update loop, and a tempfile).
When the table is only present in the SET clause, we don't need used_key_is_modified, but we need your tempfile_newdata and an additional loop after the main update loop.
In the worst case you have three loops now (although this could, probably, be reduced to two).
But I would think that this use case isn't particularly common and it is not worth the troubles distinguishing these use cases yet. So, there's no need to redo anything in this regard.
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
participants (3)
-
Alexander Barkov
-
jerome brauge
-
Sergei Golubchik