[Maria-developers] Add hidden level 2/3 (PSEDUO_HIDDEN_COLUMN / COMPLETELY_HIDDEN_COLUMN) to mariadb
Hello Everyone! Currently I am implementing 10177. But I have one doubt I am not sure how to implement a function which will create hidden level 2/3 field for server. Suppose we create a function (say add_hidden_field(.....)) which will add the field to table T. But the issue is where should we call this function in mysql_prepare_create_table / mysql_prepare_alter_table or init_from_binary_frm_image Doing it from init_from_binary_frm_image make sense because this way server and slave frm of table T will be same (We have decided that we wont log hidden level 2/3 field to binlog) , But the issue is optimizer wont be ready to decide whether we require a hidden level 3 field at that time. We can also do it from mysql_prepare_create_table , but I dont think this wont be good idea because at this time optimizer wont be ready plus this will complicate binlog handling. Or may be we should have a function which can modify table object any time. But I am not sure if it is possible/good idea ? What you guys think ? -- Regards Sachin Setiya Software Engineer at MariaDB
Hi, Sachin! Here's a review of all changes between 474f51711b1 and f0e2cf3e6c8:
diff --git a/include/my_base.h b/include/my_base.h index b93300d7562..68c828d5f74 100644 --- a/include/my_base.h +++ b/include/my_base.h @@ -277,7 +277,8 @@ enum ha_base_keytype { This flag can be calculated -- it's based on key lengths comparison. */ #define HA_KEY_HAS_PART_KEY_SEG 65536 - +/* Internal Flag Can be calcaluted */ +#define HA_INVISIBLE_SYSTEM_KEY 2<<18 /* Is it a *Invisible* key */
"an invisible key" and I'd just use HA_INVISIBLE_KEY
/* Automatic bits in key-flag */
#define HA_SPACE_PACK_USED 4 /* Test for if SPACE_PACK used */ diff --git a/mysql-test/r/features.result b/mysql-test/r/features.result index c6d1a6b0bac..3acd7436ad9 100644 --- a/mysql-test/r/features.result +++ b/mysql-test/r/features.result @@ -8,6 +8,7 @@ Feature_delay_key_write 0 Feature_dynamic_columns 0 Feature_fulltext 0 Feature_gis 0 +Feature_hidden_columns 0
You forgot to rename the variable. and let's rename test files too, it's a bit confusing now.
Feature_locale 0 Feature_subquery 0 Feature_timezone 0 diff --git a/mysql-test/r/hidden_field.result b/mysql-test/r/hidden_field.result new file mode 100644 index 00000000000..7ff6f617eef --- /dev/null +++ b/mysql-test/r/hidden_field.result @@ -0,0 +1,692 @@ +FLUSH STATUS; +create table t1(abc int primary key, xyz int invisible); +SHOW STATUS LIKE 'Feature_invisible_columns'; +Variable_name Value +desc t1; +Field Type Null Key Default Extra +abc int(11) NO PRI NULL +xyz int(11) YES NULL INVISIBLE +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `abc` int(11) NOT NULL, + `xyz` int(11) INVISIBLE DEFAULT NULL, + PRIMARY KEY (`abc`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +select * from INFORMATION_SCHEMA.COLUMNS where TABLE_SCHEMA='test' and TABLE_NAME='t1'; +TABLE_CATALOG TABLE_SCHEMA TABLE_NAME COLUMN_NAME ORDINAL_POSITION COLUMN_DEFAULT IS_NULLABLE DATA_TYPE CHARACTER_MAXIMUM_LENGTH CHARACTER_OCTET_LENGTH NUMERIC_PRECISION NUMERIC_SCALE DATETIME_PRECISION CHARACTER_SET_NAME COLLATION_NAME COLUMN_TYPE COLUMN_KEY EXTRA PRIVILEGES COLUMN_COMMENT IS_GENERATED GENERATION_EXPRESSION +def test t1 abc 1 NULL NO int NULL NULL 10 0 NULL NULL NULL int(11) PRI select,insert,update,references NEVER NULL +def test t1 xyz 2 NULL YES int NULL NULL 10 0 NULL NULL NULL int(11) INVISIBLE select,insert,update,references NEVER NULL +drop table t1; +create table t1(a1 int invisible); +ERROR 42000: A table must have at least 1 column +create table t1(a1 blob,invisible(a1)); +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '(a1))' at line 1 +create table t1(a1 int primary key invisible ,a2 int unique invisible , a3 blob,a4 int not null invisible unique); +ERROR HY000: Hidden column 'a1' must either have a default value or allow null values +create table t1(abc int not null invisible); +ERROR HY000: Hidden column 'abc' must either have a default value or allow null values +create table t1(a int invisible, b int); +insert into t1 values(1); +insert into t1(a) values(2); +insert into t1(b) values(3); +insert into t1(a,b) values(5,5); +select * from t1; +b +1 +NULL +3 +5 +select a,b from t1; +a b +NULL 1 +2 NULL +NULL 3 +5 5 +delete from t1; +insert into t1 values(1),(2),(3),(4); +select * from t1; +b +1 +2 +3 +4 +select a from t1; +a +NULL +NULL +NULL +NULL +drop table t1; +#more complex case of invisible +create table t1(a int , b int invisible , c int invisible auto_increment unique, d blob , e int unique, f int); +desc t1; +Field Type Null Key Default Extra +a int(11) YES NULL +b int(11) YES NULL INVISIBLE
remove a space character before "INVISIBLE"
+c int(11) NO PRI NULL auto_increment, INVISIBLE +d blob YES NULL +e int(11) YES UNI NULL +f int(11) YES NULL +insert into t1 values(1,'d blob',1,1),(1,'d blob',11,1),(1,'d blob',2,1),(1,'d blob',3,1),(1,'d blob',41,1); +select * from t1; +a d e f +1 d blob 1 1 +1 d blob 11 1 +1 d blob 2 1 +1 d blob 3 1 +1 d blob 41 1
would be nice to see `select a,b,c,d,e,f` here
+drop table t1; +create table sdsdsd(a int , b int, invisible(a,b)); +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '(a,b))' at line 1 +create table t1(a int,abc int as (a mod 3) virtual invisible); +desc t1; +Field Type Null Key Default Extra +a int(11) YES NULL +abc int(11) YES NULL VIRTUAL GENERATED, INVISIBLE +insert into t1 values(1,default); +ERROR 21S01: Column count doesn't match value count at row 1 +insert into t1 values(1),(22),(233); +select * from t1; +a +1 +22 +233 +select a,abc from t1; +a abc +1 1 +22 1 +233 2 +drop table t1; +create table t1(abc int primary key invisible auto_increment, a int); +desc t1; +Field Type Null Key Default Extra +abc int(11) NO PRI NULL auto_increment, INVISIBLE +a int(11) YES NULL +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `abc` int(11) NOT NULL INVISIBLE AUTO_INCREMENT, + `a` int(11) DEFAULT NULL, + PRIMARY KEY (`abc`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +insert into t1 values(1); +insert into t1 values(2); +insert into t1 values(3); +select * from t1; +a +1 +2 +3 +select abc,a from t1; +abc a +1 1 +2 2 +3 3 +delete from t1; +insert into t1 values(1),(2),(3),(4),(6); +select abc,a from t1; +abc a +4 1 +5 2 +6 3 +7 4 +8 6 +drop table t1; +create table t1(abc int); +alter table t1 change abc ss int invisible; +ERROR 42000: A table must have at least 1 column +alter table t1 add column xyz int; +alter table t1 modify column abc int ; +desc t1; +Field Type Null Key Default Extra +abc int(11) YES NULL +xyz int(11) YES NULL +insert into t1 values(22); +ERROR 21S01: Column count doesn't match value count at row 1 +alter table t1 modify column abc int invisible; +desc t1; +Field Type Null Key Default Extra +abc int(11) YES NULL INVISIBLE +xyz int(11) YES NULL +insert into t1 values(12); +drop table t1; +#some test on copy table structure with table data; +#table with invisible fields and unique keys; +create table t1(a int , b int invisible , c int invisible auto_increment unique, d blob , e int unique, f int); +desc t1; +Field Type Null Key Default Extra +a int(11) YES NULL +b int(11) YES NULL INVISIBLE +c int(11) NO PRI NULL auto_increment, INVISIBLE +d blob YES NULL +e int(11) YES UNI NULL +f int(11) YES NULL +insert into t1 values(1,'d blob',1,1),(1,'d blob',11,1),(1,'d blob',2,1),(1,'d blob',3,1),(1,'d blob',41,1); +select * from t1; +a d e f +1 d blob 1 1 +1 d blob 11 1 +1 d blob 2 1 +1 d blob 3 1 +1 d blob 41 1 +select a,b,c,d,e,f from t1; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +#this wont copy invisible fields and keys;
won't
+create table t2 as select * from t1; +desc t2; +Field Type Null Key Default Extra +a int(11) YES NULL +d blob YES NULL +e int(11) YES NULL +f int(11) YES NULL +select * from t2; +a d e f +1 d blob 1 1 +1 d blob 11 1 +1 d blob 2 1 +1 d blob 3 1 +1 d blob 41 1 +select a,b,c,d,e,f from t2; +ERROR 42S22: Unknown column 'b' in 'field list' +drop table t2; +#now this will copy invisible fields +create table t2 as select a,b,c,d,e,f from t1; +desc t2; +Field Type Null Key Default Extra +a int(11) YES NULL +b int(11) YES NULL +c int(11) NO 0 +d blob YES NULL +e int(11) YES NULL +f int(11) YES NULL +select * from t2; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +select a,b,c,d,e,f from t2; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +drop table t2,t1; +#some test related to copy of data from one table to another; +create table t1(a int , b int invisible , c int invisible auto_increment unique, d blob , e int unique, f int); +insert into t1 values(1,'d blob',1,1),(1,'d blob',11,1),(1,'d blob',2,1),(1,'d blob',3,1),(1,'d blob',41,1); +select a,b,c,d,e,f from t1; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +create table t2(a int , b int invisible , c int invisible , d blob , e int unique, f int); +insert into t2 select * from t1; +select a,b,c,d,e,f from t2; +a b c d e f +1 NULL NULL d blob 1 1 +1 NULL NULL d blob 11 1 +1 NULL NULL d blob 2 1 +1 NULL NULL d blob 3 1 +1 NULL NULL d blob 41 1 +truncate t2; +insert into t2 (a,b,c,d,e,f) select a,b,c,d,e,f from t1; +select a,b,c,d,e,f from t2; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +truncate t2; +drop table t1,t2; +#some test related to creating view on table with invisible column; +create table t1(a int , b int invisible , c int invisible auto_increment unique, d blob , e int unique, f int); +insert into t1 values(1,'d blob',1,1),(1,'d blob',11,1),(1,'d blob',2,1),(1,'d blob',3,1),(1,'d blob',41,1); +create view v as select * from t1; +desc v; +Field Type Null Key Default Extra +a int(11) YES NULL +d blob YES NULL +e int(11) YES NULL +f int(11) YES NULL +select * from v; +a d e f +1 d blob 1 1 +1 d blob 11 1 +1 d blob 2 1 +1 d blob 3 1 +1 d blob 41 1 +#v does not have invisible column; +select a,b,c,d,e,f from v; +ERROR 42S22: Unknown column 'b' in 'field list' +insert into v values(1,21,32,4); +select * from v; +a d e f +1 d blob 1 1 +1 d blob 11 1 +1 d blob 2 1 +1 d blob 3 1 +1 d blob 41 1 +1 21 32 4 +insert into v(a,b,c,d,e,f) values(1,12,3,4,5,6); +ERROR 42S22: Unknown column 'b' in 'field list'
good tests, with views. this behavior is not obvious
+drop view v; +create view v as select a,b,c,d,e,f from t1; +desc v; +Field Type Null Key Default Extra +a int(11) YES NULL +b int(11) YES NULL +c int(11) NO 0 +d blob YES NULL +e int(11) YES NULL +f int(11) YES NULL +select * from v; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +1 NULL 6 21 32 4 +#v does have invisible column but they aren't invisible anymore. +select a,b,c,d,e,f from v; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +1 NULL 6 21 32 4 +insert into v values(1,26,33,4,45,66); +select a,b,c,d,e,f from v; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +1 NULL 6 21 32 4 +1 26 33 4 45 66 +insert into v(a,b,c,d,e,f) values(1,32,31,41,5,6); +select a,b,c,d,e,f from v; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +1 NULL 6 21 32 4 +1 26 33 4 45 66 +1 32 31 41 5 6 +drop view v; +drop table t1; +#now invisible column in where and some join query +create table t1 (a int unique , b int invisible unique, c int unique invisible); +insert into t1(a,b,c) values(1,1,1); +insert into t1(a,b,c) values(2,2,2); +insert into t1(a,b,c) values(3,3,3); +insert into t1(a,b,c) values(4,4,4); +insert into t1(a,b,c) values(21,21,26); +insert into t1(a,b,c) values(31,31,35); +insert into t1(a,b,c) values(41,41,45); +insert into t1(a,b,c) values(22,22,24); +insert into t1(a,b,c) values(32,32,33); +insert into t1(a,b,c) values(42,42,43); +explain select * from t1 where b=3; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 const b b 5 const 1 +select * from t1 where b=3; +a +3 +explain select * from t1 where c=3; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 const c c 5 const 1 +select * from t1 where c=3; +a +3 +create table t2 as select a,b,c from t1; +desc t2; +Field Type Null Key Default Extra +a int(11) YES NULL +b int(11) YES NULL +c int(11) YES NULL +explain select * from t1,t2 where t1.b = t2.c and t1.c = t2.b; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 ALL NULL NULL NULL NULL 10 +1 SIMPLE t1 ALL b,c NULL NULL NULL 10 Using where; Using join buffer (flat, BNL join) +select * from t1,t2 where t1.b = t2.c and t1.c = t2.b; +a a b c +1 1 1 1 +2 2 2 2 +3 3 3 3 +4 4 4 4 +drop table t1,t2; +#Unhide invisible columns +create table t1 (a int primary key, b int invisible, c int invisible unique); +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) NOT NULL, + `b` int(11) INVISIBLE DEFAULT NULL, + `c` int(11) INVISIBLE DEFAULT NULL, + PRIMARY KEY (`a`), + UNIQUE KEY `c` (`c`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +desc t1; +Field Type Null Key Default Extra +a int(11) NO PRI NULL +b int(11) YES NULL INVISIBLE +c int(11) YES UNI NULL INVISIBLE +alter table t1 modify column b int; +desc t1; +Field Type Null Key Default Extra +a int(11) NO PRI NULL +b int(11) YES NULL +c int(11) YES UNI NULL INVISIBLE +alter table t1 change column c d int; +desc t1; +Field Type Null Key Default Extra +a int(11) NO PRI NULL +b int(11) YES NULL +d int(11) YES UNI NULL +drop table t1; +SHOW STATUS LIKE 'Feature_invisible_columns'; +Variable_name Value +#invisible is non reserved +create table t1(a int unique , invisible int invisible, c int ); +desc t1; +Field Type Null Key Default Extra +a int(11) YES UNI NULL +invisible int(11) YES NULL INVISIBLE +c int(11) YES NULL +alter table t1 change column invisible hid int invisible; +desc t1; +Field Type Null Key Default Extra +a int(11) YES UNI NULL +hid int(11) YES NULL INVISIBLE +c int(11) YES NULL +drop table t1; +set debug_dbug= "+d,test_pseudo_invisible"; +create table t1(a int); +set debug_dbug="";
better do set @old_dbug=@@debug_dbug; ... set debug_dbug=@old_dbug; otherwise you you'll disable dbug when someone runs `./mtr --debug ...`
+desc t1; +Field Type Null Key Default Extra +a int(11) YES NULL +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +insert into t1 values(1); +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +drop table t1; +set debug_dbug= "+d,test_completely_invisible"; +create table t1(a int); +set debug_dbug=""; +desc t1; +Field Type Null Key Default Extra +a int(11) YES NULL +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +insert into t1 values(1); +select * from t1; +a +1 +select invisible ,a from t1; +ERROR 42S22: Unknown column 'invisible' in 'field list' +set debug_dbug= "+d,test_completely_invisible"; +select invisible ,a from t1; +invisible a +9 1 +set debug_dbug=""; +drop table t1; +set debug_dbug= "+d,test_pseudo_invisible"; +create table t1(a int); +set debug_dbug=""; +desc t1; +Field Type Null Key Default Extra +a int(11) YES NULL +insert into t1 values(1); +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 change invisible b int; +ERROR 42S22: Unknown column 'invisible' in 't1' +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 modify invisible char; +ERROR 42S22: Unknown column 'invisible' in 't1' +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 drop invisible; +ERROR 42000: Can't DROP COLUMN `invisible`; check that it exists +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 add invisible int; +ERROR 42S21: Duplicate column name 'invisible'
ouch. this violates the principle that these columns are *only* visible in SELECT. But, I guess, this effect is unavoidable :(
+select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 add invisible2 int default 2; +select * from t1; +a invisible2 +1 2 +select invisible ,a from t1; +invisible a +9 1 +drop table t1; +set debug_dbug= "+d,test_completely_invisible"; +create table t1(a int); +desc t1; +Field Type Null Key Default Extra +a int(11) YES NULL +insert into t1 values(1); +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 change invisible b int; +ERROR 42S22: Unknown column 'invisible' in 't1' +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 modify invisible char; +ERROR 42S22: Unknown column 'invisible' in 't1' +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 drop invisible; +ERROR 42000: Can't DROP COLUMN `invisible`; check that it exists +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 add invisible int; +ERROR 42S21: Duplicate column name 'invisible'
Nope, for a completely invisible column, this ALTER should work. just rename the completely invisible column to something else.
+select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +set debug_dbug=""; +ALTER table t1 add hid int default 2; +set debug_dbug= "+d,test_completely_invisible"; +select * from t1; +a hid +1 2 +select invisible ,a from t1; +invisible a +9 1 +drop table t1; +set debug_dbug=""; +Create table t1( a int default(99) invisible, b int); +insert into t1 values(1); +insert into t1 values(2); +insert into t1 values(3); +insert into t1 values(4); +select * from t1; +b +1 +2 +3 +4 +alter table t1 add index(a); +alter table t1 add index(a,b); +show index from t1; +Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment +t1 1 a 1 a A NULL NULL NULL YES BTREE +t1 1 a_2 1 a A NULL NULL NULL YES BTREE +t1 1 a_2 2 b A NULL NULL NULL YES BTREE +drop table t1; +set debug_dbug= "+d,test_pseudo_invisible"; +Create table t1( a int default(99) invisible, b int); +Create table t2( a int default(99) invisible, b int, unique(invisible)); +ERROR 42000: Key column 'invisible' doesn't exist in table +set debug_dbug=""; +insert into t1 values(1); +insert into t1 values(2); +insert into t1 values(3); +insert into t1 values(4); +select * from t1; +b +1 +2 +3 +4 +select invisible, a, b from t1; +invisible a b +9 99 1 +9 99 2 +9 99 3 +9 99 4 +alter table t1 add index(invisible); +ERROR 42000: Key column 'invisible' doesn't exist in table +alter table t1 add index(b,invisible); +ERROR 42000: Key column 'invisible' doesn't exist in table +show index from t1; +Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment +drop table t1; +set debug_dbug= "+d,test_completely_invisible"; +Create table t1( a int default(99) invisible, b int); +Create table t2( a int default(99) invisible, b int, unique(invisible)); +ERROR 42000: Key column 'invisible' doesn't exist in table +insert into t1 values(1); +insert into t1 values(2); +insert into t1 values(3); +insert into t1 values(4); +select * from t1; +b +1 +2 +3 +4 +select invisible, a, b from t1; +invisible a b +9 99 1 +9 99 2 +9 99 3 +9 99 4 +set debug_dbug=""; +alter table t1 add index(invisible); +ERROR 42000: Key column 'invisible' doesn't exist in table +alter table t1 add index(b,invisible); +ERROR 42000: Key column 'invisible' doesn't exist in table +show index from t1; +Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment +drop table t1; +set debug_dbug= "+d,test_completely_invisible,test_invisible_index";
hmm, please move all tests that use debug_dbug to a separate file. for example, hidden_field_debug.test so that hidden_field.test could also be run in non-debug builds. we only build with debug on fulltest2 builder, so currently your whole hidden_field.test is only run on that one single builder.
+Create table t1( a int default(99) , b int, index system_gen (invisible), index(b)); +set debug_dbug=""; +Show index from t1; +Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment +t1 1 b 1 b A NULL NULL NULL YES BTREE +select * from INFORMATION_SCHEMA.STATISTICS where TABLE_SCHEMA ='test' and table_name='t1'; +TABLE_CATALOG TABLE_SCHEMA TABLE_NAME NON_UNIQUE INDEX_SCHEMA INDEX_NAME SEQ_IN_INDEX COLUMN_NAME COLLATION CARDINALITY SUB_PART PACKED NULLABLE INDEX_TYPE COMMENT INDEX_COMMENT +def test t1 1 test b 1 b A NULL NULL NULL YES BTREE +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT 99, + `b` int(11) DEFAULT NULL, + KEY `b` (`b`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1
I've just realized, there's one problem with that. completely invisible columns must be completely invisible, as if they don't exist at all. So ALTER TABLE ... ADD COLUMN should work, as if a completely invisible column didn't exist. But there can be only 64 (?) indexes per table. So one will be able to notice a completely invisible index, because one won't be able to create all 64 indexes. I don't see any solution for this, unfortunately.
+insert into t1 values(1,1); +insert into t1 values(1,1); +insert into t1 values(1,1); +insert into t1 values(1,1); +set debug_dbug= "+d,test_completely_invisible"; +select invisible, a ,b from t1; +invisible a b +9 1 1 +9 1 1 +9 1 1 +9 1 1 +explain select * from t1 where invisible =9; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ref system_gen system_gen 5 const 3 +set debug_dbug=""; +alter table t1 add x int default 3; +set debug_dbug= "+d,test_completely_invisible"; +select invisible, a ,b from t1; +invisible a b +9 1 1 +9 1 1 +9 1 1 +9 1 1 +set debug_dbug=""; +Show index from t1; +Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment +t1 1 b 1 b A NULL NULL NULL YES BTREE +create index a1 on t1(invisible); +ERROR 42000: Key column 'invisible' doesn't exist in table +drop index system_gen on t1; +ERROR 42000: Can't DROP INDEX `system_gen`; check that it exists +drop table t1; diff --git a/mysql-test/t/hidden_binlog.test b/mysql-test/t/hidden_binlog.test new file mode 100644 index 00000000000..3e876dcca2e --- /dev/null +++ b/mysql-test/t/hidden_binlog.test @@ -0,0 +1,32 @@ +--source include/master-slave.inc + +--connection master +create table t1(a int , b int invisible); +insert into t1 values(1); +insert into t1(a,b) values(2,2); +select a,b from t1; +desc t1; + +create table t2(a int , b int invisible default 5); +insert into t1 values(1); +insert into t1(a,b) values(2,2);
typo: you want to insert into t2
+select a,b from t2; +desc t2; + + +--sync_slave_with_master +select * from t1; +select a,b from t1; +desc t1; +show create table t1; + +select * from t2; +select a,b from t2; +desc t2; +show create table t2; + + +--connection master +drop table t1,t2; + +--source include/rpl_end.inc diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt index d0fb0ee1772..ddd26e1c080 100644 --- a/sql/share/errmsg-utf8.txt +++ b/sql/share/errmsg-utf8.txt @@ -7779,3 +7779,5 @@ ER_COMPRESSED_COLUMN_USED_AS_KEY eng "Compressed column '%-.192s' can't be used in key specification" ER_UNKNOWN_COMPRESSION_METHOD eng "Unknown compression method: %s" +ER_INVISIBLE_NOT_NULL_WITHOUT_DEFAULT + eng "Hidden column '%s' must either have a default value or allow null values"
1. s/Hidden/Invisible/ 2. s/allow null values/be nullable/ 3. s/'%s'/%`s/ that is "Invisible column %`s must either have a default value or be nullable" or you can say simply "Invisible column %`s must have a default value" because a nullable column has a default value of NULL
diff --git a/sql/sql_class.h b/sql/sql_class.h index 92f28a4dc07..2dba3fab72e 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -303,22 +303,25 @@ class Key :public Sql_alloc, public DDL_options { LEX_CSTRING name; engine_option_value *option_list; bool generated; + bool system_generated_invisible;
again, why "system_generated_invisible"? what else can an invisible index be?
Key(enum Keytype type_par, const LEX_CSTRING *name_arg, ha_key_alg algorithm_arg, bool generated_arg, DDL_options_st ddl_options) :DDL_options(ddl_options), type(type_par), key_create_info(default_key_create_info), - name(*name_arg), option_list(NULL), generated(generated_arg) + name(*name_arg), option_list(NULL), generated(generated_arg), + system_generated_invisible(false) { key_create_info.algorithm= algorithm_arg; } Key(enum Keytype type_par, const LEX_CSTRING *name_arg, KEY_CREATE_INFO *key_info_arg, bool generated_arg, List<Key_part_spec> *cols, engine_option_value *create_opt, DDL_options_st ddl_options) :DDL_options(ddl_options), type(type_par), key_create_info(*key_info_arg), columns(*cols), - name(*name_arg), option_list(create_opt), generated(generated_arg) + name(*name_arg), option_list(create_opt), generated(generated_arg), + system_generated_invisible(false) {} Key(const Key &rhs, MEM_ROOT *mem_root); virtual ~Key() {} diff --git a/sql/field.cc b/sql/field.cc index 4449d0ecf31..6c6f4d31534 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -1596,7 +1596,7 @@ Field::Field(uchar *ptr_arg,uint32 length_arg,uchar *null_ptr_arg, unireg_check(unireg_check_arg), field_length(length_arg), null_bit(null_bit_arg), is_created_from_null_item(FALSE), read_stats(NULL), collected_stats(0), vcol_info(0), check_constraint(0), - default_value(0) + default_value(0),field_visibility(NOT_INVISIBLE)
Hmm, I think gcc emits a warning for this. You need to initialize fields in the same order you declare them. Either move field_visibility here to be after ptr(ptr_arg) or move it's declaration to be after default_value.
{ flags=null_ptr ? 0: NOT_NULL_FLAG; comment.str= (char*) ""; diff --git a/sql/table.h b/sql/table.h index 9ecec6d636c..4e42827b003 100644 --- a/sql/table.h +++ b/sql/table.h @@ -334,6 +334,16 @@ enum enum_vcol_update_mode VCOL_UPDATE_FOR_REPLACE };
+/* Field visibility enums */ + +enum field_visible_type{ + NOT_INVISIBLE= 0, + USER_DEFINED_INVISIBLE, + // pseudo-columns (like ROWID). Can be queried explicitly in SELECT, + //otherwise hidden from anything + PSEUDO_COLUMN_INVISIBLE,
no, don't call them pseudo-columns. In AS OF we'll have completely normal stored columns using this invisibility level. May be SYSTEM_INVISIBLE ? And a comment "automatically added by the server. Can be queried explicitly in SELECT, otherwise hidden from anything"
+ COMPLETELY_INVISIBLE +};
/** Category of table found in the table share. diff --git a/sql/table.cc b/sql/table.cc index 09eea1bb835..f16c591a4fe 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -1983,6 +1992,15 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, reg_field->field_index= i; reg_field->comment=comment; reg_field->vcol_info= vcol_info; + if(field_properties!=NULL) + { + uint temp= *field_properties++; + reg_field->field_visibility= static_cast<field_visible_type> (temp & 3);
make it a macro, like f_packtype(), etc (see the end of field.h)
+ } + if (reg_field->field_visibility == USER_DEFINED_INVISIBLE) + status_var_increment(thd->status_var.feature_hidden_columns); + if (reg_field->field_visibility == NOT_INVISIBLE) + share->visible_fields++; if (field_type == MYSQL_TYPE_BIT && !f_bit_as_char(pack_flag)) { null_bits_are_used= 1; diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 2fc5e3c27a5..b16903c9db8 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -8471,6 +8471,7 @@ SHOW_VAR status_vars[]= { {"Feature_dynamic_columns", (char*) offsetof(STATUS_VAR, feature_dynamic_columns), SHOW_LONG_STATUS}, {"Feature_fulltext", (char*) offsetof(STATUS_VAR, feature_fulltext), SHOW_LONG_STATUS}, {"Feature_gis", (char*) offsetof(STATUS_VAR, feature_gis), SHOW_LONG_STATUS}, + {"Feature_hidden_columns", (char*) offsetof(STATUS_VAR, feature_hidden_columns), SHOW_LONG_STATUS},
"invisibile" :)
{"Feature_locale", (char*) offsetof(STATUS_VAR, feature_locale), SHOW_LONG_STATUS}, {"Feature_subquery", (char*) offsetof(STATUS_VAR, feature_subquery), SHOW_LONG_STATUS}, {"Feature_timezone", (char*) offsetof(STATUS_VAR, feature_timezone), SHOW_LONG_STATUS}, diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 93dd6239749..a625d3a897e 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -5479,6 +5479,11 @@ find_field_in_table(THD *thd, TABLE *table, const char *name, uint length,
if (field_ptr && *field_ptr) { + if ((*field_ptr)->field_visibility == COMPLETELY_INVISIBLE) + { + DBUG_EVALUATE_IF("test_completely_invisible", {}, + {DBUG_RETURN((Field*) 0);});
rather unconventional use of DBUG_EVALUATE_IF :) ok
+ } *cached_field_index_ptr= field_ptr - table->field; field= *field_ptr; } @@ -7553,6 +7558,18 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name,
for (; !field_iterator.end_of_fields(); field_iterator.next()) { + /* field can be null here STODO->verify , shouldnt field be null for select * from table + test case + create table t1 (empnum smallint, grp int); + create table t2 (empnum int, name char(5)); + insert into t1 values(1,1); + insert into t2 values(1,'bob'); + create view v1 as select * from t2 inner join t1 using (empnum); + select * from v1; + */
Thanks for the test case. The comment could better say /* field() is always NULL for views (see, e.g. Field_iterator_view or Field_iterator_natural_join). But view fields can never be invisible. */ with this comment the test case is no longer needed here.
+ if ((field= field_iterator.field()) && + field->field_visibility != NOT_INVISIBLE) + continue; Item *item;
if (!(item= field_iterator.create_item(thd))) @@ -8153,13 +8170,19 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List<Item> &values, List<TABLE> tbl_list; bool all_fields_have_values= true; Item *value; - Field *field; + Field *field, **f; bool abort_on_warning_saved= thd->abort_on_warning; uint autoinc_index= table->next_number_field ? table->next_number_field->field_index : ~0U; + uint field_count= 0; + bool need_default_value= false; DBUG_ENTER("fill_record"); - + //TODO will fields count be alwats equal to table->fields ?
Nope, not always. You can see how fill_record() is called. For various internal temporary tables, **ptr can be table->field + something. For CREATE ... SELECT, it can also be table->field + something. In both these cases you shouldn't do anything special below. Internal temporary tables cannot have invisible fields, so that's fine. But CREATE ... SELECT can. Please, add the following test case: CREATE TABLE t1 (b int); INSERT t1 ... CREATE TABLE t2 (a int invisible) SELECT * FROM t1; CREATE TABLE t3 (b int, a int invisible) SELECT * FROM t1; CREATE TABLE t4 (b int invisible) SELECT * FROM t1; CREATE TABLE t2 (a int invisible) SELECT b as a FROM t1; and, may be, other variations of this theme. I *think* it'll mostly work fine, because CREATE ... SELECT sets missing fields to default values, and you do that too. So either way the result is the same. Anyway, I don't think you need to count fields here. Something simpler like if (table->s->tmp_table < INTERNAL_TMP_TABLE && values.elements != table->s->fields) need_default_value= true; it'll set need_default_value in the case of CREATE...SELECT as above, but it won't loop over all fields in all other cases. And most fill_record() invocations are not for CREATE...SELECT. So it'll be almost always better than a loop.
+ for (f= ptr; f && (field= *f); f++) + field_count++; + if (field_count != values.elements) + need_default_value= true; if (!*ptr) { /* No fields to update, quite strange!*/ @@ -8177,12 +8200,16 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List<Item> &values, only one row. */ table->auto_increment_field_not_null= FALSE; + Name_resolution_context *context= & thd->lex->select_lex.context; while ((field = *ptr++) && ! thd->is_error()) { /* Ensure that all fields are from the same table */ DBUG_ASSERT(field->table == table);
- value=v++; + if (need_default_value && field->field_visibility != NOT_INVISIBLE) + value = new (thd->mem_root) Item_default_value(thd,context); + else + value=v++;
1. Hmm, you will create a new Item for every inserted row? What if it's INSERT ... SELECT * FROM very_large_table? 2. please test inserts with stored procedures and prepared statements.
if (field->field_index == autoinc_index) table->auto_increment_field_not_null= TRUE; if (field->vcol_info) diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 68ded844938..a98bc0f5667 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -5568,6 +5579,8 @@ static int get_schema_column_record(THD *thd, TABLE_LIST *tables,
for (; (field= *ptr) ; ptr++) { + if(field->field_visibility > USER_DEFINED_INVISIBLE)
space before left parenthesys, please (here and everywhere, grep your patch to find other places)
+ continue; uchar *pos; char tmp[MAX_FIELD_WIDTH]; String type(tmp,sizeof(tmp), system_charset_info); @@ -5640,13 +5653,20 @@ static int get_schema_column_record(THD *thd, TABLE_LIST *tables, table->field[20]->store(STRING_WITH_LEN("ALWAYS"), cs);
if (field->vcol_info->stored_in_db) - table->field[17]->store(STRING_WITH_LEN("STORED GENERATED"), cs); + buf.set(STRING_WITH_LEN("STORED GENERATED"), cs); else - table->field[17]->store(STRING_WITH_LEN("VIRTUAL GENERATED"), cs); + buf.set(STRING_WITH_LEN("VIRTUAL GENERATED"), cs); } else table->field[20]->store(STRING_WITH_LEN("NEVER"), cs); - + /*hidden can coexist with auto_increment and virtual */ + if(field->field_visibility == USER_DEFINED_INVISIBLE) + { + if (buf.length()) + buf.append(STRING_WITH_LEN(",")); + buf.append(STRING_WITH_LEN(" INVISIBLE"),cs);
No space before "INVISIBLE". Instead, append STRING_WITH_LEN(", ")
+ } + table->field[17]->store(buf.ptr(), buf.length(), cs); table->field[19]->store(field->comment.str, field->comment.length, cs); if (schema_table_store_record(thd, table)) DBUG_RETURN(1); diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 4006e3aec4d..1c8398b0fd7 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -3233,7 +3233,37 @@ bool Column_definition::prepare_stage1_check_typelib_default() return false; }
- +/* + This function create a hidden field. + SYNOPSIS + mysql_create_hidden_field() + thd Thread Object + field_name + type_handler field data type + field_visibility + default value + RETURN VALUE + Create_field object +*/ +static +Create_field * mysql_create_hidden_field(THD *thd, const char *field_name, + Type_handler *type_handler, field_visible_type field_visibility, + Item* default_value) +{ + Create_field *fld= new(thd->mem_root)Create_field(); + fld->set_handler(type_handler); + fld->field_name.str= thd->strmake(field_name, strlen(field_name)); + fld->field_name.length= strlen(field_name); + fld->field_visibility= field_visibility; + if (default_value) + { + Virtual_column_info *v= new (thd->mem_root) Virtual_column_info(); + v->expr= default_value; + v->utf8= 0; + fld->default_value= v; + } + return fld; +}
This will be a compiler warning in optimized builds, because a static function is not used anywhere.
/* Preparation for table creation
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hi Serg! Thanks for the review. Please review the updated patch , at bb-hidden-test branch I am currently working on this , only 2 things are left. Changes apart from comments. Rebased to latest 10.3. Completely_invisible, or HA_INVISIBLE_INDEX is only deleted when same name column/ index is created(with tests). On Thu, Sep 28, 2017 at 8:15 PM, Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Sachin!
Here's a review of all changes between 474f51711b1 and f0e2cf3e6c8:
diff --git a/include/my_base.h b/include/my_base.h index b93300d7562..68c828d5f74 100644 --- a/include/my_base.h +++ b/include/my_base.h @@ -277,7 +277,8 @@ enum ha_base_keytype { This flag can be calculated -- it's based on key lengths comparison. */ #define HA_KEY_HAS_PART_KEY_SEG 65536 - +/* Internal Flag Can be calcaluted */ +#define HA_INVISIBLE_SYSTEM_KEY 2<<18 /* Is it a *Invisible* key */
"an invisible key"
and I'd just use HA_INVISIBLE_KEY
erledigt.
/* Automatic bits in key-flag */
#define HA_SPACE_PACK_USED 4 /* Test for if SPACE_PACK used */ diff --git a/mysql-test/r/features.result b/mysql-test/r/features.result index c6d1a6b0bac..3acd7436ad9 100644 --- a/mysql-test/r/features.result +++ b/mysql-test/r/features.result @@ -8,6 +8,7 @@ Feature_delay_key_write 0 Feature_dynamic_columns 0 Feature_fulltext 0 Feature_gis 0 +Feature_hidden_columns 0
You forgot to rename the variable. and let's rename test files too, it's a bit confusing now.
Done. I guess in all test files I have replaced hidden with invisible. Apart from this I forgot this change.
Feature_locale 0 Feature_subquery 0 Feature_timezone 0 diff --git a/mysql-test/r/hidden_field.result b/mysql-test/r/hidden_field.result new file mode 100644 index 00000000000..7ff6f617eef --- /dev/null +++ b/mysql-test/r/hidden_field.result @@ -0,0 +1,692 @@ +FLUSH STATUS; +create table t1(abc int primary key, xyz int invisible); +SHOW STATUS LIKE 'Feature_invisible_columns'; +Variable_name Value +desc t1; +Field Type Null Key Default Extra +abc int(11) NO PRI NULL +xyz int(11) YES NULL INVISIBLE +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `abc` int(11) NOT NULL, + `xyz` int(11) INVISIBLE DEFAULT NULL, + PRIMARY KEY (`abc`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +select * from INFORMATION_SCHEMA.COLUMNS where TABLE_SCHEMA='test' and TABLE_NAME='t1'; +TABLE_CATALOG TABLE_SCHEMA TABLE_NAME COLUMN_NAME ORDINAL_POSITION COLUMN_DEFAULT IS_NULLABLE DATA_TYPE CHARACTER_MAXIMUM_LENGTH CHARACTER_OCTET_LENGTH NUMERIC_PRECISION NUMERIC_SCALE DATETIME_PRECISION CHARACTER_SET_NAME COLLATION_NAME COLUMN_TYPE COLUMN_KEY EXTRA PRIVILEGES COLUMN_COMMENT IS_GENERATED GENERATION_EXPRESSION +def test t1 abc 1 NULL NO int NULL NULL 10 0 NULL NULL NULL int(11) PRI select,insert,update,references NEVER NULL +def test t1 xyz 2 NULL YES int NULL NULL 10 0 NULL NULL NULL int(11) INVISIBLE select,insert,update,references NEVER NULL +drop table t1; +create table t1(a1 int invisible); +ERROR 42000: A table must have at least 1 column +create table t1(a1 blob,invisible(a1)); +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '(a1))' at line 1 +create table t1(a1 int primary key invisible ,a2 int unique invisible , a3 blob,a4 int not null invisible unique); +ERROR HY000: Hidden column 'a1' must either have a default value or allow null values +create table t1(abc int not null invisible); +ERROR HY000: Hidden column 'abc' must either have a default value or allow null values +create table t1(a int invisible, b int); +insert into t1 values(1); +insert into t1(a) values(2); +insert into t1(b) values(3); +insert into t1(a,b) values(5,5); +select * from t1; +b +1 +NULL +3 +5 +select a,b from t1; +a b +NULL 1 +2 NULL +NULL 3 +5 5 +delete from t1; +insert into t1 values(1),(2),(3),(4); +select * from t1; +b +1 +2 +3 +4 +select a from t1; +a +NULL +NULL +NULL +NULL +drop table t1; +#more complex case of invisible +create table t1(a int , b int invisible , c int invisible auto_increment unique, d blob , e int unique, f int); +desc t1; +Field Type Null Key Default Extra +a int(11) YES NULL +b int(11) YES NULL INVISIBLE
remove a space character before "INVISIBLE" Okay.
+c int(11) NO PRI NULL auto_increment, INVISIBLE +d blob YES NULL +e int(11) YES UNI NULL +f int(11) YES NULL +insert into t1 values(1,'d blob',1,1),(1,'d blob',11,1),(1,'d blob',2,1),(1,'d blob',3,1),(1,'d blob',41,1); +select * from t1; +a d e f +1 d blob 1 1 +1 d blob 11 1 +1 d blob 2 1 +1 d blob 3 1 +1 d blob 41 1
would be nice to see `select a,b,c,d,e,f` here
Done.
+drop table t1; +create table sdsdsd(a int , b int, invisible(a,b)); +ERROR 42000: You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near '(a,b))' at line 1 +create table t1(a int,abc int as (a mod 3) virtual invisible); +desc t1; +Field Type Null Key Default Extra +a int(11) YES NULL +abc int(11) YES NULL VIRTUAL GENERATED, INVISIBLE +insert into t1 values(1,default); +ERROR 21S01: Column count doesn't match value count at row 1 +insert into t1 values(1),(22),(233); +select * from t1; +a +1 +22 +233 +select a,abc from t1; +a abc +1 1 +22 1 +233 2 +drop table t1; +create table t1(abc int primary key invisible auto_increment, a int); +desc t1; +Field Type Null Key Default Extra +abc int(11) NO PRI NULL auto_increment, INVISIBLE +a int(11) YES NULL +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `abc` int(11) NOT NULL INVISIBLE AUTO_INCREMENT, + `a` int(11) DEFAULT NULL, + PRIMARY KEY (`abc`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +insert into t1 values(1); +insert into t1 values(2); +insert into t1 values(3); +select * from t1; +a +1 +2 +3 +select abc,a from t1; +abc a +1 1 +2 2 +3 3 +delete from t1; +insert into t1 values(1),(2),(3),(4),(6); +select abc,a from t1; +abc a +4 1 +5 2 +6 3 +7 4 +8 6 +drop table t1; +create table t1(abc int); +alter table t1 change abc ss int invisible; +ERROR 42000: A table must have at least 1 column +alter table t1 add column xyz int; +alter table t1 modify column abc int ; +desc t1; +Field Type Null Key Default Extra +abc int(11) YES NULL +xyz int(11) YES NULL +insert into t1 values(22); +ERROR 21S01: Column count doesn't match value count at row 1 +alter table t1 modify column abc int invisible; +desc t1; +Field Type Null Key Default Extra +abc int(11) YES NULL INVISIBLE +xyz int(11) YES NULL +insert into t1 values(12); +drop table t1; +#some test on copy table structure with table data; +#table with invisible fields and unique keys; +create table t1(a int , b int invisible , c int invisible auto_increment unique, d blob , e int unique, f int); +desc t1; +Field Type Null Key Default Extra +a int(11) YES NULL +b int(11) YES NULL INVISIBLE +c int(11) NO PRI NULL auto_increment, INVISIBLE +d blob YES NULL +e int(11) YES UNI NULL +f int(11) YES NULL +insert into t1 values(1,'d blob',1,1),(1,'d blob',11,1),(1,'d blob',2,1),(1,'d blob',3,1),(1,'d blob',41,1); +select * from t1; +a d e f +1 d blob 1 1 +1 d blob 11 1 +1 d blob 2 1 +1 d blob 3 1 +1 d blob 41 1 +select a,b,c,d,e,f from t1; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +#this wont copy invisible fields and keys;
won't
Done
+create table t2 as select * from t1; +desc t2; +Field Type Null Key Default Extra +a int(11) YES NULL +d blob YES NULL +e int(11) YES NULL +f int(11) YES NULL +select * from t2; +a d e f +1 d blob 1 1 +1 d blob 11 1 +1 d blob 2 1 +1 d blob 3 1 +1 d blob 41 1 +select a,b,c,d,e,f from t2; +ERROR 42S22: Unknown column 'b' in 'field list' +drop table t2; +#now this will copy invisible fields +create table t2 as select a,b,c,d,e,f from t1; +desc t2; +Field Type Null Key Default Extra +a int(11) YES NULL +b int(11) YES NULL +c int(11) NO 0 +d blob YES NULL +e int(11) YES NULL +f int(11) YES NULL +select * from t2; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +select a,b,c,d,e,f from t2; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +drop table t2,t1; +#some test related to copy of data from one table to another; +create table t1(a int , b int invisible , c int invisible auto_increment unique, d blob , e int unique, f int); +insert into t1 values(1,'d blob',1,1),(1,'d blob',11,1),(1,'d blob',2,1),(1,'d blob',3,1),(1,'d blob',41,1); +select a,b,c,d,e,f from t1; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +create table t2(a int , b int invisible , c int invisible , d blob , e int unique, f int); +insert into t2 select * from t1; +select a,b,c,d,e,f from t2; +a b c d e f +1 NULL NULL d blob 1 1 +1 NULL NULL d blob 11 1 +1 NULL NULL d blob 2 1 +1 NULL NULL d blob 3 1 +1 NULL NULL d blob 41 1 +truncate t2; +insert into t2 (a,b,c,d,e,f) select a,b,c,d,e,f from t1; +select a,b,c,d,e,f from t2; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +truncate t2; +drop table t1,t2; +#some test related to creating view on table with invisible column; +create table t1(a int , b int invisible , c int invisible auto_increment unique, d blob , e int unique, f int); +insert into t1 values(1,'d blob',1,1),(1,'d blob',11,1),(1,'d blob',2,1),(1,'d blob',3,1),(1,'d blob',41,1); +create view v as select * from t1; +desc v; +Field Type Null Key Default Extra +a int(11) YES NULL +d blob YES NULL +e int(11) YES NULL +f int(11) YES NULL +select * from v; +a d e f +1 d blob 1 1 +1 d blob 11 1 +1 d blob 2 1 +1 d blob 3 1 +1 d blob 41 1 +#v does not have invisible column; +select a,b,c,d,e,f from v; +ERROR 42S22: Unknown column 'b' in 'field list' +insert into v values(1,21,32,4); +select * from v; +a d e f +1 d blob 1 1 +1 d blob 11 1 +1 d blob 2 1 +1 d blob 3 1 +1 d blob 41 1 +1 21 32 4 +insert into v(a,b,c,d,e,f) values(1,12,3,4,5,6); +ERROR 42S22: Unknown column 'b' in 'field list'
good tests, with views. this behavior is not obvious
right.
+drop view v; +create view v as select a,b,c,d,e,f from t1; +desc v; +Field Type Null Key Default Extra +a int(11) YES NULL +b int(11) YES NULL +c int(11) NO 0 +d blob YES NULL +e int(11) YES NULL +f int(11) YES NULL +select * from v; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +1 NULL 6 21 32 4 +#v does have invisible column but they aren't invisible anymore. +select a,b,c,d,e,f from v; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +1 NULL 6 21 32 4 +insert into v values(1,26,33,4,45,66); +select a,b,c,d,e,f from v; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +1 NULL 6 21 32 4 +1 26 33 4 45 66 +insert into v(a,b,c,d,e,f) values(1,32,31,41,5,6); +select a,b,c,d,e,f from v; +a b c d e f +1 NULL 1 d blob 1 1 +1 NULL 2 d blob 11 1 +1 NULL 3 d blob 2 1 +1 NULL 4 d blob 3 1 +1 NULL 5 d blob 41 1 +1 NULL 6 21 32 4 +1 26 33 4 45 66 +1 32 31 41 5 6 +drop view v; +drop table t1; +#now invisible column in where and some join query +create table t1 (a int unique , b int invisible unique, c int unique invisible); +insert into t1(a,b,c) values(1,1,1); +insert into t1(a,b,c) values(2,2,2); +insert into t1(a,b,c) values(3,3,3); +insert into t1(a,b,c) values(4,4,4); +insert into t1(a,b,c) values(21,21,26); +insert into t1(a,b,c) values(31,31,35); +insert into t1(a,b,c) values(41,41,45); +insert into t1(a,b,c) values(22,22,24); +insert into t1(a,b,c) values(32,32,33); +insert into t1(a,b,c) values(42,42,43); +explain select * from t1 where b=3; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 const b b 5 const 1 +select * from t1 where b=3; +a +3 +explain select * from t1 where c=3; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 const c c 5 const 1 +select * from t1 where c=3; +a +3 +create table t2 as select a,b,c from t1; +desc t2; +Field Type Null Key Default Extra +a int(11) YES NULL +b int(11) YES NULL +c int(11) YES NULL +explain select * from t1,t2 where t1.b = t2.c and t1.c = t2.b; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t2 ALL NULL NULL NULL NULL 10 +1 SIMPLE t1 ALL b,c NULL NULL NULL 10 Using where; Using join buffer (flat, BNL join) +select * from t1,t2 where t1.b = t2.c and t1.c = t2.b; +a a b c +1 1 1 1 +2 2 2 2 +3 3 3 3 +4 4 4 4 +drop table t1,t2; +#Unhide invisible columns +create table t1 (a int primary key, b int invisible, c int invisible unique); +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) NOT NULL, + `b` int(11) INVISIBLE DEFAULT NULL, + `c` int(11) INVISIBLE DEFAULT NULL, + PRIMARY KEY (`a`), + UNIQUE KEY `c` (`c`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +desc t1; +Field Type Null Key Default Extra +a int(11) NO PRI NULL +b int(11) YES NULL INVISIBLE +c int(11) YES UNI NULL INVISIBLE +alter table t1 modify column b int; +desc t1; +Field Type Null Key Default Extra +a int(11) NO PRI NULL +b int(11) YES NULL +c int(11) YES UNI NULL INVISIBLE +alter table t1 change column c d int; +desc t1; +Field Type Null Key Default Extra +a int(11) NO PRI NULL +b int(11) YES NULL +d int(11) YES UNI NULL +drop table t1; +SHOW STATUS LIKE 'Feature_invisible_columns'; +Variable_name Value +#invisible is non reserved +create table t1(a int unique , invisible int invisible, c int ); +desc t1; +Field Type Null Key Default Extra +a int(11) YES UNI NULL +invisible int(11) YES NULL INVISIBLE +c int(11) YES NULL +alter table t1 change column invisible hid int invisible; +desc t1; +Field Type Null Key Default Extra +a int(11) YES UNI NULL +hid int(11) YES NULL INVISIBLE +c int(11) YES NULL +drop table t1; +set debug_dbug= "+d,test_pseudo_invisible"; +create table t1(a int); +set debug_dbug="";
better do
set @old_dbug=@@debug_dbug; ... set debug_dbug=@old_dbug;
otherwise you you'll disable dbug when someone runs `./mtr --debug ...`
done.
+desc t1; +Field Type Null Key Default Extra +a int(11) YES NULL +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +insert into t1 values(1); +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +drop table t1; +set debug_dbug= "+d,test_completely_invisible"; +create table t1(a int); +set debug_dbug=""; +desc t1; +Field Type Null Key Default Extra +a int(11) YES NULL +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT NULL +) ENGINE=MyISAM DEFAULT CHARSET=latin1 +insert into t1 values(1); +select * from t1; +a +1 +select invisible ,a from t1; +ERROR 42S22: Unknown column 'invisible' in 'field list' +set debug_dbug= "+d,test_completely_invisible"; +select invisible ,a from t1; +invisible a +9 1 +set debug_dbug=""; +drop table t1; +set debug_dbug= "+d,test_pseudo_invisible"; +create table t1(a int); +set debug_dbug=""; +desc t1; +Field Type Null Key Default Extra +a int(11) YES NULL +insert into t1 values(1); +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 change invisible b int; +ERROR 42S22: Unknown column 'invisible' in 't1' +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 modify invisible char; +ERROR 42S22: Unknown column 'invisible' in 't1' +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 drop invisible; +ERROR 42000: Can't DROP COLUMN `invisible`; check that it exists +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 add invisible int; +ERROR 42S21: Duplicate column name 'invisible'
ouch. this violates the principle that these columns are *only* visible in SELECT. But, I guess, this effect is unavoidable :(
Right, Because if want to give correct error then this is unavoidable.
+select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 add invisible2 int default 2; +select * from t1; +a invisible2 +1 2 +select invisible ,a from t1; +invisible a +9 1 +drop table t1; +set debug_dbug= "+d,test_completely_invisible"; +create table t1(a int); +desc t1; +Field Type Null Key Default Extra +a int(11) YES NULL +insert into t1 values(1); +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 change invisible b int; +ERROR 42S22: Unknown column 'invisible' in 't1' +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 modify invisible char; +ERROR 42S22: Unknown column 'invisible' in 't1' +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 drop invisible; +ERROR 42000: Can't DROP COLUMN `invisible`; check that it exists +select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +ALTER table t1 add invisible int; +ERROR 42S21: Duplicate column name 'invisible'
Nope, for a completely invisible column, this ALTER should work. just rename the completely invisible column to something else.
Done.
+select * from t1; +a +1 +select invisible ,a from t1; +invisible a +9 1 +set debug_dbug=""; +ALTER table t1 add hid int default 2; +set debug_dbug= "+d,test_completely_invisible"; +select * from t1; +a hid +1 2 +select invisible ,a from t1; +invisible a +9 1 +drop table t1; +set debug_dbug=""; +Create table t1( a int default(99) invisible, b int); +insert into t1 values(1); +insert into t1 values(2); +insert into t1 values(3); +insert into t1 values(4); +select * from t1; +b +1 +2 +3 +4 +alter table t1 add index(a); +alter table t1 add index(a,b); +show index from t1; +Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment +t1 1 a 1 a A NULL NULL NULL YES BTREE +t1 1 a_2 1 a A NULL NULL NULL YES BTREE +t1 1 a_2 2 b A NULL NULL NULL YES BTREE +drop table t1; +set debug_dbug= "+d,test_pseudo_invisible"; +Create table t1( a int default(99) invisible, b int); +Create table t2( a int default(99) invisible, b int, unique(invisible)); +ERROR 42000: Key column 'invisible' doesn't exist in table +set debug_dbug=""; +insert into t1 values(1); +insert into t1 values(2); +insert into t1 values(3); +insert into t1 values(4); +select * from t1; +b +1 +2 +3 +4 +select invisible, a, b from t1; +invisible a b +9 99 1 +9 99 2 +9 99 3 +9 99 4 +alter table t1 add index(invisible); +ERROR 42000: Key column 'invisible' doesn't exist in table +alter table t1 add index(b,invisible); +ERROR 42000: Key column 'invisible' doesn't exist in table +show index from t1; +Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment +drop table t1; +set debug_dbug= "+d,test_completely_invisible"; +Create table t1( a int default(99) invisible, b int); +Create table t2( a int default(99) invisible, b int, unique(invisible)); +ERROR 42000: Key column 'invisible' doesn't exist in table +insert into t1 values(1); +insert into t1 values(2); +insert into t1 values(3); +insert into t1 values(4); +select * from t1; +b +1 +2 +3 +4 +select invisible, a, b from t1; +invisible a b +9 99 1 +9 99 2 +9 99 3 +9 99 4 +set debug_dbug=""; +alter table t1 add index(invisible); +ERROR 42000: Key column 'invisible' doesn't exist in table +alter table t1 add index(b,invisible); +ERROR 42000: Key column 'invisible' doesn't exist in table +show index from t1; +Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment +drop table t1; +set debug_dbug= "+d,test_completely_invisible,test_invisible_index";
hmm, please move all tests that use debug_dbug to a separate file. for example, hidden_field_debug.test so that hidden_field.test could also be run in non-debug builds.
we only build with debug on fulltest2 builder, so currently your whole hidden_field.test is only run on that one single builder.
Done.
+Create table t1( a int default(99) , b int, index system_gen (invisible), index(b)); +set debug_dbug=""; +Show index from t1; +Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment +t1 1 b 1 b A NULL NULL NULL YES BTREE +select * from INFORMATION_SCHEMA.STATISTICS where TABLE_SCHEMA ='test' and table_name='t1'; +TABLE_CATALOG TABLE_SCHEMA TABLE_NAME NON_UNIQUE INDEX_SCHEMA INDEX_NAME SEQ_IN_INDEX COLUMN_NAME COLLATION CARDINALITY SUB_PART PACKED NULLABLE INDEX_TYPE COMMENT INDEX_COMMENT +def test t1 1 test b 1 b A NULL NULL NULL YES BTREE +show create table t1; +Table Create Table +t1 CREATE TABLE `t1` ( + `a` int(11) DEFAULT 99, + `b` int(11) DEFAULT NULL, + KEY `b` (`b`) +) ENGINE=MyISAM DEFAULT CHARSET=latin1
I've just realized, there's one problem with that. completely invisible columns must be completely invisible, as if they don't exist at all. So ALTER TABLE ... ADD COLUMN should work, as if a completely invisible column didn't exist.
Done.
But there can be only 64 (?) indexes per table. So one will be able to notice a completely invisible index, because one won't be able to create all 64 indexes.
I don't see any solution for this, unfortunately.
+insert into t1 values(1,1); +insert into t1 values(1,1); +insert into t1 values(1,1); +insert into t1 values(1,1); +set debug_dbug= "+d,test_completely_invisible"; +select invisible, a ,b from t1; +invisible a b +9 1 1 +9 1 1 +9 1 1 +9 1 1 +explain select * from t1 where invisible =9; +id select_type table type possible_keys key key_len ref rows Extra +1 SIMPLE t1 ref system_gen system_gen 5 const 3 +set debug_dbug=""; +alter table t1 add x int default 3; +set debug_dbug= "+d,test_completely_invisible"; +select invisible, a ,b from t1; +invisible a b +9 1 1 +9 1 1 +9 1 1 +9 1 1 +set debug_dbug=""; +Show index from t1; +Table Non_unique Key_name Seq_in_index Column_name Collation Cardinality Sub_part Packed Null Index_type Comment Index_comment +t1 1 b 1 b A NULL NULL NULL YES BTREE +create index a1 on t1(invisible); +ERROR 42000: Key column 'invisible' doesn't exist in table +drop index system_gen on t1; +ERROR 42000: Can't DROP INDEX `system_gen`; check that it exists +drop table t1; diff --git a/mysql-test/t/hidden_binlog.test b/mysql-test/t/hidden_binlog.test new file mode 100644 index 00000000000..3e876dcca2e --- /dev/null +++ b/mysql-test/t/hidden_binlog.test @@ -0,0 +1,32 @@ +--source include/master-slave.inc + +--connection master +create table t1(a int , b int invisible); +insert into t1 values(1); +insert into t1(a,b) values(2,2); +select a,b from t1; +desc t1; + +create table t2(a int , b int invisible default 5); +insert into t1 values(1); +insert into t1(a,b) values(2,2);
typo: you want to insert into t2
Done.
+select a,b from t2; +desc t2; + + +--sync_slave_with_master +select * from t1; +select a,b from t1; +desc t1; +show create table t1; + +select * from t2; +select a,b from t2; +desc t2; +show create table t2; + + +--connection master +drop table t1,t2; + +--source include/rpl_end.inc diff --git a/sql/share/errmsg-utf8.txt b/sql/share/errmsg-utf8.txt index d0fb0ee1772..ddd26e1c080 100644 --- a/sql/share/errmsg-utf8.txt +++ b/sql/share/errmsg-utf8.txt @@ -7779,3 +7779,5 @@ ER_COMPRESSED_COLUMN_USED_AS_KEY eng "Compressed column '%-.192s' can't be used in key specification" ER_UNKNOWN_COMPRESSION_METHOD eng "Unknown compression method: %s" +ER_INVISIBLE_NOT_NULL_WITHOUT_DEFAULT + eng "Hidden column '%s' must either have a default value or allow null values"
1. s/Hidden/Invisible/ 2. s/allow null values/be nullable/ 3. s/'%s'/%`s/
that is "Invisible column %`s must either have a default value or be nullable"
or you can say simply "Invisible column %`s must have a default value" Done , used this. because a nullable column has a default value of NULL
diff --git a/sql/sql_class.h b/sql/sql_class.h index 92f28a4dc07..2dba3fab72e 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -303,22 +303,25 @@ class Key :public Sql_alloc, public DDL_options { LEX_CSTRING name; engine_option_value *option_list; bool generated; + bool system_generated_invisible;
again, why "system_generated_invisible"? what else can an invisible index be?
Changed.
Key(enum Keytype type_par, const LEX_CSTRING *name_arg, ha_key_alg algorithm_arg, bool generated_arg, DDL_options_st ddl_options) :DDL_options(ddl_options), type(type_par), key_create_info(default_key_create_info), - name(*name_arg), option_list(NULL), generated(generated_arg) + name(*name_arg), option_list(NULL), generated(generated_arg), + system_generated_invisible(false) { key_create_info.algorithm= algorithm_arg; } Key(enum Keytype type_par, const LEX_CSTRING *name_arg, KEY_CREATE_INFO *key_info_arg, bool generated_arg, List<Key_part_spec> *cols, engine_option_value *create_opt, DDL_options_st ddl_options) :DDL_options(ddl_options), type(type_par), key_create_info(*key_info_arg), columns(*cols), - name(*name_arg), option_list(create_opt), generated(generated_arg) + name(*name_arg), option_list(create_opt), generated(generated_arg), + system_generated_invisible(false) {} Key(const Key &rhs, MEM_ROOT *mem_root); virtual ~Key() {} diff --git a/sql/field.cc b/sql/field.cc index 4449d0ecf31..6c6f4d31534 100644 --- a/sql/field.cc +++ b/sql/field.cc @@ -1596,7 +1596,7 @@ Field::Field(uchar *ptr_arg,uint32 length_arg,uchar *null_ptr_arg, unireg_check(unireg_check_arg), field_length(length_arg), null_bit(null_bit_arg), is_created_from_null_item(FALSE), read_stats(NULL), collected_stats(0), vcol_info(0), check_constraint(0), - default_value(0) + default_value(0),field_visibility(NOT_INVISIBLE)
Hmm, I think gcc emits a warning for this. You need to initialize fields in the same order you declare them. Either move field_visibility here to be after ptr(ptr_arg) or move it's declaration to be after default_value.
Done.
{ flags=null_ptr ? 0: NOT_NULL_FLAG; comment.str= (char*) ""; diff --git a/sql/table.h b/sql/table.h index 9ecec6d636c..4e42827b003 100644 --- a/sql/table.h +++ b/sql/table.h @@ -334,6 +334,16 @@ enum enum_vcol_update_mode VCOL_UPDATE_FOR_REPLACE };
+/* Field visibility enums */ + +enum field_visible_type{ + NOT_INVISIBLE= 0, + USER_DEFINED_INVISIBLE, + // pseudo-columns (like ROWID). Can be queried explicitly in SELECT, + //otherwise hidden from anything + PSEUDO_COLUMN_INVISIBLE,
no, don't call them pseudo-columns. In AS OF we'll have completely normal stored columns using this invisibility level. May be SYSTEM_INVISIBLE ?
And a comment "automatically added by the server. Can be queried explicitly in SELECT, otherwise hidden from anything"
Done.
+ COMPLETELY_INVISIBLE +};
/** Category of table found in the table share. diff --git a/sql/table.cc b/sql/table.cc index 09eea1bb835..f16c591a4fe 100644 --- a/sql/table.cc +++ b/sql/table.cc @@ -1983,6 +1992,15 @@ int TABLE_SHARE::init_from_binary_frm_image(THD *thd, bool write, reg_field->field_index= i; reg_field->comment=comment; reg_field->vcol_info= vcol_info; + if(field_properties!=NULL) + { + uint temp= *field_properties++; + reg_field->field_visibility= static_cast<field_visible_type> (temp & 3);
make it a macro, like f_packtype(), etc (see the end of field.h)
Done
+ } + if (reg_field->field_visibility == USER_DEFINED_INVISIBLE) + status_var_increment(thd->status_var.feature_hidden_columns); + if (reg_field->field_visibility == NOT_INVISIBLE) + share->visible_fields++; if (field_type == MYSQL_TYPE_BIT && !f_bit_as_char(pack_flag)) { null_bits_are_used= 1; diff --git a/sql/mysqld.cc b/sql/mysqld.cc index 2fc5e3c27a5..b16903c9db8 100644 --- a/sql/mysqld.cc +++ b/sql/mysqld.cc @@ -8471,6 +8471,7 @@ SHOW_VAR status_vars[]= { {"Feature_dynamic_columns", (char*) offsetof(STATUS_VAR, feature_dynamic_columns), SHOW_LONG_STATUS}, {"Feature_fulltext", (char*) offsetof(STATUS_VAR, feature_fulltext), SHOW_LONG_STATUS}, {"Feature_gis", (char*) offsetof(STATUS_VAR, feature_gis), SHOW_LONG_STATUS}, + {"Feature_hidden_columns", (char*) offsetof(STATUS_VAR, feature_hidden_columns), SHOW_LONG_STATUS},
"invisibile" :)
Done.
{"Feature_locale", (char*) offsetof(STATUS_VAR, feature_locale), SHOW_LONG_STATUS}, {"Feature_subquery", (char*) offsetof(STATUS_VAR, feature_subquery), SHOW_LONG_STATUS}, {"Feature_timezone", (char*) offsetof(STATUS_VAR, feature_timezone), SHOW_LONG_STATUS}, diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 93dd6239749..a625d3a897e 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -5479,6 +5479,11 @@ find_field_in_table(THD *thd, TABLE *table, const char *name, uint length,
if (field_ptr && *field_ptr) { + if ((*field_ptr)->field_visibility == COMPLETELY_INVISIBLE) + { + DBUG_EVALUATE_IF("test_completely_invisible", {}, + {DBUG_RETURN((Field*) 0);});
rather unconventional use of DBUG_EVALUATE_IF :) ok
+ } *cached_field_index_ptr= field_ptr - table->field; field= *field_ptr; } @@ -7553,6 +7558,18 @@ insert_fields(THD *thd, Name_resolution_context *context, const char *db_name,
for (; !field_iterator.end_of_fields(); field_iterator.next()) { + /* field can be null here STODO->verify , shouldnt field be null for select * from table + test case + create table t1 (empnum smallint, grp int); + create table t2 (empnum int, name char(5)); + insert into t1 values(1,1); + insert into t2 values(1,'bob'); + create view v1 as select * from t2 inner join t1 using (empnum); + select * from v1; + */
Thanks for the test case. The comment could better say /* field() is always NULL for views (see, e.g. Field_iterator_view or Field_iterator_natural_join). But view fields can never be invisible. */ with this comment the test case is no longer needed here.
Done.
+ if ((field= field_iterator.field()) && + field->field_visibility != NOT_INVISIBLE) + continue; Item *item;
if (!(item= field_iterator.create_item(thd))) @@ -8153,13 +8170,19 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List<Item> &values, List<TABLE> tbl_list; bool all_fields_have_values= true; Item *value; - Field *field; + Field *field, **f; bool abort_on_warning_saved= thd->abort_on_warning; uint autoinc_index= table->next_number_field ? table->next_number_field->field_index : ~0U; + uint field_count= 0; + bool need_default_value= false; DBUG_ENTER("fill_record"); - + //TODO will fields count be alwats equal to table->fields ?
Nope, not always. You can see how fill_record() is called. For various internal temporary tables, **ptr can be table->field + something. For CREATE ... SELECT, it can also be table->field + something. In both these cases you shouldn't do anything special below.
Internal temporary tables cannot have invisible fields, so that's fine. But CREATE ... SELECT can. Please, add the following test case:
CREATE TABLE t1 (b int); INSERT t1 ... CREATE TABLE t2 (a int invisible) SELECT * FROM t1; CREATE TABLE t3 (b int, a int invisible) SELECT * FROM t1; CREATE TABLE t4 (b int invisible) SELECT * FROM t1; CREATE TABLE t2 (a int invisible) SELECT b as a FROM t1;
and, may be, other variations of this theme. I *think* it'll mostly work fine, because CREATE ... SELECT sets missing fields to default values, and you do that too. So either way the result is the same.
Anyway, I don't think you need to count fields here. Something simpler like if (table->s->tmp_table < INTERNAL_TMP_TABLE && values.elements != table->s->fields) need_default_value= true;
it'll set need_default_value in the case of CREATE...SELECT as above, but it won't loop over all fields in all other cases. And most fill_record() invocations are not for CREATE...SELECT. So it'll be almost always better than a loop.
//TODO
+ for (f= ptr; f && (field= *f); f++) + field_count++; + if (field_count != values.elements) + need_default_value= true; if (!*ptr) { /* No fields to update, quite strange!*/ @@ -8177,12 +8200,16 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List<Item> &values, only one row. */ table->auto_increment_field_not_null= FALSE; + Name_resolution_context *context= & thd->lex->select_lex.context; while ((field = *ptr++) && ! thd->is_error()) { /* Ensure that all fields are from the same table */ DBUG_ASSERT(field->table == table);
- value=v++; + if (need_default_value && field->field_visibility != NOT_INVISIBLE) + value = new (thd->mem_root) Item_default_value(thd,context); + else + value=v++;
1. Hmm, you will create a new Item for every inserted row? What if it's INSERT ... SELECT * FROM very_large_table?
2. please test inserts with stored procedures and prepared statements.
//TODO
if (field->field_index == autoinc_index) table->auto_increment_field_not_null= TRUE; if (field->vcol_info) diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 68ded844938..a98bc0f5667 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -5568,6 +5579,8 @@ static int get_schema_column_record(THD *thd, TABLE_LIST *tables,
for (; (field= *ptr) ; ptr++) { + if(field->field_visibility > USER_DEFINED_INVISIBLE)
space before left parenthesys, please (here and everywhere, grep your patch to find other places)
Okay.
+ continue; uchar *pos; char tmp[MAX_FIELD_WIDTH]; String type(tmp,sizeof(tmp), system_charset_info); @@ -5640,13 +5653,20 @@ static int get_schema_column_record(THD *thd, TABLE_LIST *tables, table->field[20]->store(STRING_WITH_LEN("ALWAYS"), cs);
if (field->vcol_info->stored_in_db) - table->field[17]->store(STRING_WITH_LEN("STORED GENERATED"), cs); + buf.set(STRING_WITH_LEN("STORED GENERATED"), cs); else - table->field[17]->store(STRING_WITH_LEN("VIRTUAL GENERATED"), cs); + buf.set(STRING_WITH_LEN("VIRTUAL GENERATED"), cs); } else table->field[20]->store(STRING_WITH_LEN("NEVER"), cs); - + /*hidden can coexist with auto_increment and virtual */ + if(field->field_visibility == USER_DEFINED_INVISIBLE) + { + if (buf.length()) + buf.append(STRING_WITH_LEN(",")); + buf.append(STRING_WITH_LEN(" INVISIBLE"),cs);
No space before "INVISIBLE". Instead, append STRING_WITH_LEN(", ")
Done.
+ } + table->field[17]->store(buf.ptr(), buf.length(), cs); table->field[19]->store(field->comment.str, field->comment.length, cs); if (schema_table_store_record(thd, table)) DBUG_RETURN(1); diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 4006e3aec4d..1c8398b0fd7 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -3233,7 +3233,37 @@ bool Column_definition::prepare_stage1_check_typelib_default() return false; }
- +/* + This function create a hidden field. + SYNOPSIS + mysql_create_hidden_field() + thd Thread Object + field_name + type_handler field data type + field_visibility + default value + RETURN VALUE + Create_field object +*/ +static +Create_field * mysql_create_hidden_field(THD *thd, const char *field_name, + Type_handler *type_handler, field_visible_type field_visibility, + Item* default_value) +{ + Create_field *fld= new(thd->mem_root)Create_field(); + fld->set_handler(type_handler); + fld->field_name.str= thd->strmake(field_name, strlen(field_name)); + fld->field_name.length= strlen(field_name); + fld->field_visibility= field_visibility; + if (default_value) + { + Virtual_column_info *v= new (thd->mem_root) Virtual_column_info(); + v->expr= default_value; + v->utf8= 0; + fld->default_value= v; + } + return fld; +}
This will be a compiler warning in optimized builds, because a static function is not used anywhere.
Removed static.
/* Preparation for table creation
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Thanks for the review. -- Regards sachin
Hi, Sachin! On Nov 22, Sachin Setiya wrote:
Hi Serg!
Thanks for the review. Please review the updated patch , at bb-hidden-test branch I am currently working on this , only 2 things are left. Changes apart from comments. Rebased to latest 10.3. Completely_invisible, or HA_INVISIBLE_INDEX is only deleted when same name column/ index is created(with tests).
Thanks! I've only looked at diffs from my last review this time. Few minor comments below:
diff --git a/include/my_base.h b/include/my_base.h index b93300d7562..e028922b220 100644 --- a/include/my_base.h +++ b/include/my_base.h @@ -277,7 +277,8 @@ enum ha_base_keytype { This flag can be calculated -- it's based on key lengths comparison. */ #define HA_KEY_HAS_PART_KEY_SEG 65536 - +/* Internal Flag Can be calcaluted */ +#define HA_INVISIBLE_KEY 2<<18 /* An invisible key */
I'm not a great fan of tautological comments like a++; // increment a ideally, the code is self-documenting and needs no comments. sometimes comments are needed, mostly to explain *why* something is done, not *what* is done. in this case HA_INVISIBLE_KEY is very clear on itself, no need to repeat it in a comment :)
/* Automatic bits in key-flag */
#define HA_SPACE_PACK_USED 4 /* Test for if SPACE_PACK used */ diff --git a/sql/field.h b/sql/field.h index 139b292c96a..ac522f2cf16 100644 --- a/sql/field.h +++ b/sql/field.h @@ -4477,5 +4480,6 @@ bool check_expression(Virtual_column_info *vcol, LEX_CSTRING *name, #define f_no_default(x) ((x) & FIELDFLAG_NO_DEFAULT) #define f_bit_as_char(x) ((x) & FIELDFLAG_TREAT_BIT_AS_CHAR) #define f_is_hex_escape(x) ((x) & FIELDFLAG_HEX_ESCAPE) +#define f_visibility(x) (static_cast<field_visible_type> (x & 3))
note, it must be (x) - always in parentheses think of f_visibility(flags | 5) you don't want to expand it to (static_cast<field_visible_type> (flags | 5 & 3))
#endif /* FIELD_INCLUDED */ diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 93dd6239749..f8114b429db 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -8153,13 +8166,19 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List<Item> &values, List<TABLE> tbl_list; bool all_fields_have_values= true; Item *value; - Field *field; + Field *field, **f; bool abort_on_warning_saved= thd->abort_on_warning; uint autoinc_index= table->next_number_field ? table->next_number_field->field_index : ~0U; + uint field_count= 0; + bool need_default_value= false; DBUG_ENTER("fill_record"); - + //TODO will fields count be alwats equal to table->fields ? + for (f= ptr; f && (field= *f); f++) + field_count++; + if (field_count != values.elements) + need_default_value= true;
TODO from the previous review
if (!*ptr) { /* No fields to update, quite strange!*/ @@ -8177,12 +8196,16 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List<Item> &values, only one row. */ table->auto_increment_field_not_null= FALSE; + Name_resolution_context *context= & thd->lex->select_lex.context; while ((field = *ptr++) && ! thd->is_error()) { /* Ensure that all fields are from the same table */ DBUG_ASSERT(field->table == table);
- value=v++; + if (need_default_value && field->field_visibility != NOT_INVISIBLE) + value = new (thd->mem_root) Item_default_value(thd,context);
TODO from the previous review
+ else + value=v++; if (field->field_index == autoinc_index) table->auto_increment_field_not_null= TRUE; if (field->vcol_info) diff --git a/sql/sql_class.h b/sql/sql_class.h index 92f28a4dc07..87fbb56a557 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -790,6 +793,7 @@ typedef struct system_status_var ulong feature_dynamic_columns; /* +1 when creating a dynamic column */ ulong feature_fulltext; /* +1 when MATCH is used */ ulong feature_gis; /* +1 opening a table with GIS features */ + ulong feature_invisible_columns; /* +1 opening a table with hidden column */
another "hidden" missed - in a comment :)
ulong feature_locale; /* +1 when LOCALE is set */ ulong feature_subquery; /* +1 when subqueries are used */ ulong feature_timezone; /* +1 when XPATH is used */ diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 4006e3aec4d..5cfdaddaa88 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -3232,8 +3236,57 @@ bool Column_definition::prepare_stage1_check_typelib_default() } return false; } - - +/* + This function adds a hidden field to field_list + SYNOPSIS + mysql_add_hidden_field() + thd Thread Object + field_list list of all table fields + field_name name/prefix of hidden field + ( Prefix in the case when it is + *COMPLETELY_INVISIBLE* + and given name is duplicate) + type_handler field data type + field_visibility + default value + RETURN VALUE + Create_field pointer +*/ +int mysql_add_invisible_field(THD *thd, List<Create_field> * field_list, + const char *field_name, Type_handler *type_handler, + field_visible_type field_visibility, Item* default_value) +{ + Create_field *fld= new(thd->mem_root)Create_field(); + const char *new_name= NULL; + /* Get unique field name if field_visibility == COMPLETELY_INVISIBLE */ + if (field_visibility == COMPLETELY_INVISIBLE) + { + if ((new_name= make_unique_invisible_field_name(thd, field_name, + field_list))) + { + fld->field_name.str= new_name; + fld->field_name.length= strlen(new_name); + } + else + return 1; //Should not never happen
are you a bit too negative here? :) "should not happen" or "should never happen"
+ } + else + { + fld->field_name.str= thd->strmake(field_name, strlen(field_name)); + fld->field_name.length= strlen(field_name); + } + fld->set_handler(type_handler); + fld->field_visibility= field_visibility; + if (default_value) + { + Virtual_column_info *v= new (thd->mem_root) Virtual_column_info(); + v->expr= default_value; + v->utf8= 0; + fld->default_value= v; + } + field_list->push_front(fld, thd->mem_root); + return 0; +} /* Preparation for table creation
@@ -5001,17 +5098,36 @@ bool mysql_create_table(THD *thd, TABLE_LIST *create_table,
/* ** Give the key name after the first field with an optional '_#' after + @returns + 0 if keyname does not exists + [1..) index + 1 of duplicate key name **/
static bool
if you want the function to return the key number, you need to change the return type
check_if_keyname_exists(const char *name, KEY *start, KEY *end) { - for (KEY *key=start ; key != end ; key++) + uint i= 1; + for (KEY *key=start; key != end ; key++, i++) if (!my_strcasecmp(system_charset_info, name, key->name.str)) - return 1; + return i; return 0; }
+/** + Returns 1 if field name exists other wise 0
"otherwise"
+*/ +static bool +check_if_field_name_exists(const char *name, List<Create_field> * fields) +{ + Create_field *fld; + List_iterator<Create_field>it(*fields); + while ((fld = it++)) + { + if (!my_strcasecmp(system_charset_info, fld->field_name.str, name)) + return 1; + } + return 0; +}
static char * make_unique_key_name(THD *thd, const char *field_name,KEY *start,KEY *end) @@ -5069,6 +5185,31 @@ static void make_unique_constraint_name(THD *thd, LEX_CSTRING *name, } }
+/** + COMPLETELY_INVISIBLE are internally created. They are completely invisible + to Alter command (Opposite of SYSTEM_INVISIBLE which through + error when same name column is added by Alter). So in the case of when + user added a same column name as of COMPLETELY_INVISIBLE , we change + COMPLETELY_INVISIBLE column name. +*/ +static const +char * make_unique_invisible_field_name(THD *thd, const char *field_name, + List<Create_field> *fields) +{ + if (!check_if_field_name_exists(field_name, fields)) + return field_name; + char buff[MAX_FIELD_NAME], *buff_end; + buff_end= strmov(buff, field_name);
You never check that buff is long enough. Use something like buff_end= strnmov(buff, field_name, MAX_FIELD_NAME); if (buff_end - buff < 5) return NULL; // Should not happen
+ + for (uint i=1 ; i < 1000; i++)
make it 10000, one can still get 4000-5000 fields in a table, but not more, so after 10000 you can safely say "should not happen"
+ { + char *real_end= int10_to_str(i, buff_end, 10); + if (check_if_field_name_exists(buff, fields)) + continue; + return (const char *)thd->strmake(buff, real_end - buff); + } + return NULL; //Should not happen +}
/**************************************************************************** ** Alter a table definition @@ -7900,6 +8045,30 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, my_error(ER_WRONG_NAME_FOR_INDEX, MYF(0), key->name.str); goto err; } + uint dup_index= 0; + if ((dup_index= check_if_keyname_exists(key->name.str, table->key_info, + table->key_info + table->s->keys))) + { + if (table->s->key_info[dup_index - 1].flags & HA_INVISIBLE_KEY) + { + /* Drop Index from new_key_list + Why only drop HA_INVISIBLE_INDEX because we want them to be renamed + automatically.
1. so, you don't drop invisible indexes if there is no name conflict? 2. where are they recreated? 3. the recreation code needs to check whether the index is present and recreate if not. why not to recreate always? and drop always? isn't that simpler?
+ */ + List_iterator<Key> it(new_key_list); + Key *tmp; + while((tmp= it++)) + { + if(!my_strcasecmp(system_charset_info, tmp->name.str, key->name.str)) + { + it.remove(); + if (table->s->tmp_table == NO_TMP_TABLE) + (void) delete_statistics_for_index(thd, table, + &table->key_info[dup_index - 1], FALSE); + } + } + } + } } }
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
Hi Serg! Thanks for the review. On Fri, Nov 24, 2017 at 3:06 AM, Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Sachin!
On Nov 22, Sachin Setiya wrote:
Hi Serg!
Thanks for the review. Please review the updated patch , at bb-hidden-test branch I am currently working on this , only 2 things are left. Changes apart from comments. Rebased to latest 10.3. Completely_invisible, or HA_INVISIBLE_INDEX is only deleted when same name column/ index is created(with tests).
Thanks! I've only looked at diffs from my last review this time. Few minor comments below:
diff --git a/include/my_base.h b/include/my_base.h index b93300d7562..e028922b220 100644 --- a/include/my_base.h +++ b/include/my_base.h @@ -277,7 +277,8 @@ enum ha_base_keytype { This flag can be calculated -- it's based on key lengths comparison. */ #define HA_KEY_HAS_PART_KEY_SEG 65536 - +/* Internal Flag Can be calcaluted */ +#define HA_INVISIBLE_KEY 2<<18 /* An invisible key */
I'm not a great fan of tautological comments like
a++; // increment a
ideally, the code is self-documenting and needs no comments. sometimes comments are needed, mostly to explain *why* something is done, not *what* is done.
in this case HA_INVISIBLE_KEY is very clear on itself, no need to repeat it in a comment :)
Sorry, Removed.
/* Automatic bits in key-flag */
#define HA_SPACE_PACK_USED 4 /* Test for if SPACE_PACK used */ diff --git a/sql/field.h b/sql/field.h index 139b292c96a..ac522f2cf16 100644 --- a/sql/field.h +++ b/sql/field.h @@ -4477,5 +4480,6 @@ bool check_expression(Virtual_column_info *vcol, LEX_CSTRING *name, #define f_no_default(x) ((x) & FIELDFLAG_NO_DEFAULT) #define f_bit_as_char(x) ((x) & FIELDFLAG_TREAT_BIT_AS_CHAR) #define f_is_hex_escape(x) ((x) & FIELDFLAG_HEX_ESCAPE) +#define f_visibility(x) (static_cast<field_visible_type> (x & 3))
note, it must be (x) - always in parentheses think of
f_visibility(flags | 5)
you don't want to expand it to
(static_cast<field_visible_type> (flags | 5 & 3))
Done.
#endif /* FIELD_INCLUDED */ diff --git a/sql/sql_base.cc b/sql/sql_base.cc index 93dd6239749..f8114b429db 100644 --- a/sql/sql_base.cc +++ b/sql/sql_base.cc @@ -8153,13 +8166,19 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List<Item> &values, List<TABLE> tbl_list; bool all_fields_have_values= true; Item *value; - Field *field; + Field *field, **f; bool abort_on_warning_saved= thd->abort_on_warning; uint autoinc_index= table->next_number_field ? table->next_number_field->field_index : ~0U; + uint field_count= 0; + bool need_default_value= false; DBUG_ENTER("fill_record"); - + //TODO will fields count be alwats equal to table->fields ? + for (f= ptr; f && (field= *f); f++) + field_count++; + if (field_count != values.elements) + need_default_value= true;
TODO from the previous review
Done.
if (!*ptr) { /* No fields to update, quite strange!*/ @@ -8177,12 +8196,16 @@ fill_record(THD *thd, TABLE *table, Field **ptr, List<Item> &values, only one row. */ table->auto_increment_field_not_null= FALSE; + Name_resolution_context *context= & thd->lex->select_lex.context; while ((field = *ptr++) && ! thd->is_error()) { /* Ensure that all fields are from the same table */ DBUG_ASSERT(field->table == table);
- value=v++; + if (need_default_value && field->field_visibility != NOT_INVISIBLE) + value = new (thd->mem_root) Item_default_value(thd,context);
TODO from the previous review
1. Hmm, you will create a new Item for every inserted row? What if it's INSERT ... SELECT * FROM very_large_table?
2. please test inserts with stored procedures and prepared statements.
Done.
+ else + value=v++; if (field->field_index == autoinc_index) table->auto_increment_field_not_null= TRUE; if (field->vcol_info) diff --git a/sql/sql_class.h b/sql/sql_class.h index 92f28a4dc07..87fbb56a557 100644 --- a/sql/sql_class.h +++ b/sql/sql_class.h @@ -790,6 +793,7 @@ typedef struct system_status_var ulong feature_dynamic_columns; /* +1 when creating a dynamic column */ ulong feature_fulltext; /* +1 when MATCH is used */ ulong feature_gis; /* +1 opening a table with GIS features */ + ulong feature_invisible_columns; /* +1 opening a table with hidden column */
another "hidden" missed - in a comment :)
Ohh sorry. Removed all instance of hidden in code.
ulong feature_locale; /* +1 when LOCALE is set */ ulong feature_subquery; /* +1 when subqueries are used */ ulong feature_timezone; /* +1 when XPATH is used */ diff --git a/sql/sql_table.cc b/sql/sql_table.cc index 4006e3aec4d..5cfdaddaa88 100644 --- a/sql/sql_table.cc +++ b/sql/sql_table.cc @@ -3232,8 +3236,57 @@ bool Column_definition::prepare_stage1_check_typelib_default() } return false; } - - +/* + This function adds a hidden field to field_list + SYNOPSIS + mysql_add_hidden_field() + thd Thread Object + field_list list of all table fields + field_name name/prefix of hidden field + ( Prefix in the case when it is + *COMPLETELY_INVISIBLE* + and given name is duplicate) + type_handler field data type + field_visibility + default value + RETURN VALUE + Create_field pointer +*/ +int mysql_add_invisible_field(THD *thd, List<Create_field> * field_list, + const char *field_name, Type_handler *type_handler, + field_visible_type field_visibility, Item* default_value) +{ + Create_field *fld= new(thd->mem_root)Create_field(); + const char *new_name= NULL; + /* Get unique field name if field_visibility == COMPLETELY_INVISIBLE */ + if (field_visibility == COMPLETELY_INVISIBLE) + { + if ((new_name= make_unique_invisible_field_name(thd, field_name, + field_list))) + { + fld->field_name.str= new_name; + fld->field_name.length= strlen(new_name); + } + else + return 1; //Should not never happen
are you a bit too negative here? :) "should not happen" or "should never happen"
Changed :)
+ } + else + { + fld->field_name.str= thd->strmake(field_name, strlen(field_name)); + fld->field_name.length= strlen(field_name); + } + fld->set_handler(type_handler); + fld->field_visibility= field_visibility; + if (default_value) + { + Virtual_column_info *v= new (thd->mem_root) Virtual_column_info(); + v->expr= default_value; + v->utf8= 0; + fld->default_value= v; + } + field_list->push_front(fld, thd->mem_root); + return 0; +} /* Preparation for table creation
@@ -5001,17 +5098,36 @@ bool mysql_create_table(THD *thd, TABLE_LIST *create_table,
/* ** Give the key name after the first field with an optional '_#' after + @returns + 0 if keyname does not exists + [1..) index + 1 of duplicate key name **/
static bool
if you want the function to return the key number, you need to change the return type
Sorry this was changed in next commit.
check_if_keyname_exists(const char *name, KEY *start, KEY *end) { - for (KEY *key=start ; key != end ; key++) + uint i= 1; + for (KEY *key=start; key != end ; key++, i++) if (!my_strcasecmp(system_charset_info, name, key->name.str)) - return 1; + return i; return 0; }
+/** + Returns 1 if field name exists other wise 0
"otherwise"
Done
+*/ +static bool +check_if_field_name_exists(const char *name, List<Create_field> * fields) +{ + Create_field *fld; + List_iterator<Create_field>it(*fields); + while ((fld = it++)) + { + if (!my_strcasecmp(system_charset_info, fld->field_name.str, name)) + return 1; + } + return 0; +}
static char * make_unique_key_name(THD *thd, const char *field_name,KEY *start,KEY *end) @@ -5069,6 +5185,31 @@ static void make_unique_constraint_name(THD *thd, LEX_CSTRING *name, } }
+/** + COMPLETELY_INVISIBLE are internally created. They are completely invisible + to Alter command (Opposite of SYSTEM_INVISIBLE which through + error when same name column is added by Alter). So in the case of when + user added a same column name as of COMPLETELY_INVISIBLE , we change + COMPLETELY_INVISIBLE column name. +*/ +static const +char * make_unique_invisible_field_name(THD *thd, const char *field_name, + List<Create_field> *fields) +{ + if (!check_if_field_name_exists(field_name, fields)) + return field_name; + char buff[MAX_FIELD_NAME], *buff_end; + buff_end= strmov(buff, field_name);
You never check that buff is long enough. Use something like
buff_end= strnmov(buff, field_name, MAX_FIELD_NAME); if (buff_end - buff < 5) return NULL; // Should not happen
Done.
+ + for (uint i=1 ; i < 1000; i++)
make it 10000, one can still get 4000-5000 fields in a table, but not more, so after 10000 you can safely say "should not happen"
Okay , I just thought 1000 will be more then enough. changed.
+ { + char *real_end= int10_to_str(i, buff_end, 10); + if (check_if_field_name_exists(buff, fields)) + continue; + return (const char *)thd->strmake(buff, real_end - buff); + } + return NULL; //Should not happen +}
/**************************************************************************** ** Alter a table definition @@ -7900,6 +8045,30 @@ mysql_prepare_alter_table(THD *thd, TABLE *table, my_error(ER_WRONG_NAME_FOR_INDEX, MYF(0), key->name.str); goto err; } + uint dup_index= 0; + if ((dup_index= check_if_keyname_exists(key->name.str, table->key_info, + table->key_info + table->s->keys))) + { + if (table->s->key_info[dup_index - 1].flags & HA_INVISIBLE_KEY) + { + /* Drop Index from new_key_list + Why only drop HA_INVISIBLE_INDEX because we want them to be renamed + automatically.
1. so, you don't drop invisible indexes if there is no name conflict? Right 2. where are they recreated? 3. the recreation code needs to check whether the index is present and recreate if not. why not to recreate always? and drop always? isn't that simpler? Changed this , Now they are deleted and recreated again.
+ */ + List_iterator<Key> it(new_key_list); + Key *tmp; + while((tmp= it++)) + { + if(!my_strcasecmp(system_charset_info, tmp->name.str, key->name.str)) + { + it.remove(); + if (table->s->tmp_table == NO_TMP_TABLE) + (void) delete_statistics_for_index(thd, table, + &table->key_info[dup_index - 1], FALSE); + } + } + } + } } }
Regards, Sergei Chief Architect MariaDB and security@mariadb.org
-- Regards Sachin Setiya Software Engineer at MariaDB
participants (2)
-
Sachin Setiya -
Sergei Golubchik