[syslog-ng] [PATCH (3.4)] afsql: Add support for specifying custom SQL backend options.

[Balazs Scheidler]

On Wed, 2011-12-21 at 14:52 +0100, Gergely Nagy wrote:
> Balazs Scheidler <bazsi at balabit.hu> writes:
> 
> > On Wed, 2011-12-07 at 12:11 +0100, Gergely Nagy wrote:
> >> When one is using an SQL backend that supports a set of options that
> >> syslog-ng does not set itself, yet, one does want these options set,
> >> there was previously no way to accomplish that without changing the
> >> source.
> >> 
> >> This patch adds a new option: dbd-option(OPTION_NAME VALUE) (where
> >> OPTION_NAME is always a string, and value can be either a string, or a
> >> number). The options specified this way will always be set with libdbi
> >> whenever we (re)connect to the database.
> >> 
> >> A simple use-case:
> >> 
> >> destination d_slow_sql_test {
> >>   sql(type("null")
> >>       dbd-option("null.sleep.connect" 1)
> >>       dbd-option("null.sleep.query" 5)
> >>       columns("dummy")
> >>       values("1")
> >>   );
> >> };
> >> 
> >> Signed-off-by: Gergely Nagy <algernon at balabit.hu>
> >> ---
> >>  modules/afsql/afsql-grammar.ym |    3 +++
> >>  modules/afsql/afsql-parser.c   |    2 ++
> >>  modules/afsql/afsql.c          |   40 ++++++++++++++++++++++++++++++++++++++++
> >>  modules/afsql/afsql.h          |    4 ++++
> >>  4 files changed, 49 insertions(+), 0 deletions(-)
> >
> > I would apply this, however the set of dbd-options really depends on the
> > libdbi driver in question. And if once-upon-a-time I'd leave libdbi
> > behind and use something else, these would not work.
> >
> > So I would put a big-fat warning that the meaning of these options can
> > be changed at any time.
> 
> I can prepare a patch that would do two things: rename the option from
> dbd-option to libdbi-option, to make it clear that it's
> libdbi-specific. And only enable these when a certain flag is set in the
> destination.
> 
> So we'd have something like this:
> 
> destination d_slow_sql_tet {
>  sql(type("null")
>      flags(libdbi-direct-options)
>      libdbi-option("null.sleep.query" 5)
>      columns("dummy")
>      values("1")
>  );
> };
> 
> Without the flag, libdbi-option would emit a warning and ignore the
> setting.
> 
> Would this be acceptable?
> 
> The reason behind the rename, is that if you ever change away from
> libdbi, nothing guarantees that the replacement will support setting
> custom options, so dbd-option would be pointless there.
> 

I've applied the original patch, and I wouldn't play tricks with flags
like this. The now think that it is possible to emit that warning once
the SQL backend library changes, and possibly point out this future
problem in the documentation.

-- 
Bazsi