[logback-dev] [JIRA] Created: (LBCLASSIC-188) Make table andcolumn names overridable

[Ceki Gulcu]

I beleive we got sidestacked by the question of generics which imo are
secondary to the core problem. If we assume that the number and type
of each database column is fixed, with some wiggling room allowed for
the actual name of the column, once the event type is known the actual
structure of the tables is fixed for each DBAppender class. Note that
DBAppenderBase already uses generics. Both
ch.qos.logback.access.db.DBAppender and
ch.qos.logback.classic.db.DBAppender extending
ch.qos.logback.core.db.DBAppenderBase. The two DBAppender classes
target different table structures.

Now if we forget about AccessEvent and the logback-access module and
concentrate only on ch.qos.logback.classic.db.DBAppender, letting
DBAppender cater for different table structures would make the code
too complex and hard to maintain. If the table structure as it exists
in DBAppender is deemed too wasteful, we can improve the
structure. What I would like to avoid is a DBAppender which is so
abstract that each project implements its own version of it.

Anyway, DBNameResolver would allow for an indirection of table and
column names. It's a small incremental change.

If we need to add extra columns for the logger request arguments, we
can do that without breaking compatibility too horribly. (Adding
columns to a database is usually not a problem).

Normalization of various fields makes sense and if it can be achieved
without increasing too much the number of queries per insertion, then
it would be a good improvement to consider.

Instead of debating abstractions, looking at real code would
perhaps be more constructive at this stage.

On 26.02.2010 16:03, Durchholz, Joachim wrote:
> Just a general note:
>
> It think that the number and semantics of columns should be defined in
> the same class that actually fills them via stmt.setXxx calls. I guess
> that's what got me thinking about improvement proposals in the first
> place.
>
> I see three ways to accomplish this:
> 1. Subclass DBAppender.
> 2. Use a DBWriter class that logs whatever information is thrown at it.
> This means generics, because the "whatever information" depends on the
> kind of appender used but can be statically determined for each
> appender. (More details on the nature of generics below.)
> 3. Use a DBNameResolver class that provides column names. The case for
> generics is the same: different appenders will want to access different
> but statically predetermined sets of columns.
>
>> I wish I had a good and short explanation why generics do not
>> make sense here. As mentioned a little earlier,
>> DBNameResolver provides a service to DBAppender. DBAppender
>> has a DBNameResolver but does not store a collection of
>> DBNameResolvers or process DBNameResolvers.
>
> Making a class generic does not mean that it "contains" or "processes"
> objects of the parameter class. java.lang.Comparable is the classical
> counterexample to such interpretations of genericity.
> Generics are a means of consistently varying a type across result and
> parameter positions within a class or class sub-hierarchy. This can be
> used to make containers, but there are many more use cases than that.
>
>> Besides that, generics in Java are quite clumsy to use.
>
> Agreed.
> Code completion does a lot to alleviate the pain, and getting used to
> the way that generics look like does help, too.
>
>> More to the point
>> and as you probably know already, generics get erased (type
>> erasure) as explained in
>>
>>     http://java.sun.com/docs/books/tutorial/java/generics/erasure.html
>
> That does not affect the case at hand. As long as the code gets run
> through a Java compiler, you will still get static guarantees that your
> code does not violate the type constraint.
> The JVM will erase the type parameter and hence cannot enforce the
> constraints at class load time, plus type erasure will prevent some type
> checks at the Java level, but this is not the case for the code I
> proposed.
> (Also, type erasure is schedules for removal. I hope it will go away
> with Java 7.)
>
>> One cannot do much with java generics really.
>
> My personal experience has been otherwise.
> I routinely write generics to good effect.
>
> There is a learning curve.
> I worked from an Eiffel background, which is a bit less refined about
> type bounds than Java, and it took me around a week to wrap my mind
> about the basic workings and another week to pick up some fine points.
> I reaped benefits, too. I have converted some of my code to generics,
> and I found several cases where type casts and string discriminators
> that I had considered statically safe were, in face, unsafe and just
> waiting for the rare occasion to throw an exception. Equivalently, I got
> rid of a lot of "else { throw new RuntimeException ("This can't
> happen"); }" type of code.
>
> YMMV :)
>
> ----
>
>> Anyway, what's your second idea?
>
> I'd simply use overridable functions in DBAppender to provide the SQL
> for database access.
>
> So:
>
> class DBAppender {
>    protected String eventTableName() { return "event_log"; }
>    protected String eventTimestampColumnName () { return "timestmp"; }
>    ...
>    protected String insertPropertiesSql;
>    protected void subAppend(...) {
>      if (insertPropertiesSql == null) {
>        insertPropertiesSql =
>          "INSERT INTO " + eventTableName() + "("
>          + eventTimestampColumnName() + ", "
>          ...
>      }
>      ... // existing code goes here
>    }
> }
>
> Subclasses can easily override the table and column names to their
> taste.
> (Sounds simpler to me than any of the alternatives. DBAppender should
> not compute anything, it should be a thin layer that just spits out the
> data that it's given. It shouldn't even be merging properties, it should
> be given pre-merged property sets by a caller... or an object that does
> the merge on request. Hey, LoggingEvent could do that, all input for
> mergePropertyMaps is from a LoggingEvent anyway.) (Oops. That went off
> on a tangent. Sorry for that.)
>
> Regards,
> Jo
> _______________________________________________
> logback-dev mailing list
> logback-dev at qos.ch
> http://qos.ch/mailman/listinfo/logback-dev