maria-developers team mailing list archive

[Michael Widenius <monty@xxxxxxxxxxxx>]

Hi!

>>>>> "sanja" == sanja  <sanja@xxxxxxxxxxxx> writes:

sanja> At file:///Users/bell/maria/bzr/work-maria-5.2-engine/
sanja> ------------------------------------------------------------
sanja> revno: 2734
sanja> revision-id: sanja@xxxxxxxxxxxx-20091204114937-cfkvax0g36d3nq3j
sanja> parent: psergey@xxxxxxxxxxxx-20091202142609-18bp41q8mejxl47t
sanja> committer: sanja@xxxxxxxxxxxx
sanja> branch nick: work-maria-5.2-engine
sanja> timestamp: Fri 2009-12-04 13:49:37 +0200
sanja> message:
sanja>   Maria WL#61
  
<cut>


> +++ b/CMakeLists.txt	2009-12-04 11:49:37 +0000
> @@ -251,6 +251,7 @@
>  
>          IF (ENGINE_BUILD_TYPE STREQUAL "STATIC") 
>            SET (mysql_plugin_defs  "${mysql_plugin_defs},builtin_${PLUGIN_NAME}_plugin")
> +          SET (mariaext_plugin_defs  "${mariaext_plugin_defs},bltnmext_${PLUGIN_NAME}_plugin")
>            SET (MYSQLD_STATIC_ENGINE_LIBS ${MYSQLD_STATIC_ENGINE_LIBS} ${PLUGIN_NAME})
>            SET (STORAGE_ENGINE_DEFS "${STORAGE_ENGINE_DEFS} -DWITH_${ENGINE}_STORAGE_ENGINE")
>            SET (WITH_${ENGINE}_STORAGE_ENGINE TRUE)
> @@ -269,6 +270,7 @@
>  IF(NOT WITHOUT_PARTITION_STORAGE_ENGINE)
>    SET (STORAGE_ENGINE_DEFS "${STORAGE_ENGINE_DEFS} -DWITH_PARTITION_STORAGE_ENGINE")
>    SET (mysql_plugin_defs  "${mysql_plugin_defs},builtin_partition_plugin")
> +  SET (mariaext_plugin_defs  "${mariaext_plugin_defs},bltnmext_partition_plugin")
>  ENDIF(NOT WITHOUT_PARTITION_STORAGE_ENGINE)

Probably stupid questions, but why mariaext instead of just mariadb?

What does 'bltnmext_partition_plugin' stand for ?
Can we use a more understandable name, like builtin_mariadb_partition_plugin ?

<cut>

> --- a/include/mysql/plugin.h	2009-09-07 20:50:10 +0000

<cut>

> +/*
> +  MariaDB extension for plugins declaration structure.
> +*/
> +
> +struct st_mariaext_plugin
> +{
> +  const char *sversion;  /* plugin version string */

sversion -> version ?

> +  int maturity;          /* HA_PLUGIN_MATURITY_XXX */
> +};
> +
>  /*************************************************************************
>    API for Full-text parser plugin. (MYSQL_FTPARSER_PLUGIN)
>  */
> 
> === modified file 'sql/sql_plugin.cc'
> --- a/sql/sql_plugin.cc	2009-11-12 04:31:28 +0000
> +++ b/sql/sql_plugin.cc	2009-12-04 11:49:37 +0000

> +
> -  for (plugin= tmp.plugin_dl->plugins; plugin->info; plugin++)
> +  for (plugin= tmp.plugin_dl->plugins, ext= tmp.plugin_dl->mariaext;
> +       plugin->info;
> +       plugin++, ext++)
>    {
> +    mariaext= ext;
> +    if (!ext->sversion)
> +    {
> +      mariaext= empty_mariaext;
> +      ext--;
> +    }

Please add a comment for the above if.
For example:

/*
  Plugin didn't have any mariadb extensions; Use default one and
  reset counter to do same for next internal plugin.
*/

> +    /* extensions should be the same numbers as static plugins) */
> +    DBUG_ASSERT(*bltnmexts);
> +    for (plugin= *builtins, ext= *bltnmexts;
> +         plugin->info;
> +         plugin++, ext++)
>      {
> +
> +      /* in case if plugin describe less extensions then plugins */
> +      mariaext= ext;
> +      if (!ext->sversion)
> +      {
> +         mariaext= empty_mariaext;
> +         ext--;
> +      }
> +

I am a bit unsure about the above code;  If we allow one to describe
less extensions, then it's not sure that when one adds a new plugin
with extension information, one remembers to also add all missing
extensions.

I think that for staticly compiled plugins we should require that
they have also extension information. What do you think about this?

<cut>

> --- a/sql/sql_show.cc	2009-11-12 04:31:28 +0000

<cut>

> +  switch (mariaext->maturity) {
> +  case PLUGIN_MATURITY_UNKNOWN:
> +    table->field[10]->store(PLUGIN_MATURITY_UNKNOWN_STR,
> +                            sizeof(PLUGIN_MATURITY_UNKNOWN_STR) - 1, cs);
> +    break;
> +  case PLUGIN_MATURITY_TEST:
> +    table->field[10]->store(PLUGIN_MATURITY_TEST_STR,
> +                            sizeof(PLUGIN_MATURITY_TEST_STR) - 1, cs);
> +    break;
> +  case PLUGIN_MATURITY_ALPHA:
> +    table->field[10]->store(PLUGIN_MATURITY_ALPHA_STR,
> +                            sizeof(PLUGIN_MATURITY_ALPHA_STR) - 1, cs);
> +    break;
> +  case PLUGIN_MATURITY_BETA:
> +    table->field[10]->store(PLUGIN_MATURITY_BETA_STR,
> +                            sizeof(PLUGIN_MATURITY_BETA_STR) - 1, cs);
> +    break;
> +  case PLUGIN_MATURITY_GAMMA:
> +    table->field[10]->store(PLUGIN_MATURITY_GAMMA_STR,
> +                            sizeof(PLUGIN_MATURITY_GAMMA_STR) - 1, cs);
> +    break;
> +  case PLUGIN_MATURITY_RELEASE:
> +    table->field[10]->store(PLUGIN_MATURITY_RELEASE_STR,
> +                            sizeof(PLUGIN_MATURITY_RELEASE_STR) - 1, cs);
> +    break;

If we would store the maturity as an array of LEX_STRINGS, we could do
the above as:

    if ((uint) mariaext->maturity <= PLUGIN_MATURITY_RELEASE)
      table->field[10]->store(maturity_array[mariaext->maturity].str,
                              maturity_array[mariaext->maturity].length,
                              cs);
    else
    {
      DBUG_ASSERT(0);
      table->field[10]->store("Unknown", 7, cs);
    }

<cut>

> === modified file 'storage/federatedx/ha_federatedx.cc'
> --- a/storage/federatedx/ha_federatedx.cc	2009-11-03 11:08:09 +0000
> +++ b/storage/federatedx/ha_federatedx.cc	2009-12-04 11:49:37 +0000
> @@ -3491,3 +3491,9 @@
>    NULL                        /* config options                  */
>  }
>  mysql_declare_plugin_end;
> +mariaext_declare_plugin(federated)
> +{
> +  "1.0",                      /* string version */
> +  PLUGIN_MATURITY_UNKNOWN     /* maturity */
> +}

->
> +  "2.0",                      /* string version */
> +  PLUGIN_MATURITY_BETA        /* maturity */

> === modified file 'storage/pbxt/src/ha_pbxt.cc'
> --- a/storage/pbxt/src/ha_pbxt.cc	2009-09-03 06:15:03 +0000
> +++ b/storage/pbxt/src/ha_pbxt.cc	2009-12-04 11:49:37 +0000
> @@ -5507,6 +5507,18 @@
>  drizzle_declare_plugin_end;
>  #else
>  mysql_declare_plugin_end;
> +#ifdef MARIADB_BASE_VERSION
> +mariaext_declare_plugin(pbxt)
> +{ /* PBXT */
> +  "1.0",                      /* string version */

1.0.09g RC3

> +  PLUGIN_MATURITY_UNKNOWN     /* maturity */

PLUGIN_MATURITY_GAMMA

> +},
> +{ /* PBXT_STATISTICS */
> +  "1.0",                      /* string version */

1.0.09g RC3

> +  PLUGIN_MATURITY_UNKNOWN     /* maturity */

PLUGIN_MATURITY_GAMMA

> +}

Regards,
Monty