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 [email protected]

_______________________________________________
Mailing list: https://launchpad.net/~maria-developers
Post to     : [email protected]
Unsubscribe : https://launchpad.net/~maria-developers
More help   : https://help.launchpad.net/ListHelp

Reply via email to