On Oct 11, 2013, at 10:29 AM, Gary Gregory wrote:

> On Fri, Oct 11, 2013 at 11:10 AM, Nick Williams
> <nicho...@nicholaswilliams.net> wrote:
>> (Foreword: Apologies for my scarcity of late. I'm a month behind on my book, 
>> mostly due to my wasted work on getCallerClass for Java 8, and I still have 
>> $work to contend with.)
>> 
>> Agreed that the unicode stuff is a problem with a non-compliant driver. 
>> There's nothing we can do about that per se. Other, compliant drivers will 
>> require unicode columns to use setNString, etc. Unicode is the direction 
>> everything is headed (indeed, most things are unicode by default now, such 
>> as MySQL), so we need to leave unicode as the default.
>> 
>> The data types are an interesting problem. We can't just support any Type 
>> constant. Here's why:
>> 
>> The way you use the Types constant is to call setObject. The problem with 
>> setObject is that the actual value passed to it must match the Type constant 
>> passed to it. So, you can't pass a String into setObject and use 
>> Types.INTEGER--that will result in an error for most drivers (though some 
>> are more forgiving).
>> 
>> The way the whole JDBCAppender works is to use the pattern layout to 
>> determine column value. This gives the end user ultimate flexibility--they 
>> can use one field or multiple fields from a LogEvent for each column, format 
>> them different ways, etc. The pattern layout results in a String, so we need 
>> to be aware of how to convert that String to the Type the user has 
>> specified. That's a lot of work, and may not be possible for all Types the 
>> user can specify.
>> 
>> However, I _do_ understand how our users could often need to have integer 
>> columns, so I think it's worth adding a special case for integer column 
>> types. Of course, integer columns could be either INTEGER or BIGINT in the 
>> most common cases, so I'm not completely sure the best way to proceed with 
>> that.
>> 
>> Thoughts?
> 
> I was thinking of using Types values to then have a switch on the type
> value in order to call setInt vs. setString vs. setXXX
> 
> The string could be anything but using the names Types constants gives
> us a known vocabulary of sorts.

But like I pointed out, we can't support a majority of the Types. What we CAN 
do, however, is remove the "unicode" attribute and replace it with a "type" 
attribute that initially supports values VARCHAR, NVARCHAR, INTEGER, and BIGINT 
and defaults to NVARCHAR. These names are equivalent to the Types constants, 
but the semantic difference is we would only document these four values instead 
of documenting "use a constant from Types." This would give us the flexibility 
to support further types in the future without confusing the user by having to 
list the 33 constants that they CAN'T use. What about that?

Nick

> 
> Gary
> 
>> 
>> Nick
>> 
>> On Oct 9, 2013, at 5:06 PM, Gary Gregory wrote:
>> 
>>> I'd like Nick's feedback on this, so let's see what he has to say.
>>> 
>>> WRT process, you should use JIRA to create feature requests and bug
>>> reports. You can than attach SVN a unified diff file to a JIRA. It is
>>> always nice when I see unit tests in the patches too :)
>>> 
>>> IMO, about the Unicode stuff, the modern current APIs should be the
>>> default, so I would not change it. Remember that you are dealing with
>>> a non-compliant JDBC driver here, if the driver claims to be JDBC 4
>>> but does not implement all the APIs, then you know who to blame ;)
>>> 
>>> Gary
>>> 
>>> On Wed, Oct 9, 2013 at 5:55 PM, Tihomir Meščić <tihomir.mes...@gmail.com> 
>>> wrote:
>>>> Yes, something like that would be great, since this is not only an issue
>>>> with the Integer type, other data types
>>>> are also not supported. I've made my own implementation of the JDBC 
>>>> appender
>>>> that does exactly that. I don't
>>>> know what's the policy of the project, but I'll be happy to contribute the
>>>> code.
>>>> 
>>>> There is one more very nasty issue with the JDBC appender - the appender
>>>> will call setNString() method
>>>> on the PreparedStatement by default (this is the unicode version, available
>>>> in JDBC version 4). When using PostgreSQL
>>>> database with the latest JDBC driver this will cause an error when
>>>> inserting. It looked like a bug in log4j but then
>>>> (after a couple of hours of digging through log4j code) it turned out that
>>>> the JDBC driver for Postgres only partially implements JDBC
>>>> version 4 (it does not implement the setNString method) and nobody knows
>>>> when it'll implement the spec completely.
>>>> The fix was to set <Column isUnicode="false" in the appender configuration 
>>>> -
>>>> that way, the setString() method on the PreparedStatement
>>>> would be called.
>>>> IMHO, a more conservative, backwards compatible solution (non-Unicode)
>>>> should be the default...
>>>> 
>>>> Kind regards,
>>>> Tihomir
>>>> 
>>>> 
>>>> 
>>>> 2013/10/9 Gary Gregory <garydgreg...@gmail.com>
>>>>> 
>>>>> On Wed, Oct 9, 2013 at 11:24 AM, Tihomir Meščić
>>>>> <tihomir.mes...@gmail.com> wrote:
>>>>>> Ok, my database (Postgresql) has a table (log_entries) that's used for
>>>>>> logging purposes.
>>>>>> One of the attributes is of type INTEGER.
>>>>>> In the new version of the app we are migrating to log4j version 2
>>>>>> (beta9)
>>>>>> and we want to
>>>>>> use the JDBCAppender that bundled with log4j. The INTEGER attribute is
>>>>>> something specific for our
>>>>>> application and we are using ThreadContext (MDC) to set the value of the
>>>>>> parameter.
>>>>>> 
>>>>>> Currently, log4j provides no support for integer type attributes in the
>>>>>> Column element of the JDBC appender
>>>>>> configuration (the only things supported are string (default), timestamp
>>>>>> -
>>>>>> isEventTimestamp flag and Clob - isCLob flag).
>>>>>> 
>>>>>> When using the default settings in the Column element of the JDBC
>>>>>> appender
>>>>>> log4j will create a prepared statement
>>>>>> and try to set the value using the Statement.setString() method. Of
>>>>>> course,
>>>>>> the JDBC driver throws an excpetion:
>>>>>> 
>>>>>> Caused by: org.postgresql.util.PSQLException: ERROR: column "mn_type_d"
>>>>>> is
>>>>>> of type integer but expression is of type character varying
>>>>>> Hint: You will need to rewrite or cast the expression.
>>>>>> 
>>>>>> 
>>>>>> My appender:
>>>>>> 
>>>>>>   <JDBC name="jdbcAppender" tableName="log_entries">
>>>>>>     <DriverManager url="jdbc:postgresql://10.28.10.32:5432/xxx"
>>>>>> username="xxx" password="xxx" />
>>>>>>     <Column name="log_entries_id"
>>>>>> literal="nextval('hibernate_sequence')"
>>>>>> />
>>>>>> 
>>>>>>     .....
>>>>>>     <Column name="message" isUnicode="false" pattern="%message" />
>>>>>> 
>>>>>>     <Column name="mn_type_d" isUnicode="false" pattern="%X{mn_type_d}"
>>>>>> />
>>>>>> <-- this is of type integer in the DB but LOG4J tries to insert it as a
>>>>>> String  -->
>>>>>>   </JDBC>
>>>>> 
>>>>> So maybe we need to be able to say:
>>>>> 
>>>>> <Column name="mn_type_d" pattern="%X{mn_type_d}" type="INTEGER" />
>>>>> 
>>>>> Where type is a name from java.sql.Types.
>>>>> 
>>>>> Nick? Thoughts?
>>>>> 
>>>>> Gary
>>>>> 
>>>>>> 
>>>>>> 
>>>>>> Kind regards,
>>>>>> Tihomir
>>>>>> 
>>>>>> 
>>>>>> 2013/10/9 Gary Gregory <garydgreg...@gmail.com>
>>>>>>> 
>>>>>>> Hi Tihomir,
>>>>>>> 
>>>>>>> Can you be more descriptive please with a practical example?
>>>>>>> 
>>>>>>> Gary
>>>>>>> 
>>>>>>> On Wed, Oct 9, 2013 at 10:43 AM, Tihomir Meščić
>>>>>>> <tihomir.mes...@gmail.com> wrote:
>>>>>>>> Hi everyone,
>>>>>>>> 
>>>>>>>> there is a missing feature in the JDBCAppender for log4j version 2,
>>>>>>>> it
>>>>>>>> does
>>>>>>>> not support data types
>>>>>>>> other than string, date and clob. So for example, if the table you
>>>>>>>> are
>>>>>>>> trying to log to has an Integer
>>>>>>>> column, there is way to force log4j to insert it. I ended up writing
>>>>>>>> my
>>>>>>>> own
>>>>>>>> JDBC appender. I think
>>>>>>>> that this feature is very important and it should definitively be
>>>>>>>> included
>>>>>>>> in the final version.
>>>>>>>> 
>>>>>>>> Log4j version: 2.0.0-beta9
>>>>>>>> 
>>>>>>>> Kind regards,
>>>>>>>> Tihomir
>>>>>>> 
>>>>>>> 
>>>>>>> 
>>>>>>> --
>>>>>>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
>>>>>>> Java Persistence with Hibernate, Second Edition
>>>>>>> JUnit in Action, Second Edition
>>>>>>> Spring Batch in Action
>>>>>>> Blog: http://garygregory.wordpress.com
>>>>>>> Home: http://garygregory.com/
>>>>>>> Tweet! http://twitter.com/GaryGregory
>>>>>>> 
>>>>>>> ---------------------------------------------------------------------
>>>>>>> To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org
>>>>>>> For additional commands, e-mail: log4j-dev-h...@logging.apache.org
>>>>>>> 
>>>>>> 
>>>>> 
>>>>> 
>>>>> 
>>>>> --
>>>>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
>>>>> Java Persistence with Hibernate, Second Edition
>>>>> JUnit in Action, Second Edition
>>>>> Spring Batch in Action
>>>>> Blog: http://garygregory.wordpress.com
>>>>> Home: http://garygregory.com/
>>>>> Tweet! http://twitter.com/GaryGregory
>>>>> 
>>>>> ---------------------------------------------------------------------
>>>>> To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org
>>>>> For additional commands, e-mail: log4j-dev-h...@logging.apache.org
>>>>> 
>>>> 
>>> 
>>> 
>>> 
>>> --
>>> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
>>> Java Persistence with Hibernate, Second Edition
>>> JUnit in Action, Second Edition
>>> Spring Batch in Action
>>> Blog: http://garygregory.wordpress.com
>>> Home: http://garygregory.com/
>>> Tweet! http://twitter.com/GaryGregory
>>> 
>>> ---------------------------------------------------------------------
>>> To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org
>>> For additional commands, e-mail: log4j-dev-h...@logging.apache.org
>>> 
>> 
> 
> 
> 
> -- 
> E-Mail: garydgreg...@gmail.com | ggreg...@apache.org
> Java Persistence with Hibernate, Second Edition
> JUnit in Action, Second Edition
> Spring Batch in Action
> Blog: http://garygregory.wordpress.com
> Home: http://garygregory.com/
> Tweet! http://twitter.com/GaryGregory
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: log4j-dev-unsubscr...@logging.apache.org
> For additional commands, e-mail: log4j-dev-h...@logging.apache.org
> 

Attachment: smime.p7s
Description: S/MIME cryptographic signature

Reply via email to