maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Nikita!

On Jan 31, Nikita Malyavin wrote:
> revision-id: 4b01d3aee60 (versioning-1.0.7-2-g4b01d3aee60)
> parent(s): 44144533e50
> author: Nikita Malyavin <nikitamalyavin@xxxxxxxxx>
> committer: Nikita Malyavin <nikitamalyavin@xxxxxxxxx>
> timestamp: 2019-01-30 22:53:43 +1000
> message:
> 
> MDEV-17082 Application-time periods: CREATE
> 
> * add syntax `CREATE TABLE ... PERIOD FOR <apptime>`
> * add table period entity
> 
> diff --git a/mysql-test/suite/period/r/create.result b/mysql-test/suite/period/r/create.result
> --- /dev/null
> +++ b/mysql-test/suite/period/r/create.result
> @@ -0,0 +1,82 @@
> +create or replace table t (id int primary key, s date, e date,
> +period for mytime(s,e));
> +show create table t;
> +Table	Create Table
> +t	CREATE TABLE `t` (
> +  `id` int(11) NOT NULL,
> +  `s` date NOT NULL,
> +  `e` date NOT NULL,
> +  PRIMARY KEY (`id`),
> +  PERIOD FOR `mytime` (`s`, `e`),
> +  CONSTRAINT `CONSTRAINT_1` CHECK (`s` < `e`)

as discussed, let's give the constraint the same name as the period.
but don't forget to add a test where a user explicitly creates a
constraint with this name. Like

  create or replace table t (id int primary key, s date, e date,
  period for mytime(s,e), constraint mytime (id < 100));

> +) ENGINE=MyISAM DEFAULT CHARSET=latin1
> +create or replace table t (id int primary key, s timestamp(6), e timestamp(6),
> +period for mytime(s,e));
> +show create table t;
> +Table	Create Table
> +t	CREATE TABLE `t` (
> +  `id` int(11) NOT NULL,
> +  `s` timestamp(6) NOT NULL DEFAULT '0000-00-00 00:00:00.000000',
> +  `e` timestamp(6) NOT NULL DEFAULT '0000-00-00 00:00:00.000000',
> +  PRIMARY KEY (`id`),
> +  PERIOD FOR `mytime` (`s`, `e`),
> +  CONSTRAINT `CONSTRAINT_1` CHECK (`s` < `e`)
> +) ENGINE=MyISAM DEFAULT CHARSET=latin1
> +# SQL16, Part 2, 11.3  <table definition>, Syntax Rules, 2)a)
> +# 2) If a <table period definition> TPD is specified, then:
> +# a) <table scope> shall not be specified.
> +create or replace temporary table t (s date, e date, period for mytime(s,e));
> +ERROR HY000: Temporary tables with application-time period not allowed

_are_ not allowed.
or, better,
Application-time period table cannot be temporary

> +# SQL16, Part 2, 11.3  <table definition>, Syntax Rules, 2)e)iii)
> +# The <data type or domain name> contained in CD1 is either DATE or a
> +# timestamp type and it is equivalent to the <data type or domain name>
> +# contained in CD2.
> +create or replace table t (id int primary key, s datetime, e date,
> +period for mytime(s,e));
> +ERROR HY000: PERIOD FOR `mytime` fields types mismatch

_field_ type mismatch
or
Fields `s` and `t` of PERIOD FOR `mytime` have different types.
or
Fields of PERIOD FOR `mytime` have different types.

> +create or replace table t (s timestamp(2), e timestamp(6),
> +period for mytime(s,e));
> +ERROR HY000: PERIOD FOR `mytime` fields types mismatch
> +create or replace table t (id int primary key, s int, e date,
> +period for mytime(s,e));
> +ERROR 42000: Incorrect column specifier for column 's'

incorrect column _type_?

> +create or replace table t (id int primary key, s date, e date,
> +period for mytime(s,x));
> +ERROR 42S22: Unknown column 'x' in 'mytime'
> +create or replace table t (id int primary key, s date, e date,
> +period for mytime(s,e),
> +period for mytime2(s,e));
> +ERROR HY000: Cannot specify more than one application-time period
> +# SQL16, Part 2, 11.3  <table definition>, Syntax Rules, 2)d)
> +# No <column name> in any <column definition> shall be equivalent to PN.
> +create or replace table t (mytime int, s date, e date,
> +period for mytime(s,e));
> +ERROR HY000: Could not specify name `mytime` for field. It is already used by period.

Better "Column and period `mytime` have the same name".

Because "already used" implies that period got the name first, while in
the table definition the field was specified first.

> +# SQL16, Part 2, 11.3  <table definition>, Syntax Rules, 2)e)v)2)A)
> +# Neither CD1 nor CD2 shall contain an <identity column specification>, a
> +# <generation clause>, a <system time period start column specification>,
> +#  or a <system time period end column specification>.
> +create or replace table t (id int primary key,
> +s date,
> +e date generated always as (s+1),
> +period for mytime(s,e));
> +ERROR HY000: Period field `e` cannot be GENERATED ALWAYS AS
> +create or replace table t (id int primary key,
> +s date,
> +e date as (s+1) VIRTUAL,
> +period for mytime(s,e));
> +ERROR HY000: Period field `e` cannot be GENERATED ALWAYS AS
> +create or replace table t (id int primary key, s timestamp(6), e timestamp(6),
> +st timestamp(6) as row start,
> +en timestamp(6) as row end,
> +period for system_time (st, en),
> +period for mytime(st,e)) with system versioning;
> +ERROR HY000: Period field `st` cannot be GENERATED ALWAYS AS
> +# SQL16, Part 2, 11.3  <table definition>, Syntax Rules, 2)
> +# Let IDCN be an implementation-dependent <constraint name> that is not
> +# equivalent to the <constraint name> of any table constraint descriptor
> +# included in S.
> +create or replace table t (x int, s date, e date,
> +period for mytime(s, e),
> +constraint mytime check (x > 1));

please, add SHOW CREATE TABLE here (add it after every successful CREATE TABLE).

> +create or replace database test;
> diff --git a/sql/datadict.cc b/sql/datadict.cc
> --- a/sql/datadict.cc
> +++ b/sql/datadict.cc
> @@ -292,6 +292,12 @@ bool dd_read_extra2(const uchar *frm_image, size_t len, extra2_fields *fields)

We've discussed that TRUNCATE TABLE bug fix, it won't need dd_read_extra2().
You still want to have a separate function for reading extra2, fine, but
as dd_frm_type() won't needed it, I'd rather move it into table.cc
and declared it static.

In fact, if you'd like to keep extra2 code separate, you could move
everything extra2 related to a separate object (like your extra2_fields)
with methods for reading and writing it. But, please, don't do this
refactoring before March.

>            fields->field_flags.str= extra2;
>            fields->field_flags.length= length;
>            break;
> +        case EXTRA2_APPLICATION_TIME_PERIOD:
> +          if (fields->application_period.str)
> +            DBUG_RETURN(true);
> +          fields->application_period.str= extra2;
> +          fields->application_period.length= length;
> +          break;
>          default:
>            /* abort frm parsing if it's an unknown but important extra2 value */
>            if (type >= EXTRA2_ENGINE_IMPORTANT)
> diff --git a/sql/handler.cc b/sql/handler.cc
> --- a/sql/handler.cc
> +++ b/sql/handler.cc
> @@ -7352,3 +7352,99 @@ bool Vers_parse_info::check_sys_fields(const Lex_table_name &table_name,
>
> +bool Table_scope_and_contents_source_st::check_fields(
> +  THD *thd, Alter_info *alter_info, TABLE_LIST &create_table)
> +{
> +  bool res= vers_check_system_fields(thd, alter_info, create_table);
> +  if (res)
> +    return true;
> +
> +  if (!period_info.name)
> +    return false;
> +
> +  if (tmp_table())
> +  {
> +    my_error(ER_PERIOD_TEMPORARY_NOT_ALLOWED, MYF(0));
> +    return true;
> +  }
> +
> +  Table_period_info::start_end_t &period= period_info.period;
> +  const Create_field *row_start= NULL;
> +  const Create_field *row_end= NULL;
> +  List_iterator<Create_field> it(alter_info->create_list);
> +  while (const Create_field *f= it++)
> +  {
> +    if (period.start.streq(f->field_name)) row_start= f;
> +    else if (period.end.streq(f->field_name)) row_end= f;
> +
> +    if (period_info.name.streq(f->field_name))
> +    {
> +      // TODO this stub should be removed by MDEV-16976

why removed?

> +      my_error(ER_PERIOD_NAME_IS_NOT_ALLOWED_FOR_FIELD, MYF(0), f->field_name.str);
> +      return true;
> +    }
> +  }
>
> diff --git a/sql/table.h b/sql/table.h
> --- a/sql/table.h
> +++ b/sql/table.h
> @@ -1730,6 +1747,9 @@ class IS_table_read_plan;
>  /** The threshold size a blob field buffer before it is freed */
>  #define MAX_TDC_BLOB_SIZE 65536
>  
> +/** number of bytes read by uint2korr and sint2korr */
> +#define korr2size 2

No, that's silly. It's like #define TWO 2
Of course korr2size is 2. And korr3size is 3, and korr4size is 4.
If you want to get rid of the magic number, don't put it into the name
of the macro :)

That's why I suggested something like

  #define fieldno_size   2
  #define fieldno_korr   uint2korr
  #define fieldno_store  int2store

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx