[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

RE: Old bug, old module - What should be right?



Julian Mehnle wrote:
> You do not have to make an incompatible change.  "user" is very well
> allowed as a column name in PostgreSQL, you just have to double-quote it
> to distinguish it from the "user" keyword:
> 
> | CREATE TABLE requests (
> |     server varchar(127) DEFAULT '' NOT NULL,
> |     bytes mediumint(9) DEFAULT '0' NOT NULL,
> |     "user" varchar(15) DEFAULT '' NOT NULL,
> |     [...]
> | );
> 
> This doesn't even justify a change of the sample SQL code in the
> module's documentation, since this issue is indeed PostgreSQL specific.
> Perhaps a Pg-specific note in the docs would do.

I was wrong.  A more substantial change (though not an incompatible one)
is necessary in order to make Apache::DBILogger actually work with such a
table:

The module must properly use SQL quoting, which is supported by DBI.  The
module actually quotes the to-be-inserted values already, but not the
column names.  Here is a minimally invasive patch[1].

> On a general note, having had a quick look at Apache::DBILogger, I think
> this module is in pretty bad shape.  The CPAN module failed 4 of 5 test
> builds[1], and the documentation's formatting is broken, especially that
> of the sample SQL code -- try to look at the module's documentation in
> HTML format[2].

The module's code is awful, too, now that I've had a closer look at it.

References:
  1. http://files.mehnle.net/software/perl/libapache-dbilogger-perl-0.93-SQL-quoting.diff



Reply to: