Hi Sergei!

On Fri, 1 Sept 2023 at 01:09, Sergei Golubchik <serg@mariadb.org> wrote:
> +static int store_schema_period_record(THD *thd, TABLE_LIST *tl,
> +                                      TABLE *schema_table,
> +                                      const LEX_CSTRING *db_name,
> +                                      const LEX_CSTRING *table_name,
> +                                      const TABLE_SHARE::period_info_t &period)
> +{
> +  TABLE_SHARE *s= tl->table->s;
> +  const CHARSET_INFO *cs= system_charset_info;
> +
> +  int err=      schema_table->field[0]->store(STRING_WITH_LEN("def"), cs);
> +  if(!err) err= schema_table->field[1]->store(db_name, cs);
> +  if(!err) err= schema_table->field[2]->store(table_name, cs);
> +  if(!err) err= schema_table->field[3]->store(period.name, cs);

well, ok. but as you've seen elsewhere these go without checks, as these
calls generally cannot fail, they don't even allocate any memory.

I thought it can allocate if the string is longer than some (small) constant. Now I checked by the code that this constant is 65535:) And max name length is shorter. OK, but what about conversions? I suppose we store names in utf-8, and output it in some tricky encoding, can't we? There is MY_CS_ILUNI check in my_convert_fix.

>  static void
>  store_key_column_usage(TABLE *table, const LEX_CSTRING *db_name,
>                         const LEX_CSTRING *table_name, const char *key_name,
> -                       size_t key_len, const char *con_type, size_t con_len,
> +                       size_t key_len, const char *col_name, size_t col_len,
>                         longlong idx)
>  {
> -  CHARSET_INFO *cs= system_charset_info;
> -  table->field[0]->store(STRING_WITH_LEN("def"), cs);
> -  table->field[1]->store(db_name->str, db_name->length, cs);
> -  table->field[2]->store(key_name, key_len, cs);
> -  table->field[3]->store(STRING_WITH_LEN("def"), cs);
> -  table->field[4]->store(db_name->str, db_name->length, cs);
> -  table->field[5]->store(table_name->str, table_name->length, cs);
> -  table->field[6]->store(con_type, con_len, cs);
> +  store_key_column_usage(table, *db_name, *table_name, {key_name, key_len},
> +                         {col_name, col_len});

pretty, but better, please, put it back. You can use the old
store_key_column_usage from your get_schema_key_period_usage_record()
just the same. There's no need to have two store_key_column_usage()
functions.

>    table->field[7]->store((longlong) idx, TRUE);
>  }
no I can't, since I have no idx in spec. 

> @@ -9703,6 +9824,18 @@ ST_FIELD_INFO key_column_usage_fields_info[]=
>    CEnd()
>  };

> +ST_FIELD_INFO key_period_usage_fields_info[]=
> +{
> +  Column("CONSTRAINT_CATALOG", Catalog(), NOT_NULL, OPEN_FULL_TABLE),
> +  Column("CONSTRAINT_SCHEMA",  Name(),    NOT_NULL, OPEN_FULL_TABLE),
> +  Column("CONSTRAINT_NAME",    Name(),    NOT_NULL, OPEN_FULL_TABLE),
> +  Column("TABLE_CATALOG",      Catalog(), NOT_NULL, OPEN_FULL_TABLE),
> +  Column("TABLE_SCHEMA",       Name(),    NOT_NULL, OPEN_FULL_TABLE),
> +  Column("TABLE_NAME",         Name(),    NOT_NULL, OPEN_FULL_TABLE),
> +  Column("PERIOD_NAME",        Name(),    NOT_NULL, OPEN_FULL_TABLE),

Cannot they all be OPEN_FRM_ONLY?
I suppose they can... Didn't think that columns can require an access to the table data (like handler, I suppose), when I was copying, so didn't check what the options are there.

--
Yours truly,
Nikita Malyavin