[MDEV-16288] ALTER TABLE…ALGORITHM=DEFAULT does not override alter_algorithm Created: 2018-05-25  Updated: 2020-05-04  Resolved: 2020-05-04

Status: Closed
Project: MariaDB Server
Component/s: Data Definition - Alter Table
Affects Version/s: 10.3.7
Fix Version/s: 10.3.23, 10.4.13

Type: Bug Priority: Major
Reporter: Marko Mäkelä Assignee: Thirunarayanan Balathandayuthapani
Resolution: Fixed Votes: 0
Labels: None

Attachments: File mdev16288-10.3-61c0df94655f2dc3146456e49f3f51610251e79fv1.patch    
Issue Links:
Problem/Incident
is caused by MDEV-13134 Introduce ALTER TABLE attributes ALGO... Closed
Relates
relates to MDEV-18570 ALGORITHM=INSTANT or NOCOPY fails to ... Confirmed
relates to MDEV-21855 Document difference between DEFAULT a... Open

 Description   

Even if ALGORITHM=DEFAULT is explicitly specified in ALTER TABLE, the variable alter_algorithm will be substituted. That variable should only be substituted when no ALGORITHM has been specified.

--source include/have_innodb.inc
 
CREATE TABLE t(id INT PRIMARY KEY, u INT UNSIGNED NOT NULL UNIQUE)
 
ENGINE=InnoDB;
 
INSERT INTO t(id,u) VALUES(1,1),(2,2),(3,3);
 
 
 
SET alter_algorithm=instant;
 
ALTER TABLE t ADD COLUMN d DATETIME DEFAULT current_timestamp();
 
--error ER_ALTER_OPERATION_NOT_SUPPORTED
 
ALTER TABLE t DROP COLUMN u;
 
--error ER_ALTER_OPERATION_NOT_SUPPORTED
 
ALTER TABLE t DROP COLUMN u, ALGORITHM=NOCOPY;
 
# this should succeed, but currently fails!
 
ALTER TABLE t DROP COLUMN u, ALGORITHM=DEFAULT;
 
DROP TABLE t;



 Comments   
Comment by Marko Mäkelä [ 2018-09-19 ]

Another test case:

--source include/have_innodb.inc
 
 
 
SET alter_algorithm=instant;
 
 
 
create table t(a int primary key, b int) engine=innodb;
 
insert into t set a=1;
 
--enable_info
 
--echo # FIXME: This should succeed, and return "affected rows: 0"
 
--error ER_ALTER_OPERATION_NOT_SUPPORTED
 
alter table t drop column b, algorithm=default;
 
--echo # FIXME: This should succeed, and return "affected rows: 1"
 
--error ER_ALTER_OPERATION_NOT_SUPPORTED
 
alter table t drop primary key, algorithm=default;
 
--disable_info
 
drop table t;

Comment by Thirunarayanan Balathandayuthapani [ 2020-04-23 ]

The following patch could solve the issue:
Set requested_algorithm to ALTER_TABLE_ALGORITHM_NONE when there is no mention of
algorithm in alter table.

mdev16288-10.3-61c0df94655f2dc3146456e49f3f51610251e79fv1.patch

Please review it

Comment by Thirunarayanan Balathandayuthapani [ 2020-04-23 ]

Patch is in bb-10.3-MDEV-16288

Comment by Alexander Barkov [ 2020-04-30 ]

Hi Thiru,

The patch https://jira.mariadb.org/secure/attachment/51404/51404_mdev16288-10.3-61c0df94655f2dc3146456e49f3f51610251e79fv1.patch looks ok to push for me.

I'd only like you to add the entire two test cases from this report into the patch.

Thanks.

Comment by Alexander Barkov [ 2020-04-30 ]

One more thought:

I feel slightly non-comfortable with this code duplication:

  /* Use alter_algorithm when user didn't specify algorithm in alter */
 
  if (requested_algorithm == Alter_info::ALTER_TABLE_ALGORITHM_NONE)
 
    requested_algorithm = (Alter_info::enum_alter_table_algorithm) thd->variables.alter_algorithm;


    if (alter_info->requested_algorithm ==
 
        Alter_info::ALTER_TABLE_ALGORITHM_NONE)
 
    {
 
      alter_info->requested_algorithm=
 
	(Alter_info::enum_alter_table_algorithm)
 
	   thd->variables.alter_algorithm;
 
    }

Why not have a method:

enum_alter_table_algorithm Alter_info::algorithm(const THD *thd) const
 
{
 
  if (requested_algorithm == ALTER_TABLE_ALGORITHM_NONE)
 
    return (Alter_info::enum_alter_table_algorithm) thd->variables.alter_algorithm;
 
  return requested_algorithm;
 
}

?

So you can use it in multiple points in the code, instead of duplicating the code.

I suggest to let Alter_info::requested_algorithm stay ALTER_TABLE_ALGORITHM_NONE. Let's encapsulate to make sure that the caller do not test the member directly and uses the method only.

Comment by Alexander Barkov [ 2020-04-30 ]

This patch is OK to push:
https://github.com/MariaDB/server/commit/a37708ecea0d09787fc5e7b71b47279a2c8e4735

Generated at Thu Feb 08 08:27:47 UTC 2024 using Jira 8.20.16#820016-sha1:9d11dbea5f4be3d4cc21f03a88dd11d8c8687422.