Hi, Jan!
I have various comments, see below. Nothing big, nothing that's really
the approach you've taken. In fact, I like where it's going.
But this is a huge and intrusive patch. I wonder, should we rather do it
in 10.2? See below, you yourself write:
On Feb 25, Jan Lindström wrote:
>
> Caution: Please take backups of your database before migrating 10.1.12.
This is not what users normally do when upgrading from a GA version to
the next minor GA version.
Anyway, see comments about the patch below:
> diff --git a/mysql-test/suite/encryption/r/innodb-bad-key-change.result b/mysql-test/suite/encryption/r/innodb-bad-key-change.result
> index cf97918..39e08f4 100644
> --- a/mysql-test/suite/encryption/r/innodb-bad-key-change.result
> +++ b/mysql-test/suite/encryption/r/innodb-bad-key-change.result
> @@ -36,8 +36,7 @@ SELECT * FROM t1;
> ERROR HY000: Got error 192 'Table encrypted but decryption failed. This could be because correct encryption management plugin is not loaded, used encryption key is not available or encryption method does not match.' from InnoDB
> SHOW WARNINGS;
> Level Code Message
> -Warning 1812 Tablespace is missing for table 'test/t1'
> -Warning 192 Table test/t1 is encrypted but encryption service or used key_id 2 is not available. Can't continue reading table.
> +Warning 192 Table test/t1 is encrypted but encryption service or used key_id is not available. Can't continue reading table.
why did that happen?
> Error 1296 Got error 192 'Table encrypted but decryption failed. This could be because correct encryption management plugin is not loaded, used encryption key is not available or encryption method does not match.' from InnoDB
> DROP TABLE t1;
> # Start server with keys.txt
> diff --git a/storage/innobase/include/dict0tableoptions.h b/storage/innobase/include/dict0tableoptions.h
> new file mode 100644
> index 0000000..def0e39
> --- /dev/null
> +++ b/storage/innobase/include/dict0tableoptions.h
> @@ -0,0 +1,127 @@
> +/*****************************************************************************
> +
> +Copyright (c) 2016, MariaDB Corporation.
> +
> +This program is free software; you can redistribute it and/or modify it under
> +the terms of the GNU General Public License as published by the Free Software
> +Foundation; version 2 of the License.
> +
> +This program is distributed in the hope that it will be useful, but WITHOUT
> +ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
> +FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License along with
> +this program; if not, write to the Free Software Foundation, Inc.,
> +51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA
> +
> +*****************************************************************************/
> +
> +/**************************************************//**
> +@file include/dict0tableoptions.h
> +Definitions for the system table SYS_TABLE_OPTIONS
> +
> +Created 22/01/2006 Jan Lindström
> +*******************************************************/
> +
> +#ifndef dict0tableoptions_h
> +#define dict0tableoptions_h
> +
> +#include "univ.i"
> +
> +/** Data structure to hold contents of SYS_TABLE_OPTIONS */
This is a move in the interesting direction. One of the ideas I was toying with
for a few years is to remove .frm files for InnoDB tables.
In MariaDB a storage engine can live without frm files if the engine supports
table discovery. This is rather easy to implement, but InnoDB does not store
the complete table definition, so the discovery will be lossy.
Now with your SYS_TABLE_OPTIONS table we're getting closer to having the complete
table definition saved inside InnoDB. And one step closer to frm-less InnoDB tables.
> +struct dict_tableoptions_t{
> + table_id_t table_id;
why do you need table_id in the option structure?
> + /* true if table page compressed */
> + bool page_compressed;
> + /* Used compression level if set */
> + ulonglong page_compression_level;
> + /* dict0types.h: ATOMIC_WRITES_DEFAULT, _ON, or _OFF */
> + atomic_writes_t atomic_writes;
> + /* fil0crypt.h: FIL_SPACE_ENCRYPTION_DEFAULT, _ON, or _OFF */
> + fil_encryption_t encryption;
> + /* Used encryption key identifier if set */
> + ulonglong encryption_key_id;
> + /* true if table options need to be stored */
why wouldn't they need to be?
> + bool need_stored;
> +};
> +
> +/********************************************************************//**
> +This function parses a SYS_TABLE_OPTIONS record, extracts necessary
> +information from the record and returns it to the caller.
> +@return error message or NULL if successfull */
> +UNIV_INTERN
> +const char*
> +dict_process_sys_tableoptions(
> +/*==========================*/
> + mem_heap_t* heap, /*!< in/out: heap memory */
> + const rec_t* rec, /*!< in: current SYS_TABLE_OPTIONS rec */
> + dict_tableoptions_t* table_options); /*!< out: table options */
> +
> +/********************************************************************//**
> +Gets the table options from SYS_TABLE_OPTIONS based on table_id
> +@return true if found, false if not found */
> +UNIV_INTERN
> +bool
> +dict_get_table_options(
> +/*===================*/
> + table_id_t table_id, /*!< in: table id */
> + dict_tableoptions_t* options, /*!< out:table options */
> + bool fixed); /*!< in: can we fix the
> + dictionary ? */
> +
> +/********************************************************************//**
> +Gets the table options from SYS_TABLE_OPTIONS
> +@return true if found, false if not found */
> +UNIV_INTERN
> +bool
> +dict_get_table_options(
> +/*===================*/
> + dict_table_t* table, /*!< in/out: table */
> + bool fixed); /*!< in: can we fix the
> + dictionary ? */
> +
> +/********************************************************************//**
> +Update the record in SYS_TABLE_OPTIONS.
> +@return DB_SUCCESS if OK, dberr_t if the update failed */
> +UNIV_INTERN
> +dberr_t
> +dict_update_tableoptions(
> +/*=====================*/
> + const dict_table_t* table); /*!< in: table object */
> +
> +/********************************************************************//**
> +Insert record into SYS_TABLE_OPTIONS
> +@return DB_SUCCESS if OK, dberr_t if the insert failed */
> +UNIV_INTERN
> +dberr_t
> +dict_insert_tableoptions(
> +/*=====================*/
> + const dict_table_t* table, /*!< in: table object */
> + bool fixed); /*!< in: can we fix the
> + dictionary ? */
> +
> +/********************************************************************//**
> +Update the table flags in SYS_TABLES.
> +@return DB_SUCCESS if OK, dberr_t if the update failed */
> +UNIV_INTERN
> +dberr_t
> +dict_update_table_flags(
> +/*=====================*/
> + const dict_table_t* table, /*!< in: table object */
> + bool fixed); /*!< in: can we fix the
> + dictionary ? */
> +
> +/********************************************************************//**
> +Delete the record in SYS_TABLE_OPTIONS.
> +@return DB_SUCCESS if OK, dberr_t if the update failed */
> +UNIV_INTERN
> +dberr_t
> +dict_delete_tableoptions(
> +/*=====================*/
> + const dict_table_t* table, /*!< in: table object */
> + trx_t* trx, /*!< in: trx */
> + bool fixed); /*!< in: can we fix the
> + dictionary ? */
> +
> +#endif /* dict0tableoptions_h */
> +
How does it work future-wise? How do you add new options to the table, say, in 10.2?> diff --git a/storage/innobase/dict/dict0tableoptions.cc b/storage/innobase/dict/dict0tableoptions.cc
> new file mode 100644
> index 0000000..527c5e3
> --- /dev/null
> +++ b/storage/innobase/dict/dict0tableoptions.cc
> @@ -0,0 +1,482 @@
> +/*****************************************************************************
> +
> +Copyright (c) 2016, MariaDB Corporation.
> +
> +This program is free software; you can redistribute it and/or modify it under
> +the terms of the GNU General Public License as published by the Free Software
> +Foundation; version 2 of the License.
> +
> +This program is distributed in the hope that it will be useful, but WITHOUT
> +ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS
> +FOR A PARTICULAR PURPOSE. See the GNU General Public License for more details.
> +
> +You should have received a copy of the GNU General Public License along with
> +this program; if not, write to the Free Software Foundation, Inc.,
> +51 Franklin Street, Suite 500, Boston, MA 02110-1335 USA
> +
> +*****************************************************************************/
> +
> +/**************************************************//**
> +@file dict/dict0tableoptions.cc
> +Function implementations for the system table SYS_TABLE_OPTIONS
> +
> +Created 22/01/2006 Jan Lindström
> +*******************************************************/
> +
> +#include "mysql_version.h"
> +#include "btr0pcur.h"
> +#include "btr0btr.h"
> +#include "page0page.h"
> +#include "mach0data.h"
> +#include "dict0dict.h"
> +#include "dict0boot.h"
> +#include "dict0stats.h"
> +#include "dict0mem.h"
> +#include "rem0cmp.h"
> +#include "srv0start.h"
> +#include "srv0srv.h"
> +#include "dict0crea.h"
> +#include "dict0priv.h"
> +#include "ha_prototypes.h" /* innobase_casedn_str() */
> +#include "fts0priv.h"
> +#include "dict0tableoptions.h"
> +#include "dict0load.h"
> +#include "row0mysql.h"
> +
> +/********************************************************************//**
> +This function parses a SYS_TABLE_OPTIONS record, extracts necessary
> +information from the record and returns it to the caller.
> +@return error message or NULL if successfull */
> +UNIV_INTERN
> +const char*
> +dict_process_sys_tableoptions(
> +/*==========================*/
> + mem_heap_t* heap, /*!< in/out: heap memory */
> + const rec_t* rec, /*!< in: current SYS_TABLE_OPTIONS rec */
> + dict_tableoptions_t* table_options) /*!< out: table options */
> +{
> + const byte* field;
> + ulint len=0;
> +
> + if (rec_get_deleted_flag(rec, 0)) {
> + return("delete-marked record in SYS_TABLE_OPTIONS");
> + }
> +
> + if (rec_get_n_fields_old(rec) != DICT_NUM_FIELDS__SYS_TABLEOPTIONS) {
> + return("wrong number of columns in SYS_TABLE_OPTIONS record");
> + }
> diff --git a/storage/innobase/api/api0api.cc b/storage/innobase/api/api0api.cc
> index 739ea9f..4af32a5 100644
> --- a/storage/innobase/api/api0api.cc
> +++ b/storage/innobase/api/api0api.cc
> @@ -270,7 +271,7 @@ ib_open_table_by_name(
> dict_table_t* table;
>
> table = dict_table_open_on_name(name, FALSE, FALSE,
> - DICT_ERR_IGNORE_NONE);
> + DICT_ERR_IGNORE_NONE, NULL);
What does that mean that you pass NULL as the last argument?
You won't know whether the table is encrypted or compressed - so you,
basically won't be able to read the table?
>
> if (table != NULL && table->ibd_file_missing) {
> table = NULL;
> diff --git a/storage/innobase/buf/buf0buf.cc b/storage/innobase/buf/buf0buf.cc
> index f4e7c0d..f1953af 100644
> --- a/storage/innobase/buf/buf0buf.cc
> +++ b/storage/innobase/buf/buf0buf.cc
> @@ -4677,7 +4678,7 @@ buf_page_io_complete(
> "However key management plugin or used key_id %lu is not found or"
> " used encryption algorithm or method does not match."
> " Can't continue opening the table.",
> - bpage->key_version);
> + bpage->space, bpage->key_version);
This seems to be an unrelated bugfix. And it seems to be already fixed
in the latest 10.1
>
> if (bpage->space > TRX_SYS_SPACE) {
> if (corrupted) {
> diff --git a/storage/innobase/dict/dict0dict.cc b/storage/innobase/dict/dict0dict.cc
> index 2b72835..13f1ee6 100644
> --- a/storage/innobase/dict/dict0dict.cc
> +++ b/storage/innobase/dict/dict0dict.cc
> @@ -1187,6 +1189,16 @@ dict_table_open_on_name(
>
> /* If table is encrypted return table */
> if (ignore_err == DICT_ERR_IGNORE_NONE
> + && !table->is_encrypted && options) {
hmm, where else table->is_encrypted can be set?
> + if ((options->encryption == FIL_SPACE_ENCRYPTION_ON ||
> + (options->encryption == FIL_SPACE_ENCRYPTION_DEFAULT && srv_encrypt_tables))
> + && !encryption_key_id_exists((unsigned int)options->encryption_key_id)) {
> + table->is_encrypted = true;
> + }
> + }
> +
> + /* If table is encrypted return table */
> + if (ignore_err == DICT_ERR_IGNORE_NONE
> && table->is_encrypted) {
> /* Make life easy for drop table. */
> if (table->can_be_evicted) {
> diff --git a/storage/innobase/dict/dict0load.cc b/storage/innobase/dict/dict0load.cc
> index d8bd0a6..2b732f1 100644
> --- a/storage/innobase/dict/dict0load.cc
> +++ b/storage/innobase/dict/dict0load.cc
> @@ -56,7 +64,8 @@ static const char* SYSTEM_TABLE_NAME[] = {
> "SYS_FOREIGN",
> "SYS_FOREIGN_COLS",
> "SYS_TABLESPACES",
> - "SYS_DATAFILES"
> + "SYS_DATAFILES",
> + "SYS_TABLE_OPTIONS"
May be "SYS_MARIADB_OPTIONS" ? Not really InnoDB sys table style,
I agree, but guarantees no conflicts in the future.
> };
>
> /* If this flag is TRUE, then we will load the cluster index's (and tables')
> @@ -2365,6 +2386,11 @@ dict_load_table(
> btr_pcur_close(&pcur);
> mtr_commit(&mtr);
>
> + if (table && options) {
no need to do if (table), because table is guaranteed to be not NULL here
> + ut_ad(table->table_options);
> + memcpy(table->table_options, options, sizeof(dict_tableoptions_t));
> + }
> +
> if (table->space == 0) {
> /* The system tablespace is always available. */
> } else if (table->flags2 & DICT_TF2_DISCARDED) {
> diff --git a/storage/innobase/dict/dict0mem.cc b/storage/innobase/dict/dict0mem.cc
> index 1724ac0..39d0d55 100644
> --- a/storage/innobase/dict/dict0mem.cc
> +++ b/storage/innobase/dict/dict0mem.cc
> @@ -85,7 +86,6 @@ dict_mem_table_create(
> mem_heap_t* heap;
>
> ut_ad(name);
> - ut_a(dict_tf_is_valid(flags));
why?
> ut_a(!(flags2 & ~DICT_TF2_BIT_MASK));
>
> heap = mem_heap_create(DICT_HEAP_SIZE);
> diff --git a/storage/innobase/fil/fil0fil.cc b/storage/innobase/fil/fil0fil.cc
> index 928c72c..4f75465 100644
> --- a/storage/innobase/fil/fil0fil.cc
> +++ b/storage/innobase/fil/fil0fil.cc
> @@ -716,27 +717,6 @@ fil_node_open_file(
> ut_error;
> }
>
> - if (UNIV_UNLIKELY(space->flags != flags)) {
> - fprintf(stderr,
> - "InnoDB: Error: table flags are 0x%lx"
> - " in the data dictionary\n"
> - "InnoDB: but the flags in file %s are 0x%lx!\n",
> - space->flags, node->name, flags);
do you still the check somewhere
where SYS_TABLE_OPTIONS is compared with frm?
> -
> - ut_error;
> - }
> -
> - if (UNIV_UNLIKELY(space->flags != flags)) {
> - if (!dict_tf_verify_flags(space->flags, flags)) {
> - fprintf(stderr,
> - "InnoDB: Error: table flags are 0x%lx"
> - " in the data dictionary\n"
> - "InnoDB: but the flags in file %s are 0x%lx!\n",
> - space->flags, node->name, flags);
> - ut_error;
> - }
> - }
> -
> if (size_bytes >= FSP_EXTENT_SIZE * UNIV_PAGE_SIZE) {
> /* Truncate the size to whole extent size. */
> size_bytes = ut_2pow_round(size_bytes,
> @@ -1917,11 +1909,6 @@ fil_check_first_page(
> flags = mach_read_from_4(FSP_HEADER_OFFSET + FSP_SPACE_FLAGS + page);
>
> if (UNIV_PAGE_SIZE != fsp_flags_get_page_size(flags)) {
> - fprintf(stderr,
> - "InnoDB: Error: Current page size %lu != "
> - " page size on page %lu\n",
> - UNIV_PAGE_SIZE, fsp_flags_get_page_size(flags));
> -
why?
> return("innodb-page-size mismatch");
> }
>
> @@ -3379,8 +3362,7 @@ fil_create_new_single_table_tablespace(
> ulint size, /*!< in: the initial size of the
> tablespace file in pages,
> must be >= FIL_IBD_FILE_INITIAL_SIZE */
> - fil_encryption_t mode, /*!< in: encryption mode */
> - ulint key_id) /*!< in: encryption key_id */
> + const dict_table_t* table) /*!< in: table or NULL */
what does NULL mean here?
> {
> os_file_t file;
> ibool ret;
a bit more verbose comment would've been be nice.> @@ -3640,6 +3627,140 @@ fil_report_bad_tablespace(
> (ulong) expected_id, (ulong) expected_flags);
> }
>
> +/******************************************************************
> +Set flags for a tablespace */
> +static
> +void
> +fil_space_set_fsp_flags(
> +/*=====================*/
> + ulint id, /*!< in: space id */
> + uint flags) /*!< in: fsp flags */
> +{
> + fil_space_t* space;
> +
> + ut_ad(fil_system);
> +
> + mutex_enter(&fil_system->mutex);
> +
> + space = fil_space_get_by_id(id);
> +
> + if (space != NULL) {
> + space->flags = flags;
> + }
> +
> + mutex_exit(&fil_system->mutex);
> +}
> +
> +/******************************************************************
> +Update tablespace (fsp) flags on page 0
like, say that it's used to migrate from mariadb-flags-in-page-0
to sys_table_options approach.
> +@return true if successfull, false if not */
> +static
> +void
> +fil_update_page0(
> +/*=============*/
> + byte* page0, /*!< in: page 0 or NULL */
why do you need this page0 argument if it is *always* NULL?
> diff --git a/storage/innobase/fts/fts0fts.cc b/storage/innobase/fts/fts0fts.cc
> + os_file_t data_file, /*!< in: data file */
> + ulint space, /*!< in: space id */
> + ulint flags, /*!< in: old fsp flags */
> + ulint fsp_flags)/*!< in: new fsp flags */
> +{
> + ulint psize = FSP_FLAGS_GET_PAGE_SSIZE_MARIADB(flags);
> + ulint zip_size = FSP_FLAGS_GET_ZIP_SSIZE(flags);
> +
> + if (!psize) {
> + psize = UNIV_PAGE_SIZE_ORIG;
> + }
> +
> + fsp_flags = fsp_flags_set_page_size(fsp_flags, psize);
> +
> + if (flags != fsp_flags) {
> +
> + ib_logf(IB_LOG_LEVEL_INFO,
> + "InnoDB: Adjusted space_id %lu tablespace flags from %lu to %lu.\n",
> + space, flags, fsp_flags);
> +
> + mtr_t mtr;
> + mtr_start(&mtr);
> +
> + if (!page0) {
> + buf_block_t* block = buf_page_get_gen(space,
> + zip_size, 0,
> + RW_X_LATCH,
> + NULL,
> + BUF_GET,
> + __FILE__, __LINE__,
> + &mtr);
> + page0 = buf_block_get_frame(block);
> + }
> +
> + mlog_write_ulint(page0 + FSP_HEADER_OFFSET + FSP_SPACE_FLAGS,
> + fsp_flags, MLOG_4BYTES, &mtr);
> + /* Redo log this as bytewise update to page 0
> + followed by an MLOG_FILE_WRITE_FSP_FLAGS */
> + byte* log_ptr = mlog_open(&mtr, 11 + 8);
> + if (log_ptr != NULL) {
> + log_ptr = mlog_write_initial_log_record_fast(
> + page0,
> + MLOG_FILE_WRITE_FSP_FLAGS,
> + log_ptr, &mtr);
> + mach_write_to_4(log_ptr, fsp_flags);
> + log_ptr += 4;
> + mach_write_to_4(log_ptr, space);
> + log_ptr += 4;
> + mlog_close(&mtr, log_ptr);
> + }
> +
> + mtr_commit(&mtr);
> + lsn_t end_lsn = mtr.end_lsn;
> + buf_flush_init_for_writing(page0, NULL, end_lsn);
> + flags = mach_read_from_4(FSP_HEADER_OFFSET + FSP_SPACE_FLAGS + page0);
> + ut_ad(flags == fsp_flags);
> +
> + /* Flush dirty page to the storage */
> + ulint n_pages = 0;
> + ulint sum_pages = 0;
> + bool success = false;
> + do {
> + success = buf_flush_list(ULINT_MAX, end_lsn, &n_pages);
> + buf_flush_wait_batch_end(NULL, BUF_FLUSH_LIST);
> + sum_pages += n_pages;
> + } while (!success);
> +
> + fil_space_set_fsp_flags(space, fsp_flags);
> + }
> +}
> +
> +/******************************************************************
> +Parse a MLOG_FILE_WRITE_FSP_FLAGS log entry
> +@return position on log buffer */
> +UNIV_INTERN
> +byte*
> +fil_parse_write_fsp_flags(
> +/*======================*/
> + byte* ptr, /*!< in: Log entry start */
> + byte* end_ptr,/*!< in: Log entry end */
> + buf_block_t* block) /*!< in: buffer block */
> +{
> + /* check that redo log entry is complete */
> + uint entry_size = 4 + 4; // size of flags + space_id
> +
> + if (end_ptr - ptr < entry_size){
> + return NULL;
> + }
> +
> + ulint flags = mach_read_from_4(ptr);
> + ptr += 4;
> + ulint space_id = mach_read_from_4(ptr);
> + ptr += 4;
> +
> + ut_a(fsp_flags_is_valid(flags));
> +
> + /* update fil_space memory cache with flags */
> + fil_space_set_fsp_flags(space_id, flags);
> +
> + return ptr;
> +}
> +
> /********************************************************************//**
> Tries to open a single-table tablespace and optionally checks that the
> space id in it is correct. If this does not succeed, print an error message
> index cc4a4f9..f81fd31 100644
> --- a/storage/innobase/fts/fts0fts.cc
> +++ b/storage/innobase/fts/fts0fts.cc
> @@ -1981,7 +1982,13 @@ fts_create_one_index_table(
> dict_mem_table_add_col(new_table, heap, "ilist", DATA_BLOB,
> 4130048, 0);
>
> - error = row_create_table_for_mysql(new_table, trx, false, FIL_SPACE_ENCRYPTION_DEFAULT, FIL_DEFAULT_ENCRYPTION_KEY);
> + /* Get default encryption key id if set */
> + if (new_table && new_table->table_options &&
new_table cannot be NULL here
> + new_table->table_options->encryption_key_id == 0) {
> + new_table->table_options->encryption_key_id = innobase_get_default_encryption_key_id(trx);
do you want to do it here? may be better do it in row_create_table_for_mysql?
> + }
> +
> + error = row_create_table_for_mysql(new_table, trx, false);
>
> if (error != DB_SUCCESS) {
> trx->error_state = error;
> diff --git a/storage/innobase/handler/ha_innodb.cc b/storage/innobase/handler/ha_innodb.cc
> index 527c5be..3d5aaee 100644
> --- a/storage/innobase/handler/ha_innodb.cc
> +++ b/storage/innobase/handler/ha_innodb.cc
> @@ -5459,6 +5460,43 @@ ha_innobase::innobase_initialize_autoinc()
> }
>
> /*****************************************************************//**
> +Set up the table options structure based on frm. */
> +UNIV_INTERN
> +void
> +ha_innobase::set_table_options(
> +/*===========================*/
> + THD* thd, /*!< in: thd */
> + TABLE* table, /*!< in: table */
> + dict_tableoptions_t* options) /*!< in: table options */
> +{
> + ha_table_option_struct* moptions = table->s->option_struct;
> +
> + options->page_compressed = moptions->page_compressed;
> + options->page_compression_level = moptions->page_compression_level;
> + options->atomic_writes = (atomic_writes_t)moptions->atomic_writes;
> + options->encryption = (fil_encryption_t)moptions->encryption;
> + options->encryption_key_id = moptions->encryption_key_id;
> +
> + if (options->encryption_key_id == 0) {
> + options->encryption_key_id = THDVAR(thd, default_encryption_key_id);
> + }
No, this is not how it works. The *server* sets the encryption_key_id
based on the default_encryption_key_id. The engine does not
need to bother.
> +
> + if (options->page_compression_level == 0) {
> + options->page_compression_level = page_zip_level;
> + }
same here
> +
> + /* Table options should be stored if they are not same as
> + defaults */
> + if (moptions->page_compressed ||
> + (options->atomic_writes == ATOMIC_WRITES_ON ||
> + options->atomic_writes == ATOMIC_WRITES_OFF) ||
> + (options->encryption == FIL_SPACE_ENCRYPTION_OFF ||
> + options->encryption == FIL_SPACE_ENCRYPTION_ON)) {
> + options->need_stored = true;
> + }
> +}
> +
> +/*****************************************************************//**
> Creates and opens a handle to a table which already exists in an InnoDB
> database.
> @return 1 if error, 0 if success */
> @@ -11603,6 +11647,10 @@ ha_innobase::check_table_options(
> return "ENCRYPTION_KEY_ID";
>
> }
> +
> + table_options->need_stored = true;
> +
> + table_options->need_stored = true;
typo
> }
>
> /* Check atomic writes requirements */
> @@ -20422,3 +20483,25 @@ ib_push_warning(
> my_free(buf);
> va_end(args);
> }
> +
> +/********************************************************************//**
> +Helper function to get default_encryption_key_id from THD
> +(trx->mysql_thd).
> +@return default_encryption_key_id from THD or
> +FIL_DEFAULT_ENCRYPTION_KEY */
> +UNIV_INTERN
> +ulint
> +innobase_get_default_encryption_key_id(
See comment above. You _probably_ don't need this method
> +/*===================================*/
> + trx_t* trx) /*! in: trx */
> +{
> + ulint key_id = FIL_DEFAULT_ENCRYPTION_KEY;
> +
> + if (trx && trx->mysql_thd) {
> + THD *thd = (THD *)trx->mysql_thd;
> +
> + key_id = THDVAR(thd, default_encryption_key_id);
> + }
> +
> + return (key_id);
> +}
> diff --git a/storage/innobase/handler/i_s.cc b/storage/innobase/handler/i_s.cc
> index ef69e7d..75baa9d 100644
> --- a/storage/innobase/handler/i_s.cc
> +++ b/storage/innobase/handler/i_s.cc
> @@ -9130,3 +9129,230 @@ UNIV_INTERN struct st_maria_plugin i_s_innodb_sys_semaphore_waits =
> STRUCT_FLD(version_info, INNODB_VERSION_STR),
> STRUCT_FLD(maturity, MariaDB_PLUGIN_MATURITY_BETA),
> };
> +
> +/** SYS_TABLE_OPTIONS ************************************************/
> +/* Fields of the dynamic table INFORMATION_SCHEMA.INNODB_SYS_TABLE_OPTIONS */
> +static ST_FIELD_INFO innodb_sys_tableoptions_fields_info[] =
Good idea!
> diff --git a/storage/innobase/include/dict0dict.h b/storage/innobase/include/dict0dict.h
> +{
> + // SYS_TABLE_OPTIONS_TABLE_ID 0
> + {STRUCT_FLD(field_name, "TABLE_ID"),
> + STRUCT_FLD(field_length, MY_INT64_NUM_DECIMAL_DIGITS),
> + STRUCT_FLD(field_type, MYSQL_TYPE_LONGLONG),
> + STRUCT_FLD(value, 0),
> + STRUCT_FLD(field_flags, MY_I_S_UNSIGNED),
> + STRUCT_FLD(old_name, ""),
> + STRUCT_FLD(open_method, SKIP_OPEN_TABLE)},
> +
> + // SYS_TABLE_OPTIONS_PAGE_COMPRESSED 1
> + {STRUCT_FLD(field_name, "PAGE_COMPRESSED"),
> + STRUCT_FLD(field_length, 7),
> + STRUCT_FLD(field_type, MYSQL_TYPE_STRING),
> + STRUCT_FLD(value, 0),
> + STRUCT_FLD(field_flags, 0),
> + STRUCT_FLD(old_name, ""),
> + STRUCT_FLD(open_method, SKIP_OPEN_TABLE)},
> +
> + // SYS_TABLE_OPTIONS_PAGE_COMPRESSION_LEVEL 2
> + {STRUCT_FLD(field_name, "PAGE_COMPRESSION_LEVEL"),
> + STRUCT_FLD(field_length, MY_INT32_NUM_DECIMAL_DIGITS),
> + STRUCT_FLD(field_type, MYSQL_TYPE_LONG),
> + STRUCT_FLD(value, 0),
> + STRUCT_FLD(field_flags, MY_I_S_UNSIGNED),
> + STRUCT_FLD(old_name, ""),
> + STRUCT_FLD(open_method, SKIP_OPEN_TABLE)},
> +
> + // SYS_TABLE_OPTIONS_ATOMIC_WRITES 3
> + {STRUCT_FLD(field_name, "ATOMIC_WRITES"),
> + STRUCT_FLD(field_length, 9),
> + STRUCT_FLD(field_type, MYSQL_TYPE_STRING),
> + STRUCT_FLD(value, 0),
> + STRUCT_FLD(field_flags, 0),
> + STRUCT_FLD(old_name, ""),
> + STRUCT_FLD(open_method, SKIP_OPEN_TABLE)},
> +
> + // SYS_TABLE_OPTIONS_ENCRYPTED 4
> + {STRUCT_FLD(field_name, "ENCRYPTED"),
> + STRUCT_FLD(field_length, 9),
> + STRUCT_FLD(field_type, MYSQL_TYPE_STRING),
> + STRUCT_FLD(value, 0),
> + STRUCT_FLD(field_flags, 0),
> + STRUCT_FLD(old_name, ""),
> + STRUCT_FLD(open_method, SKIP_OPEN_TABLE)},
> +
> + // SYS_TABLE_OPTIONS_ENCRYPTION_KEY_ID 5
> + {STRUCT_FLD(field_name, "ENCRYPTION_KEY_ID"),
> + STRUCT_FLD(field_length, MY_INT32_NUM_DECIMAL_DIGITS),
> + STRUCT_FLD(field_type, MYSQL_TYPE_LONG),
> + STRUCT_FLD(value, 0),
> + STRUCT_FLD(field_flags, MY_I_S_UNSIGNED),
> + STRUCT_FLD(old_name, ""),
> + STRUCT_FLD(open_method, SKIP_OPEN_TABLE)},
> +
> + END_OF_ST_FIELD_INFO
> +};
> +
> +/*******************************************************************//**
> +Function to go through each record in SYS_TABLE_OPTIONS table, and fill the
> +information_schema.innodb_sys_table_options table with related table information
> +@return 0 on success */
> +static
> +int
> +i_s_dict_fill_sys_table_options(
> +/*============================*/
> + THD* thd, /*!< in: thread */
> + TABLE_LIST* tables, /*!< in/out: tables to fill */
> + Item* ) /*!< in: condition (not used) */
> +{
> + btr_pcur_t pcur;
> + const rec_t* rec;
> + mem_heap_t* heap;
> + mtr_t mtr;
> +
> + DBUG_ENTER("i_s_sys_table_options_fill_table");
> + RETURN_IF_INNODB_NOT_STARTED(tables->schema_table_name);
> +
> + /* deny access to user without PROCESS_ACL privilege */
> + if (check_global_access(thd, PROCESS_ACL, true)) {
> + DBUG_RETURN(0);
> + }
> +
> + heap = mem_heap_create(1000);
> + mutex_enter(&(dict_sys->mutex));
> + mtr_start(&mtr);
> +
> + rec = dict_startscan_system(&pcur, &mtr, SYS_TABLE_OPTIONS);
> +
> + while (rec) {
> + const char* err_msg;
> + dict_tableoptions_t options;
> +
> + /* Create and populate a dict_tableoptions_t structure with
> + information from SYS_TABLE_OPTIONS row */
> + memset(&options, 0, sizeof(dict_tableoptions_t));
> +
> + err_msg = dict_process_sys_tableoptions(
> + heap, rec, &options);
> +
> + mtr_commit(&mtr);
> + mutex_exit(&dict_sys->mutex);
> +
> + if (!err_msg) {
> + Field** fields = tables->table->field;
> +
> + OK(fields[SYS_TABLE_OPTIONS_TABLE_ID]->store((longlong) options.table_id));
> + OK(fields[SYS_TABLE_OPTIONS_PAGE_COMPRESSION_LEVEL]->store((ulint)options.page_compression_level));
> + OK(fields[SYS_TABLE_OPTIONS_ENCRYPTION_KEY_ID]->store((ulint)options.encryption_key_id));
> +
> + if (options.page_compressed) {
> + OK(field_store_string(fields[SYS_TABLE_OPTIONS_PAGE_COMPRESSED], "YES"));
> + } else {
> + OK(field_store_string(fields[SYS_TABLE_OPTIONS_PAGE_COMPRESSED], "NO"));
> + }
> +
> + if (options.encryption == FIL_SPACE_ENCRYPTION_DEFAULT) {
> + OK(field_store_string(fields[SYS_TABLE_OPTIONS_ENCRYPTED], "DEFAULT"));
> + } else if (options.encryption == FIL_SPACE_ENCRYPTION_ON) {
> + OK(field_store_string(fields[SYS_TABLE_OPTIONS_ENCRYPTED], "ON"));
> + } else {
> + OK(field_store_string(fields[SYS_TABLE_OPTIONS_ENCRYPTED], "OFF"));
> + }
> +
> + if (options.atomic_writes == ATOMIC_WRITES_DEFAULT) {
> + OK(field_store_string(fields[SYS_TABLE_OPTIONS_ATOMIC_WRITES], "DEFAULT"));
> + } else if (options.atomic_writes == ATOMIC_WRITES_ON) {
> + OK(field_store_string(fields[SYS_TABLE_OPTIONS_ATOMIC_WRITES], "ON"));
> + } else {
> + OK(field_store_string(fields[SYS_TABLE_OPTIONS_ATOMIC_WRITES], "OFF"));
> + }
> +
> + OK(schema_table_store_record(thd, tables->table));
> +
> + } else {
> + push_warning_printf(thd, Sql_condition::WARN_LEVEL_WARN,
> + ER_CANT_FIND_SYSTEM_REC, "%s",
> + err_msg);
> + }
> +
> + mem_heap_empty(heap);
> +
> + /* Get the next record */
> + mutex_enter(&dict_sys->mutex);
> + mtr_start(&mtr);
> + rec = dict_getnext_system(&pcur, &mtr);
> + }
> +
> + mtr_commit(&mtr);
> + mutex_exit(&dict_sys->mutex);
> + mem_heap_free(heap);
> +
> + DBUG_RETURN(0);
> +}
> +/*******************************************************************//**
> +Bind the dynamic table INFORMATION_SCHEMA.INNODB_SYS_TABLE_OPTIONS
> +@return 0 on success */
> +static
> +int
> +innodb_sys_table_options_init(
> +/*============================*/
> + void* p) /*!< in/out: table schema object */
> +{
> + ST_SCHEMA_TABLE* schema;
> +
> + DBUG_ENTER("innodb_sys_table_options_init");
> +
> + schema = (ST_SCHEMA_TABLE*) p;
> +
> + schema->fields_info = innodb_sys_tableoptions_fields_info;
> + schema->fill_table = i_s_dict_fill_sys_table_options;
> +
> + DBUG_RETURN(0);
> +}
> +
> +UNIV_INTERN struct st_maria_plugin i_s_innodb_sys_table_options =
> +{
> + /* the plugin type (a MYSQL_XXX_PLUGIN value) */
> + /* int */
> + STRUCT_FLD(type, MYSQL_INFORMATION_SCHEMA_PLUGIN),
> +
> + /* pointer to type-specific plugin descriptor */
> + /* void* */
> + STRUCT_FLD(info, &i_s_info),
> +
> + /* plugin name */
> + /* const char* */
> + STRUCT_FLD(name, "INNODB_SYS_TABLE_OPTIONS"),
> +
> + /* plugin author (for SHOW PLUGINS) */
> + /* const char* */
> + STRUCT_FLD(author, maria_plugin_author),
> +
> + /* general descriptive text (for SHOW PLUGINS) */
> + /* const char* */
> + STRUCT_FLD(descr, "InnoDB SYS_TABLE_OPTIONS"),
> +
> + /* the plugin license (PLUGIN_LICENSE_XXX) */
> + /* int */
> + STRUCT_FLD(license, PLUGIN_LICENSE_GPL),
> +
> + /* the function to invoke when plugin is loaded */
> + /* int (*)(void*); */
> + STRUCT_FLD(init, innodb_sys_table_options_init),
> +
> + /* the function to invoke when plugin is unloaded */
> + /* int (*)(void*); */
> + STRUCT_FLD(deinit, i_s_common_deinit),
> +
> + /* plugin version (for SHOW PLUGINS) */
> + /* unsigned int */
> + STRUCT_FLD(version, INNODB_VERSION_SHORT),
> +
> + /* struct st_mysql_show_var* */
> + STRUCT_FLD(status_vars, NULL),
> +
> + /* struct st_mysql_sys_var** */
> + STRUCT_FLD(system_vars, NULL),
> +
> + /* Maria extension */
> + STRUCT_FLD(version_info, INNODB_VERSION_STR),
> + STRUCT_FLD(maturity, MariaDB_PLUGIN_MATURITY_BETA),
> +};
> index b15d364..fa65884 100644
> --- a/storage/innobase/include/dict0dict.h
> +++ b/storage/innobase/include/dict0dict.h
> @@ -944,15 +948,8 @@ dict_tf_set(
> ulint* flags, /*!< in/out: table */
> rec_format_t format, /*!< in: file format */
> ulint zip_ssize, /*!< in: zip shift size */
> - bool remote_path, /*!< in: table uses DATA DIRECTORY
> + bool remote_path); /*!< in: table uses DATA DIRECTORY
> */
> - bool page_compressed,/*!< in: table uses page compressed
> - pages */
> - ulint page_compression_level, /*!< in: table page compression
> - level */
> - ulint atomic_writes) /*!< in: table atomic
> - writes option value*/
> - __attribute__((nonnull));
Why do you remove nonnull attributes? They help gcc to optimize the code
> /********************************************************************//**
> Convert a 32 bit integer table flags to the 32 bit integer that is
> written into the tablespace header at the offset FSP_SPACE_FLAGS and is
> diff --git a/storage/innobase/include/dict0dict.ic b/storage/innobase/include/dict0dict.ic
> index a3a3446..3580081 100644
> --- a/storage/innobase/include/dict0dict.ic
> +++ b/storage/innobase/include/dict0dict.ic
> @@ -685,11 +629,7 @@ dict_sys_tables_type_validate(
> ulint zip_ssize = DICT_TF_GET_ZIP_SSIZE(type);
> ulint atomic_blobs = DICT_TF_HAS_ATOMIC_BLOBS(type);
> ulint unused = DICT_TF_GET_UNUSED(type);
> - ulint page_compression = DICT_TF_GET_PAGE_COMPRESSION(type);
> - ulint page_compression_level = DICT_TF_GET_PAGE_COMPRESSION_LEVEL(type);
By the way, what about page compression in MySQL InnoDB?
where is that stored? will you support those tables?
> - ulint atomic_writes = DICT_TF_GET_ATOMIC_WRITES(type);
> -
> - ut_a(atomic_writes <= ATOMIC_WRITES_OFF);
> + ulint unused_mariadb = DICT_TF_GET_UNUSED_MARIADB(type);
>
> /* The low order bit of SYS_TABLES.TYPE is always set to 1.
> If the format is UNIV_FORMAT_B or higher, this field is the same
> diff --git a/storage/innobase/include/fil0fil.h b/storage/innobase/include/fil0fil.h
> index 7fe0cc8..bc7f317 100644
> --- a/storage/innobase/include/fil0fil.h
> +++ b/storage/innobase/include/fil0fil.h
> @@ -39,7 +39,7 @@ Created 10/25/1995 Heikki Tuuri
> #include "ibuf0types.h"
> #include "log0log.h"
> #endif /* !UNIV_HOTBACKUP */
> -
> +#include "my_crypt.h"
why?
> #include <list>
>
> // Forward declaration
> diff --git a/storage/innobase/srv/srv0start.cc b/storage/innobase/srv/srv0start.cc
> index d7b37b5..29ce93d 100644
> --- a/storage/innobase/srv/srv0start.cc
> +++ b/storage/innobase/srv/srv0start.cc
> @@ -2537,7 +2543,16 @@ innobase_start_or_create_for_mysql(void)
>
> sys_datafiles_created = true;
>
> - /* This function assumes that SYS_DATAFILES exists */
> + err = dict_create_or_check_sys_table_options();
> +
> + if (err != DB_SUCCESS) {
> + return(err);
> + }
> +
> + sys_table_options_created = true;
> +
> + /* This function assumes that SYS_DATAFILES
> + and SYS_TABLE_OPTIONS exists */
"exist"
> dict_check_tablespaces_and_store_max_id(dict_check);
> }
Regards,
Sergei
Chief Architect MariaDB
and security@mariadb.org