[JIRA] Created: (LBCLASSIC-188) Make table and column names overridable
Make table and column names overridable --------------------------------------- Key: LBCLASSIC-188 URL: http://jira.qos.ch/browse/LBCLASSIC-188 Project: logback-classic Issue Type: Sub-task Components: appender Affects Versions: 0.9.18 Reporter: Ceki Gulcu Assignee: Logback dev list To comply with local project rules, it can be helpful if the table and column names used by DBAppender could be overridden. The easiest way to solve this problem is for DBAppender to delegate the resolution of table names and columns to a different class, say DBNameResolver. Here is a possible interface: interface DBNameResolver { String getTableName(String standardTableName); String getColumnName(String standardColumnName); } The default implementation of DBNameResolver could be: class DefaulDBNameResolver implements DBNameResolver { public String getTableName(String standardTableName) { if("logging_event".equals(standardTableName) { return standardTableName; } if("logging_event_property".equals(standardTableName) { return standardTableName; } if("logging_event_exception".equals(standardTableName) { return standardTableName; } throw new IllegalArgumentException(standardTableName + " is an unknown table name"); } String getColumnName(String standardColumnName) { ... } } If a user wanted to use different names she would simply implement DBNameResolver according to her requirements. Any volunteers to implement this? -- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.qos.ch/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
public String getTableName(String standardTableName) { if("logging_event".equals(standardTableName) { return standardTableName; } if("logging_event_property".equals(standardTableName) { return standardTableName; } if("logging_event_exception".equals(standardTableName) { return standardTableName; } throw new IllegalArgumentException(standardTableName + " is an unknown table name"); }
How about enum TableName {EVENT, EVENT_PROPERTY, EVENT_EXCEPTION} public String getTableName(TableName tableName) { switch (tableName) { case EVENT: return "logging_event"; case EVENT_PROPERTY: return "logging_event_property"; case EVENT_EXCEPTION: return "logging_event_exception"; } } Using an enum has two advantages: - Compilers can (some will) complain if you forget a case in the switch - uses of TableName can be traced with cross-referencing tools The disadvantage is that you can't extend the list. However, if that kind of flexibility is needed, I'm not sure that a separate resolver class is a good solution. It would need to provide a table name for whatever the internal workings of DBAppender require, so the coupling would be too tight. (If logback can use generic classes, then DBAppender<DBNameResolver> would be optimal IMHO.) Regards, Jo
(If logback can use generic classes, then DBAppender<DBNameResolver> would be optimal IMHO.)
Right, http://logback.qos.ch/dependencies.html says Java 1.5. Here's a design outline with generics. I left out enums because I'm not so sure whether they're such a great idea after all, and because it works just fine without them :) First, the base class: class DBAppender<DBNameResolver> { // Uses dbNameResolver.getEventTableName etc. } class DBNameResolver { String getEventTableName () { return "logging_event"; } String getTimestampColumnName () { return "timestmp"; } String getFormattedMessageColumnName () { return "formatted_message"; } ... String getEventPropertyTableName () { return "logging_event_property"; } ... String getEventExceptionTableName () { return "logging_event_exception"; } ... } A variant for storing log parameters: class DBAppenderWithParameters<DbNameResolverWithParameters> { // Uses new dbNameResolver function getArgumentColumnNames // to construct the INSERT statement } class DbNameResolverWithParameters extends DBNameResolver { // Keep inherited definitions, just add the parameter column names String [] argumentColumnNames = new String [] {"arg1", "arg2", "arg3", "arg4", "arg5", "arg5", "arg7", "arg8"}; String [] getArgumentColumnNames { return argumentColumNames; } } HTH Jo
I appreciate the input although considering that DBNameResolver is a mere component of DBAppender, using generics in this case is probably not the best design one can come up with. On 26.02.2010 13:27, Durchholz, Joachim wrote:
(If logback can use generic classes, then DBAppender<DBNameResolver> would be optimal IMHO.)
Right, http://logback.qos.ch/dependencies.html says Java 1.5.
Here's a design outline with generics. I left out enums because I'm not so sure whether they're such a great idea after all, and because it works just fine without them :)
First, the base class:
class DBAppender<DBNameResolver> { // Uses dbNameResolver.getEventTableName etc. } class DBNameResolver { String getEventTableName () { return "logging_event"; } String getTimestampColumnName () { return "timestmp"; } String getFormattedMessageColumnName () { return "formatted_message"; } ... String getEventPropertyTableName () { return "logging_event_property"; } ... String getEventExceptionTableName () { return "logging_event_exception"; } ... }
A variant for storing log parameters:
class DBAppenderWithParameters<DbNameResolverWithParameters> { // Uses new dbNameResolver function getArgumentColumnNames // to construct the INSERT statement } class DbNameResolverWithParameters extends DBNameResolver { // Keep inherited definitions, just add the parameter column names String [] argumentColumnNames = new String [] {"arg1", "arg2", "arg3", "arg4", "arg5", "arg5", "arg7", "arg8"}; String [] getArgumentColumnNames { return argumentColumNames; } }
HTH Jo
I appreciate the input although considering that DBNameResolver is a mere component of DBAppender, using generics in this case is probably not the best design one can come up with.
The generic design allows subclasses of DBAppender to require (and be guaranteed to get) additional column names. E.g. LBCLASSIC-187 "extend table definition with columns for message parameters" will work with this. Enums won't, you can't extend the list of values in a subclass. The generics solution would also allow extensions in other directions. E.g. to provide additional table names, such as a separate table to normalize out message parameter values. Hope the feedback is still useful Jo P.S.: I've got a last proposal up my sleeve that's geared towards simplicity. But one at a time :)
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. Besides that, generics in Java are quite clumsy to use. 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 One cannot do much with java generics really. To be honest, I don't see the point of generics here. Anyway, what's your second idea? On 26.02.2010 14:01, Durchholz, Joachim wrote:
I appreciate the input although considering that DBNameResolver is a mere component of DBAppender, using generics in this case is probably not the best design one can come up with.
The generic design allows subclasses of DBAppender to require (and be guaranteed to get) additional column names.
E.g. LBCLASSIC-187 "extend table definition with columns for message parameters" will work with this. Enums won't, you can't extend the list of values in a subclass.
The generics solution would also allow extensions in other directions. E.g. to provide additional table names, such as a separate table to normalize out message parameter values.
Hope the feedback is still useful Jo
P.S.: I've got a last proposal up my sleeve that's geared towards simplicity. But one at a time :)
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
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@qos.ch http://qos.ch/mailman/listinfo/logback-dev
On 26.02.2010 12:46, Durchholz, Joachim wrote:
public String getTableName(String standardTableName) { if("logging_event".equals(standardTableName) { return standardTableName; } if("logging_event_property".equals(standardTableName) { return standardTableName; } if("logging_event_exception".equals(standardTableName) { return standardTableName; } throw new IllegalArgumentException(standardTableName + " is an unknown table name"); }
How about enum TableName {EVENT, EVENT_PROPERTY, EVENT_EXCEPTION} public String getTableName(TableName tableName) { switch (tableName) { case EVENT: return "logging_event"; case EVENT_PROPERTY: return "logging_event_property"; case EVENT_EXCEPTION: return "logging_event_exception"; } }
Using an enum has two advantages: - Compilers can (some will) complain if you forget a case in the switch - uses of TableName can be traced with cross-referencing tools
The disadvantage is that you can't extend the list. However, if that kind of flexibility is needed, I'm not sure that a separate resolver class is a good solution. It would need to provide a table name for whatever the internal workings of DBAppender require, so the coupling would be too tight. (If logback can use generic classes, then DBAppender<DBNameResolver> would be optimal IMHO.)
Using enums is a good idea. Both implementations of DBAppender the one in logback-classic and the one in logback-access depend on distinct table structures, but a table structure is assumed nonetheless. The DBNameResolver only decouples the table/col names from that assumed structure.
Regards, Jo
[ http://jira.qos.ch/browse/LBCLASSIC-188?page=com.atlassian.jira.plugin.syste... ] Rü commented on LBCLASSIC-188: ------------------------------ I think the "standardTableName" and "standardColumnName" should be enums instead!
Make table and column names overridable ---------------------------------------
Key: LBCLASSIC-188 URL: http://jira.qos.ch/browse/LBCLASSIC-188 Project: logback-classic Issue Type: Sub-task Components: appender Affects Versions: 0.9.18 Reporter: Ceki Gulcu Assignee: Logback dev list
To comply with local project rules, it can be helpful if the table and column names used by DBAppender could be overridden. The easiest way to solve this problem is for DBAppender to delegate the resolution of table names and columns to a different class, say DBNameResolver. Here is a possible interface: interface DBNameResolver { String getTableName(String standardTableName); String getColumnName(String standardColumnName); } The default implementation of DBNameResolver could be: class DefaulDBNameResolver implements DBNameResolver { public String getTableName(String standardTableName) { if("logging_event".equals(standardTableName) { return standardTableName; } if("logging_event_property".equals(standardTableName) { return standardTableName; } if("logging_event_exception".equals(standardTableName) { return standardTableName; } throw new IllegalArgumentException(standardTableName + " is an unknown table name"); } String getColumnName(String standardColumnName) { ... } } If a user wanted to use different names she would simply implement DBNameResolver according to her requirements. Any volunteers to implement this?
-- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.qos.ch/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
[ http://jira.qos.ch/browse/LBCLASSIC-188?page=com.atlassian.jira.plugin.syste... ] Ceki Gulcu commented on LBCLASSIC-188: -------------------------------------- Sure. Joachim Durchholz made the same suggestion.
Make table and column names overridable ---------------------------------------
Key: LBCLASSIC-188 URL: http://jira.qos.ch/browse/LBCLASSIC-188 Project: logback-classic Issue Type: Sub-task Components: appender Affects Versions: 0.9.18 Reporter: Ceki Gulcu Assignee: Logback dev list
To comply with local project rules, it can be helpful if the table and column names used by DBAppender could be overridden. The easiest way to solve this problem is for DBAppender to delegate the resolution of table names and columns to a different class, say DBNameResolver. Here is a possible interface: interface DBNameResolver { String getTableName(String standardTableName); String getColumnName(String standardColumnName); } The default implementation of DBNameResolver could be: class DefaulDBNameResolver implements DBNameResolver { public String getTableName(String standardTableName) { if("logging_event".equals(standardTableName) { return standardTableName; } if("logging_event_property".equals(standardTableName) { return standardTableName; } if("logging_event_exception".equals(standardTableName) { return standardTableName; } throw new IllegalArgumentException(standardTableName + " is an unknown table name"); } String getColumnName(String standardColumnName) { ... } } If a user wanted to use different names she would simply implement DBNameResolver according to her requirements. Any volunteers to implement this?
-- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.qos.ch/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
[ http://jira.qos.ch/browse/LBCLASSIC-188?page=com.atlassian.jira.plugin.syste... ] Ceki Gulcu commented on LBCLASSIC-188: -------------------------------------- I just pulled in git://github.com/nurkiewicz/logback.git. Cool. The tests using hsqldb pass which is nice. However, for other databases the tests are active on one particular machine called Orion which I currently can't access. I'll run the integration test later on. Coming back to the contribution itself, I think it would be preferable to reduce DBNameResolver to something like: public interface DBNameResolver { <E extends Enum<?>> String getTableName(E e); <E extends Enum<?>> String getColumnName(E e); } This way you can still invoke methods using enums (without converting to string). Moreover, a single enum could work for all column and table names. WDYT?
Make table and column names overridable ---------------------------------------
Key: LBCLASSIC-188 URL: http://jira.qos.ch/browse/LBCLASSIC-188 Project: logback-classic Issue Type: Sub-task Components: appender Affects Versions: 0.9.18 Reporter: Ceki Gulcu Assignee: Logback dev list
To comply with local project rules, it can be helpful if the table and column names used by DBAppender could be overridden. The easiest way to solve this problem is for DBAppender to delegate the resolution of table names and columns to a different class, say DBNameResolver. Here is a possible interface: interface DBNameResolver { String getTableName(String standardTableName); String getColumnName(String standardColumnName); } The default implementation of DBNameResolver could be: class DefaulDBNameResolver implements DBNameResolver { public String getTableName(String standardTableName) { if("logging_event".equals(standardTableName) { return standardTableName; } if("logging_event_property".equals(standardTableName) { return standardTableName; } if("logging_event_exception".equals(standardTableName) { return standardTableName; } throw new IllegalArgumentException(standardTableName + " is an unknown table name"); } String getColumnName(String standardColumnName) { ... } } If a user wanted to use different names she would simply implement DBNameResolver according to her requirements. Any volunteers to implement this?
-- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.qos.ch/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
[ http://jira.qos.ch/browse/LBCLASSIC-188?page=com.atlassian.jira.plugin.syste... ] Ceki Gulcu commented on LBCLASSIC-188: -------------------------------------- You can see the result of merging your contribution and my proposed simplifications to the DBNameResolver interface at: http://github.com/ceki/logback/tree/dbname The only downside I can see is that your version of CustomDBNameResolver was fully configurable via a config file while my version cannot be configured via an XML config file but it can be configured programmatically. If configuration via XML config file is required, then we can work something out.
Make table and column names overridable ---------------------------------------
Key: LBCLASSIC-188 URL: http://jira.qos.ch/browse/LBCLASSIC-188 Project: logback-classic Issue Type: Sub-task Components: appender Affects Versions: 0.9.18 Reporter: Ceki Gulcu Assignee: Logback dev list
To comply with local project rules, it can be helpful if the table and column names used by DBAppender could be overridden. The easiest way to solve this problem is for DBAppender to delegate the resolution of table names and columns to a different class, say DBNameResolver. Here is a possible interface: interface DBNameResolver { String getTableName(String standardTableName); String getColumnName(String standardColumnName); } The default implementation of DBNameResolver could be: class DefaulDBNameResolver implements DBNameResolver { public String getTableName(String standardTableName) { if("logging_event".equals(standardTableName) { return standardTableName; } if("logging_event_property".equals(standardTableName) { return standardTableName; } if("logging_event_exception".equals(standardTableName) { return standardTableName; } throw new IllegalArgumentException(standardTableName + " is an unknown table name"); } String getColumnName(String standardColumnName) { ... } } If a user wanted to use different names she would simply implement DBNameResolver according to her requirements. Any volunteers to implement this?
-- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.qos.ch/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
[ http://jira.qos.ch/browse/LBCLASSIC-188?page=com.atlassian.jira.plugin.syste... ] Ceki Gulcu edited comment on LBCLASSIC-188 at 3/19/10 12:05 AM: ---------------------------------------------------------------- You can see the result of merging your contribution and my proposed simplifications to the DBNameResolver interface at: http://github.com/ceki/logback/tree/dbname The only downside I can see is that your version of CustomDBNameResolver was fully configurable via a config file while my version cannot be configured via an XML config file but it can be configured programmatically. If configuration via XML config file is required, then we can work something out. By the way, since your contribution is on the substantial side, could you please sign the ICLA ? It can be found at: http://logback.qos.ch/cla.txt was (Author: noreply.ceki@qos.ch): You can see the result of merging your contribution and my proposed simplifications to the DBNameResolver interface at: http://github.com/ceki/logback/tree/dbname The only downside I can see is that your version of CustomDBNameResolver was fully configurable via a config file while my version cannot be configured via an XML config file but it can be configured programmatically. If configuration via XML config file is required, then we can work something out.
Make table and column names overridable ---------------------------------------
Key: LBCLASSIC-188 URL: http://jira.qos.ch/browse/LBCLASSIC-188 Project: logback-classic Issue Type: Sub-task Components: appender Affects Versions: 0.9.18 Reporter: Ceki Gulcu Assignee: Logback dev list
To comply with local project rules, it can be helpful if the table and column names used by DBAppender could be overridden. The easiest way to solve this problem is for DBAppender to delegate the resolution of table names and columns to a different class, say DBNameResolver. Here is a possible interface: interface DBNameResolver { String getTableName(String standardTableName); String getColumnName(String standardColumnName); } The default implementation of DBNameResolver could be: class DefaulDBNameResolver implements DBNameResolver { public String getTableName(String standardTableName) { if("logging_event".equals(standardTableName) { return standardTableName; } if("logging_event_property".equals(standardTableName) { return standardTableName; } if("logging_event_exception".equals(standardTableName) { return standardTableName; } throw new IllegalArgumentException(standardTableName + " is an unknown table name"); } String getColumnName(String standardColumnName) { ... } } If a user wanted to use different names she would simply implement DBNameResolver according to her requirements. Any volunteers to implement this?
-- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.qos.ch/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
[ http://jira.qos.ch/browse/LBCLASSIC-188?page=com.atlassian.jira.plugin.syste... ] Ceki Gulcu commented on LBCLASSIC-188: -------------------------------------- Got the CLA. Thank you. Coming back to the issue of configuring CustomDBNameResolver via XML, one very simple table and column naming strategy would be to add a prefix (or a suffix) to the standard name. So a CustomDBNameResolver could take a few parameters such as tableNamePrefix, tableNameSuffix, columnNamePrefix, and columnNameSuffix which is trivially easy for joran to deal with.
Make table and column names overridable ---------------------------------------
Key: LBCLASSIC-188 URL: http://jira.qos.ch/browse/LBCLASSIC-188 Project: logback-classic Issue Type: Sub-task Components: appender Affects Versions: 0.9.18 Reporter: Ceki Gulcu Assignee: Logback dev list
To comply with local project rules, it can be helpful if the table and column names used by DBAppender could be overridden. The easiest way to solve this problem is for DBAppender to delegate the resolution of table names and columns to a different class, say DBNameResolver. Here is a possible interface: interface DBNameResolver { String getTableName(String standardTableName); String getColumnName(String standardColumnName); } The default implementation of DBNameResolver could be: class DefaulDBNameResolver implements DBNameResolver { public String getTableName(String standardTableName) { if("logging_event".equals(standardTableName) { return standardTableName; } if("logging_event_property".equals(standardTableName) { return standardTableName; } if("logging_event_exception".equals(standardTableName) { return standardTableName; } throw new IllegalArgumentException(standardTableName + " is an unknown table name"); } String getColumnName(String standardColumnName) { ... } } If a user wanted to use different names she would simply implement DBNameResolver according to her requirements. Any volunteers to implement this?
-- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.qos.ch/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
[ http://jira.qos.ch/browse/LBCLASSIC-188?page=com.atlassian.jira.plugin.syste... ] Ceki Gulcu edited comment on LBCLASSIC-188 at 3/19/10 1:43 PM: --------------------------------------------------------------- Got the CLA. Thank you. (I can now merge the dbname branch into master.) Coming back to the issue of configuring CustomDBNameResolver via XML, one very simple table and column naming strategy would be to add a prefix (or a suffix) to the standard name. So a CustomDBNameResolver could take a few parameters such as tableNamePrefix, tableNameSuffix, columnNamePrefix, and columnNameSuffix which is trivially easy for joran to deal with. was (Author: noreply.ceki@qos.ch): Got the CLA. Thank you. Coming back to the issue of configuring CustomDBNameResolver via XML, one very simple table and column naming strategy would be to add a prefix (or a suffix) to the standard name. So a CustomDBNameResolver could take a few parameters such as tableNamePrefix, tableNameSuffix, columnNamePrefix, and columnNameSuffix which is trivially easy for joran to deal with.
Make table and column names overridable ---------------------------------------
Key: LBCLASSIC-188 URL: http://jira.qos.ch/browse/LBCLASSIC-188 Project: logback-classic Issue Type: Sub-task Components: appender Affects Versions: 0.9.18 Reporter: Ceki Gulcu Assignee: Logback dev list
To comply with local project rules, it can be helpful if the table and column names used by DBAppender could be overridden. The easiest way to solve this problem is for DBAppender to delegate the resolution of table names and columns to a different class, say DBNameResolver. Here is a possible interface: interface DBNameResolver { String getTableName(String standardTableName); String getColumnName(String standardColumnName); } The default implementation of DBNameResolver could be: class DefaulDBNameResolver implements DBNameResolver { public String getTableName(String standardTableName) { if("logging_event".equals(standardTableName) { return standardTableName; } if("logging_event_property".equals(standardTableName) { return standardTableName; } if("logging_event_exception".equals(standardTableName) { return standardTableName; } throw new IllegalArgumentException(standardTableName + " is an unknown table name"); } String getColumnName(String standardColumnName) { ... } } If a user wanted to use different names she would simply implement DBNameResolver according to her requirements. Any volunteers to implement this?
-- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.qos.ch/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
[ http://jira.qos.ch/browse/LBCLASSIC-188?page=com.atlassian.jira.plugin.syste... ] Tomasz Nurkiewicz commented on LBCLASSIC-188: --------------------------------------------- I am not actually convinced to your version of DBNameResolver. You can now pass virtually any enum to the resolver (for instance java.util.concurrent.TimeUnit) instead of table or column name enum. Also the interface seems very complicated with "<E extends Enum<?>>" construct. Maybe I don't get the idea right? Second thing is putting all column names in a single enum. It has the advantage that adding new tables to DbAppender won't have such a big impact on code (no new classes or methods). But when number of columns increase, single enum containing all columns in all tables would very likely to grow enormously. Also, each value in this enum should have table name prefix to avoid conflicts, which is very awkward. Also I find the ability to change particular column names directly from logback.xml being very important - that's why some many setters in my version of CustomDbNameAppender. Of course typically table/column, prefix/suffix customization is only needed, so I added SimpleDBNameResolver as suggested. I believe that majority of users would like to change single table/column name - and changing this via logback.xml is much easier and straightforward rather than implementing some logback-specific interface (please note that this tieds the application with Logback API). In the meantime I pushed all my changes to http://github.com/nurkiewicz/logback with my original interface. Since the API is not yet clarified (see above), I left it without Javadocs yet. Also please note that @since 0.9.19 tag is absolutely needed. I haven't tested how Joran handles new DbAppender configuration, but I assume it works.
Make table and column names overridable ---------------------------------------
Key: LBCLASSIC-188 URL: http://jira.qos.ch/browse/LBCLASSIC-188 Project: logback-classic Issue Type: Sub-task Components: appender Affects Versions: 0.9.18 Reporter: Ceki Gulcu Assignee: Logback dev list
To comply with local project rules, it can be helpful if the table and column names used by DBAppender could be overridden. The easiest way to solve this problem is for DBAppender to delegate the resolution of table names and columns to a different class, say DBNameResolver. Here is a possible interface: interface DBNameResolver { String getTableName(String standardTableName); String getColumnName(String standardColumnName); } The default implementation of DBNameResolver could be: class DefaulDBNameResolver implements DBNameResolver { public String getTableName(String standardTableName) { if("logging_event".equals(standardTableName) { return standardTableName; } if("logging_event_property".equals(standardTableName) { return standardTableName; } if("logging_event_exception".equals(standardTableName) { return standardTableName; } throw new IllegalArgumentException(standardTableName + " is an unknown table name"); } String getColumnName(String standardColumnName) { ... } } If a user wanted to use different names she would simply implement DBNameResolver according to her requirements. Any volunteers to implement this?
-- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.qos.ch/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
[ http://jira.qos.ch/browse/LBCLASSIC-188?page=com.atlassian.jira.plugin.syste... ] Ceki Gulcu commented on LBCLASSIC-188: --------------------------------------
I am not actually convinced to your version of DBNameResolver. You can now pass virtually any enum to the resolver (for instance java.util.concurrent.TimeUnit) instead of table or column name enum. Also the interface seems very complicated with "<E extends Enum<?>>" construct. Maybe I don't get the idea right?
I think that since the enums are passed to the resolver from within logback, the risk of using the wrong enum type is very low.
Second thing is putting all column names in a single enum. It has the advantage that adding new tables to DbAppender won't have such a big impact on code (no new classes or methods). But when number of columns increase, single enum containing all columns in all tables would very likely to grow enormously. Also, each value in this enum should have table name prefix to avoid conflicts, which is very awkward.
The number of columns and tables cannot increase freely. The table structure is likely to change very slowly if at all. Even if the same column name is used in different tables, collisions are not an issue. If column name 'A' exists both in table X and table Y, the column name 'A' is correct for both tables (by definion), i.e. collisions are not a problem.
Also I find the ability to change particular column names directly from logback.xml being very important - that's why some many setters in my version of CustomDbNameAppender. Of course typically table/column, prefix/suffix customization is only needed, so I added SimpleDBNameResolver as suggested. I believe that majority of users would like to change single table/column name - and changing this via logback.xml is much easier and straightforward rather than implementing some logback-specific interface (please note that this tieds the application with Logback API).
Adding support for map like components in joran would resolve this question for good. However, I already mentioned a possible naming strategy based on prefix/suffixes without requiring enhancements to Joran.
Make table and column names overridable ---------------------------------------
Key: LBCLASSIC-188 URL: http://jira.qos.ch/browse/LBCLASSIC-188 Project: logback-classic Issue Type: Sub-task Components: appender Affects Versions: 0.9.18 Reporter: Ceki Gulcu Assignee: Logback dev list
To comply with local project rules, it can be helpful if the table and column names used by DBAppender could be overridden. The easiest way to solve this problem is for DBAppender to delegate the resolution of table names and columns to a different class, say DBNameResolver. Here is a possible interface: interface DBNameResolver { String getTableName(String standardTableName); String getColumnName(String standardColumnName); } The default implementation of DBNameResolver could be: class DefaulDBNameResolver implements DBNameResolver { public String getTableName(String standardTableName) { if("logging_event".equals(standardTableName) { return standardTableName; } if("logging_event_property".equals(standardTableName) { return standardTableName; } if("logging_event_exception".equals(standardTableName) { return standardTableName; } throw new IllegalArgumentException(standardTableName + " is an unknown table name"); } String getColumnName(String standardColumnName) { ... } } If a user wanted to use different names she would simply implement DBNameResolver according to her requirements. Any volunteers to implement this?
-- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.qos.ch/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
[ http://jira.qos.ch/browse/LBCLASSIC-188?page=com.atlassian.jira.plugin.syste... ] Tomasz Nurkiewicz commented on LBCLASSIC-188: --------------------------------------------- OK, I merged ceki/logback master branch with my fork and refactored the code according to Your suggestions. In my fork you will also find: * SimpleDBNameResolver resolver that adds simple prefix/suffix capabilities * Bugfix to CustomDBNameResolver - the implementation uses Map<String, String>, but the map is browsed using enum keys, not enum string representations. I added many unit tests, one of them revealed this bug. * Few lines of Javadocs Go ahead if You would like to merge my contribution into 0.9.20.
Make table and column names overridable ---------------------------------------
Key: LBCLASSIC-188 URL: http://jira.qos.ch/browse/LBCLASSIC-188 Project: logback-classic Issue Type: Sub-task Components: appender Affects Versions: 0.9.18 Reporter: Ceki Gulcu Assignee: Logback dev list
To comply with local project rules, it can be helpful if the table and column names used by DBAppender could be overridden. The easiest way to solve this problem is for DBAppender to delegate the resolution of table names and columns to a different class, say DBNameResolver. Here is a possible interface: interface DBNameResolver { String getTableName(String standardTableName); String getColumnName(String standardColumnName); } The default implementation of DBNameResolver could be: class DefaulDBNameResolver implements DBNameResolver { public String getTableName(String standardTableName) { if("logging_event".equals(standardTableName) { return standardTableName; } if("logging_event_property".equals(standardTableName) { return standardTableName; } if("logging_event_exception".equals(standardTableName) { return standardTableName; } throw new IllegalArgumentException(standardTableName + " is an unknown table name"); } String getColumnName(String standardColumnName) { ... } } If a user wanted to use different names she would simply implement DBNameResolver according to her requirements. Any volunteers to implement this?
-- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.qos.ch/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
[ http://jira.qos.ch/browse/LBCLASSIC-188?page=com.atlassian.jira.plugin.syste... ] Ceki Gulcu resolved LBCLASSIC-188. ---------------------------------- Fix Version/s: 0.9.20 Resolution: Fixed Merged with your repo in http://github.com/ceki/logback/commit/a3423c5ec6e11b19562
Make table and column names overridable ---------------------------------------
Key: LBCLASSIC-188 URL: http://jira.qos.ch/browse/LBCLASSIC-188 Project: logback-classic Issue Type: Sub-task Components: appender Affects Versions: 0.9.18 Reporter: Ceki Gulcu Assignee: Logback dev list Fix For: 0.9.20
To comply with local project rules, it can be helpful if the table and column names used by DBAppender could be overridden. The easiest way to solve this problem is for DBAppender to delegate the resolution of table names and columns to a different class, say DBNameResolver. Here is a possible interface: interface DBNameResolver { String getTableName(String standardTableName); String getColumnName(String standardColumnName); } The default implementation of DBNameResolver could be: class DefaulDBNameResolver implements DBNameResolver { public String getTableName(String standardTableName) { if("logging_event".equals(standardTableName) { return standardTableName; } if("logging_event_property".equals(standardTableName) { return standardTableName; } if("logging_event_exception".equals(standardTableName) { return standardTableName; } throw new IllegalArgumentException(standardTableName + " is an unknown table name"); } String getColumnName(String standardColumnName) { ... } } If a user wanted to use different names she would simply implement DBNameResolver according to her requirements. Any volunteers to implement this?
-- This message is automatically generated by JIRA. - If you think it was sent incorrectly contact one of the administrators: http://jira.qos.ch/secure/Administrators.jspa - For more information on JIRA, see: http://www.atlassian.com/software/jira
participants (5)
-
Ceki Gulcu -
Ceki Gulcu (JIRA) -
Durchholz, Joachim -
Rü (JIRA)
-
Tomasz Nurkiewicz (JIRA)