Re: 82a49812507: MDEV-22597 Add views for periods in information_schema
Hi, Nikita, On Aug 31, Nikita Malyavin wrote:
revision-id: 82a49812507 (mariadb-11.0.1-228-g82a49812507) parent(s): 23b15d24eb3 author: Nikita Malyavin committer: Nikita Malyavin timestamp: 2023-08-30 22:49:36 +0400 message:
MDEV-22597 Add views for periods in information_schema
Two new information_schema views are added: * PERIOD table -- columns TABLE_CATALOG, TABLE_SCHEMA, TABLE_NAME, PERIOD_NAME, START_COLUMN_NAME, END_COLUMN_NAME. * KEY_PERIOD_USAGE -- works similar to KEY_COLUMN_USAGE, but for periods. Columns CONSTRAINT_CATALOG, CONSTRAINT_SCHEMA, CONSTRAINT_NAME, TABLE_CATALOG, TABLE_SCHEMA, TABLE_NAME, PERIOD_NAME
Two new columns are added to the COLUMNS view: IS_SYSTEM_TIME_PERIOD_START, IS_SYSTEM_TIME_PERIOD_END - contain YES/NO.
diff --git a/mysql-test/main/information_schema-big.result b/mysql-test/main/information_schema-big.result index 94c8d274fad..ad0451293c9 100644 --- a/mysql-test/main/information_schema-big.result +++ b/mysql-test/main/information_schema-big.result
good, you didn't forget to update big and embedded tests
diff --git a/mysql-test/suite/period/r/i_s_notembedded.result b/mysql-test/suite/period/r/i_s_notembedded.result new file mode 100644 index 00000000000..879376cc8b4 --- /dev/null +++ b/mysql-test/suite/period/r/i_s_notembedded.result @@ -0,0 +1,52 @@ +select * from information_schema.periods; +TABLE_CATALOG TABLE_SCHEMA TABLE_NAME PERIOD START_COLUMN_NAME END_COLUMN_NAME +create or replace table t1 (id int primary key, s timestamp(6), e timestamp(6), +period for mytime(s,e)); +create or replace table t2 (id int primary key, s timestamp(6), e timestamp(6), +period for mytime(s,e)) with system versioning; +show columns from t1; +Field Type Null Key Default Extra +id int(11) NO PRI NULL +s timestamp(6) NO NULL +e timestamp(6) NO NULL +select * from information_schema.periods where table_schema = 'test'; +TABLE_CATALOG TABLE_SCHEMA TABLE_NAME PERIOD START_COLUMN_NAME END_COLUMN_NAME +def test t2 SYSTEM_TIME row_start row_end
I don't think so, they're INVISIBLE_SYSTEM. for all practical purposes they don't exist as columns in the table. Shouldn't be shown either. But don't remove this test, let's keep it to show that row_start/row_end are *not* shown here. And add table t3 with full standard definition of period for system_time. it should be shown all right.
+def test t2 mytime s e +def test t1 mytime s e +create user periods_hidden@localhost; ... diff --git a/sql/sql_show.cc b/sql/sql_show.cc index 1dc60a01f38..876d6c6f244 100644 --- a/sql/sql_show.cc +++ b/sql/sql_show.cc @@ -6165,11 +6166,83 @@ static void store_variable_type(THD *thd, const sp_variable *spvar, } }
+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.
+ + if (err) return err; + + int period_field= 4; + for (auto *field: {period.start_field(s), period.end_field(s)}) + { +#ifndef NO_EMBEDDED_ACCESS_CHECKS + bool col_granted= get_column_grant(thd, &tl->grant, + db_name->str, table_name->str, + field->field_name.str) & COL_ACLS; + if (col_granted) +#endif + { + schema_table->field[period_field]->set_notnull(); + err= schema_table->field[period_field]->store(field->field_name, cs); + } + + if (err) + return err; + period_field++; + } + + return schema_table_store_record(thd, schema_table); +} + + +static int +get_schema_period_records(THD *thd, TABLE_LIST *tl, + TABLE *schema_table, //!< @ref + //!< Show::periods_fields_info + bool res, + const LEX_CSTRING *db_name, + const LEX_CSTRING *table_name) +{ + TABLE *table= tl->table; + + if (!table || (!table->s->period.name && !table->versioned())) + return 0; + +#ifndef NO_EMBEDDED_ACCESS_CHECKS + check_access(thd, SELECT_ACL, db_name->str, + &tl->grant.privilege, 0, 0, MY_TEST(tl->schema_table)); + if (is_temporary_table(tl)) + { + tl->grant.privilege|= TMP_TABLE_ACLS; + } +#endif + int err= 0; + if (table->versioned()) + err= store_schema_period_record(thd, tl, schema_table, db_name, table_name, + table->s->vers); + if (!err && table->s->period.name) + err= store_schema_period_record(thd, tl, schema_table, db_name, table_name, + table->s->period); + + return err; +}
-static int get_schema_column_record(THD *thd, TABLE_LIST *tables, - TABLE *table, bool res, - const LEX_CSTRING *db_name, - const LEX_CSTRING *table_name) +static +int get_schema_column_record(THD *thd, TABLE_LIST *tables, + TABLE *table, //!< @ref Show::columns_fields_info + bool res, + const LEX_CSTRING *db_name, + const LEX_CSTRING *table_name) { LEX *lex= thd->lex; const char *wild= lex->wild ? lex->wild->ptr() : NullS; @@ -7513,30 +7592,41 @@ static int get_schema_triggers_record(THD *thd, TABLE_LIST *tables, DBUG_RETURN(0); }
+static int +store_key_column_usage(TABLE *table, const LEX_CSTRING &db_name, + const LEX_CSTRING &table_name, + const LEX_CSTRING &key_name, + const LEX_CSTRING &col_name) +{ + CHARSET_INFO *cs= system_charset_info; + static const LEX_CSTRING def{STRING_WITH_LEN("def")}; + const LEX_CSTRING *values[] { &def, &db_name, &key_name, + &def, &db_name, &table_name, &col_name }; + int err = 0; + for (uint i = 0; i < array_elements(values) && !err; i++) + table->field[i]->store(values[i], cs);
you forgot `err=` here :) but as I wrote above, it's not needed here anyway
+ return 0; +}
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); }
-static int get_schema_key_column_usage_record(THD *thd, - TABLE_LIST *tables, - TABLE *table, bool res, - const LEX_CSTRING *db_name, - const LEX_CSTRING *table_name) +static int +get_schema_key_column_usage_record(THD *thd, TABLE_LIST *tables, + TABLE *table, + //!< @ref Show::key_column_usage_fields_info + bool res, + const LEX_CSTRING *db_name, + const LEX_CSTRING *table_name) { DBUG_ENTER("get_schema_key_column_usage_record"); if (res) @@ -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?
+ CEnd() +}; +
ST_FIELD_INFO table_names_fields_info[]= { @@ -10208,6 +10358,8 @@ ST_SCHEMA_TABLE schema_tables[]= {0, 0, 0, 0, 0, 0, 0, 0, 0, 0} };
+static_assert(array_elements(schema_tables) == SCH_ENUM_SIZE + 1, + "Update enum_schema_tables as well.");
+1
int initialize_schema_table(st_plugin_int *plugin) {
Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
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
Hi, Nikita, On Sep 03, Nikita Malyavin wrote:
+ 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.
these columns are varchar, utf8, so the utf8 value is simply copied into the dedicated place inside the record[0] buffer. no conversion and no memory allocations.
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.
Ah, indeed, sorry. I didn't notice that.
@@ -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.
KEY_COLUMN_USAGE (where you supposedly copied from) shows foreign keys and has columns like REFERENCED_TABLE_SCHEMA, REFERENCED_TABLE_NAME, REFERENCED_COLUMN_NAME. This information is not stored in the frm yet. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
One more stack of questions: On Fri, 1 Sept 2023 at 01:09, Sergei Golubchik <serg@mariadb.org> wrote:
diff --git a/mysql-test/suite/period/r/i_s_notembedded.result b/mysql-test/suite/period/r/i_s_notembedded.result new file mode 100644 index 00000000000..879376cc8b4 --- /dev/null +++ b/mysql-test/suite/period/r/i_s_notembedded.result @@ -0,0 +1,52 @@ +select * from information_schema.periods; +TABLE_CATALOG TABLE_SCHEMA TABLE_NAME PERIOD START_COLUMN_NAME END_COLUMN_NAME +create or replace table t1 (id int primary key, s timestamp(6), e timestamp(6), +period for mytime(s,e)); +create or replace table t2 (id int primary key, s timestamp(6), e timestamp(6), +period for mytime(s,e)) with system versioning; +show columns from t1; +Field Type Null Key Default Extra +id int(11) NO PRI NULL +s timestamp(6) NO NULL +e timestamp(6) NO NULL +select * from information_schema.periods where table_schema = 'test'; +TABLE_CATALOG TABLE_SCHEMA TABLE_NAME PERIOD START_COLUMN_NAME END_COLUMN_NAME +def test t2 SYSTEM_TIME row_start row_end
I don't think so, they're INVISIBLE_SYSTEM. for all practical purposes they don't exist as columns in the table. Shouldn't be shown either. But don't remove this test, let's keep it to show that row_start/row_end are *not* shown here. And add table t3 with full standard definition of period for system_time. it should be shown all right.
The columns -- I've checked -- can be accessed in SELECT, so why not show it? What should be the value then? An empty string? Or make it NULLable? The period itself is also accessible in SELECT, so I suppose it should be shown. Calm wishes, Nikita
Hi, Nikita, On Sep 03, Nikita Malyavin wrote:
One more stack of questions:
+show columns from t1; +Field Type Null Key Default Extra +id int(11) NO PRI NULL +s timestamp(6) NO NULL +e timestamp(6) NO NULL +select * from information_schema.periods where table_schema = 'test'; +TABLE_CATALOG TABLE_SCHEMA TABLE_NAME PERIOD START_COLUMN_NAME END_COLUMN_NAME +def test t2 SYSTEM_TIME row_start row_end
I don't think so, they're INVISIBLE_SYSTEM. for all practical purposes they don't exist as columns in the table. Shouldn't be shown either. But don't remove this test, let's keep it to show that row_start/row_end are *not* shown here. And add table t3 with full standard definition of period for system_time. it should be shown all right.
The columns -- I've checked -- can be accessed in SELECT, so why not show it?
They can be accessed by SELECT, but they aren't _columns_. No other I_S table show them, SHOW CREATE doesn't show them. They're pseudo-columns, like, for example, ROWID in Oracle.
What should be the value then? An empty string? Or make it NULLable? The period itself is also accessible in SELECT, so I suppose it should be shown.
No. If SHOW CREATE TABLE doesn't show something - this something doesn't exist as an object in the table, so information_schema.periods should not show it either. Regards, Sergei VP of MariaDB Server Engineering and security@mariadb.org
On Mon, 4 Sept 2023 at 00:26, Sergei Golubchik <serg@mariadb.org> wrote:
Hi, Nikita,
One more stack of questions:
+show columns from t1; +Field Type Null Key Default Extra +id int(11) NO PRI NULL +s timestamp(6) NO NULL +e timestamp(6) NO NULL +select * from information_schema.periods where table_schema = 'test'; +TABLE_CATALOG TABLE_SCHEMA TABLE_NAME PERIOD START_COLUMN_NAME END_COLUMN_NAME +def test t2 SYSTEM_TIME row_start row_end
I don't think so, they're INVISIBLE_SYSTEM. for all practical purposes
On Sep 03, Nikita Malyavin wrote: they
don't exist as columns in the table. Shouldn't be shown either. But don't remove this test, let's keep it to show that row_start/row_end are *not* shown here. And add table t3 with full standard definition of period for system_time. it should be shown all right.
The columns -- I've checked -- can be accessed in SELECT, so why not show it?
They can be accessed by SELECT, but they aren't _columns_. No other I_S table show them, SHOW CREATE doesn't show them. They're pseudo-columns, like, for example, ROWID in Oracle.
What should be the value then? An empty string? Or make it NULLable? The period itself is also accessible in SELECT, so I suppose it should be shown.
No. If SHOW CREATE TABLE doesn't show something - this something doesn't exist as an object in the table, so information_schema.periods should not show it either.
Ok, and besides I've just found that INVISIBLE_SYSTEM is documented as
Can be queried explicitly in SELECT, otherwise invisible from anything
Another plus to your point. -- Yours truly, Nikita Malyavin
Hi, the new head is d557722f2d0ae Calm wishes, Nikita
participants (2)
-
Nikita Malyavin -
Sergei Golubchik