serg, I believe patches are good enough for your review, there're 2 in bb-10.5-robert.
da6d7f7 is the main patch
7740cb2 add-on making SE plugins to not use plugin->data

This doesn't look much like a declarative approach, one still has to write boilerplate code like

struct partition_handlerton : public handlerton

{

  partition_handlerton()

  {

    db_type= DB_TYPE_PARTITION_DB;

    create= partition_create_handler;

    partition_flags= ::partition_flags;

    alter_table_flags= ::alter_table_flags;

    flags= HTON_NOT_USER_SELECTABLE | HTON_HIDDEN |

           HTON_TEMPORARY_NOT_SUPPORTED;

    tablefile_extensions= ha_partition_ext;

  }

};

May be we can use designated initializers instead? They're ISO C99, and as we're using C++11, C99 supposedly should be fine.

With designated initializers one would write something like

struct st_mysql_storage_engine hton= {

  MYSQL_HANDLERTON_INTERFACE_VERSION,

  .db_type= DB_TYPE_PARTITION_DB,

  .create= partition_create_handler,

  .partition_flags= partition_flags,

  .alter_table_flags= alter_table_flags,

  .flags= HTON_NOT_USER_SELECTABLE | HTON_HIDDEN |

           HTON_TEMPORARY_NOT_SUPPORTED,

  .tablefile_extensions= ha_partition_ext

};

The question is — whether it'll work on all our builders.

Hi serg, designated initializers weren't included in the c++11 standard and this is the reason I avoided them and chose the current approach. They will be available starting from c++20.

robertbindar, given that designated initializers are supported by gcc/clang since quite a while as well as by MSVC since recently, Sergei is suggesting to disregard their non-standard status and make use of them.

serg, from the API as well as the server point of view it is pretty much declarative. Also what Robert has implemented is the right thing to do if we agree to move toward virtual methods. Said the above, could you confirm designated initializers is what we should do?

Not necessarily. If designated initializers are not acceptable yet, we can do something else. I had this macro workaround that almost looks like designated initializers

#define init_ht(...)                              \

[]{                                               \

  st_mysql_storage_engine _;                      \

  _.version = MYSQL_HANDLERTON_INTERFACE_VERSION; \

  __VA_ARGS__;                                    \

  return _;                                       \

}()

and

static struct st_mysql_storage_engine hton= init_ht(

  _.db_type= DB_TYPE_PARTITION_DB,

  _.create= partition_create_handler,

  _.partition_flags= partition_flags,

  _.alter_table_flags= alter_table_flags,

  _.flags= HTON_NOT_USER_SELECTABLE | HTON_HIDDEN |

           HTON_TEMPORARY_NOT_SUPPORTED,

  _.tablefile_extensions= ha_partition_ext

);

which is fairly close to the true designated initializers syntax three comments above.

But designated initializers, that trick, or something else — the point is to let storage engine authors to write as little code as possible, to reduce the amount of boilerplate code.

But this robertbindar's commit just moves code around, from init() function into constructors. Same amount of code, slightly different boilerplate decoration.

Besides, what are the benefits of using a declarative approach here? Consistency with other plugin types, I agree. Anything else?

First plugins used the declarative approach because it, precisely, required less boilerplate code — many plugins don't need any init() function at all. This argument hardly applies to storage engines, they almost always needs init() and often initialize handlerton very sparsely, so full structure initializers would be fairly unreadable for them.

1. We removed init() from ha_sequence, oqgraph, sequence, test_sql_discovery. Not much indeed.
2. Quite a few engines had to set global engine_hton pointer in init(), which is allocated by the server and used internally by an engine.
3. Mixed hton initialisation (partially by the server, partially by an engine) is not great either.
4. We could eventually eliminate st_plugin_int::data and some complexity around it.

In other words what Robert did, makes a few aspects of hton handling more intuitive and natural. Unfortunately producing derived classes to allow sparse initialisation doesn't look nice indeed.

To wrap up my position:
I feel like it were clearly beneficial as it is if we were heading towards virtual methods instead of function pointers.
If that's not something we want, backoff to not-yet-standard designited initializers.
If that doesn't work, abandon this effort until either of the above becomes available.

I have a feeling this just replaces boilerplate code with another kind of a boilerplate code. Number of lines didn't go down, it even went up a little bit.

But it's good that simple engines can do without an init method. Meaning there's a reason to try to do the declarative approach after all.

what's wrong with 2?

agree about 3 and 4.

2 is mostly about a few additional lines of code to save a pointer to server allocated handlerton. E.g. they may have to implement init() only to save this pointer.