maria-developers team mailing list archive

[Sergei Golubchik <serg@xxxxxxxxxxx>]

Hi, Alexey!

On Dec 03, Alexey Botchkov wrote:
> revision-id: 07f5d036fbb5bdcb011a84f5c882a062d9e609e7 (mariadb-10.4.0-54-g07f5d03)
> parent(s): 3afae13b548c903d86a55530d59fbf7d8666281f
> committer: Alexey Botchkov
> timestamp: 2018-12-03 02:35:52 +0400
> message:
> 
> MDEV-5313 Improving merge audit api.

"improving audit api" ?
we don't merge it anymore, right?

> service_json and service_cfg_table interfaces added.
> 
> diff --git a/include/mysql/service_cfg_table.h b/include/mysql/service_cfg_table.h
> new file mode 100644
> index 0000000..36660ff
> --- /dev/null
> +++ b/include/mysql/service_cfg_table.h
> @@ -0,0 +1,109 @@
> +/* Copyright (C) 2018 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, Fifth Floor, Boston, MA 02111-1301 USA */
> +
> +#ifndef MYSQL_SERVICE_CFG_TABLE
> +#define MYSQL_SERVICE_CFG_TABLE
> +
> +/**
> +  @file
> +  cfg table service
> +
> +  Reading plugin settings from the server table.
> +
> +  This service lets the plugin to read it's settings
> +  from the server table.
> +  This 'cfg' table has two VARCHAR fields per row
> +  which are
> +     'id' - the name of the setting
> +     'value' - the value of the respective 'id' setting
> +  The structure 'cfg_table_row' reflects this format for
> +  the data interchange with the service.
> +  Fuctions of the service:
> +    cfg_table_check_exists
> +        checks if the table exists.
> +    cfg_table_create - creates the table
> +      parameers:
> +          table_name - the name of the table
> +          int id_len, int value_len - the length of the 'id' and 'value'
> +                                      fields.
> +          const struct cfg_table_row *defaults - table will be filled
> +                        with these default rows.
> +                        The list of rows is ended by {0,0} row
> +
> +    cfg_table_set - replaces (inserts if not exists) the setting
> +      parameters:
> +        table_name - the name of the table
> +        const char *id, const char *value - the 'id' and the new value
> +
> +    cfg_table_get - reads the cft tale content
> +    It return the number of rows in the table or -1 if error.
> +      parameters:
> +         table_name,
> +         struct cfg_table_row *data_buf - buffer to store the records
> +         int n_buf_row - the size of the buffer
> +*/

This is, of course, a very much overdue functionality.
I'm not sure it should be limited to key/value pairs.
On the other hand, if it's plugin config only, loaded once on startup -
then it should be fine. Just need to make clear it's not for run-time
data.

In the implementation, make sure that when a table is opened, there are
no other opened/locked tables. This will avoid deadlocks and prevent
plugins from using it from inside another statement.

Also,

* I'd put the table name into pluginname namespace. Not quite sure how
  to do it, though.

* Why do you need cfg_table_check_exists and cfg_table_create? A plugin
  only needs set and get methods. A table is automatically created on
  the first access, a plugin can always assume it exists.

* may be even just one table, plugin_config? with two columns
  plugin_name and json_config? plugin gets the json on startup and
  that's all. No way for a plugin to access the table directly.

> diff --git a/include/mysql/service_json.h b/include/mysql/service_json.h
> new file mode 100644
> index 0000000..5d0e260
> --- /dev/null
> +++ b/include/mysql/service_json.h
...
> +int js_type(const char *js, const char *js_end,
> +            enum js_value_types *vt,
> +            const char **v, int *vlen);
> +int js_get_array_item(const char *js, const char *js_end, int n_item,
> +                      enum js_value_types *vt,
> +                      const char **v, int *vlen);
> +int js_get_object_key(const char *js, const char *js_end, const char *key,
> +                      enum js_value_types *vt,
> +                      const char **v, int *vlen);
> +int js_get_object_nkey(const char *js,const char *js_end, int nkey,
> +                       const char **keyname, const char **keyname_end,
> +                       enum js_value_types *vt,
> +                       const char **v, int *vlen);

I wouldn't introduce ten different apis to access json. Just
json_get(json, path) and json_set(json, path, value). Same path syntax
everywhere, like on SQL level.

Regards,
Sergei
Chief Architect MariaDB
and security@xxxxxxxxxxx