ID 2860742 was a bad bugfix, please undo it!
Brought to you by:
ickzon
Menu
▾
▴
#607 ID 2860742 was a bad bugfix, please undo it!
Bugfix ID 2860742 In My Humble Opinion is a bad bugfix for following 2 reasons:
- it makes JTDS driver have a different behavior on byte-handing than the SQLServerDriver
- it allows to write byte values which afterwards cannot be readed anymore ( error.convert.numericoverflow is thrown! )
The root dilemma stays in the fact that java.lang.byte is signed [-128;+127] whilst its correspondent database type TINYINT is unsigned [0;255].
Anyway, with SQLServerDriver version 2.0 and JDTS 1.2.2 it was possible to do:
- define in java byte number = -10;
- write this byte to database
- SQLServerDriver stores TINYINT value 246
- read value from database
- returned byte value == -10
Now with JDTS 1.2.5:
- define in java byte number = -10;
- write this byte to database, no problem
- SQLServerDriver stores TINYINT value 246
- reread value from database
- java.sql.SQLException: Numeric overflow in conversion of value 246 to type TINYINT.
I propose to
1. either allow only byte values from 0 to 127 for read AND WRITE operation, and document that JDTS differs in that point to SQLServerDriver behavior,
or
2. rollback bugfix ID 2860742
(personally I prefer the ladder)
best regards
Guenther Demetz
Hi Guenther,
thank you for reporting that issue. Unfortunately, this is a quite complicated topic.
As you already pointed out, the main problem is that columns of type TINYINT cannot be handled appropriately via setByte()/getByte() due to the different range of values. One approach is to just map the value range of byte to the value range of unsigned TINYINT (as done in jTDS 1.2.2 and SQLServerDriver). Nevertheless, the approach has some drawbacks since it actually changes all negative values. Should one really expect -10 to be stored as 246 in the TINYINT column, just because setByte() is used? This way you simply store garbage in the database that can only be read back to its original value via getByte(). As soon as you start mixing different getter/setter methods you would end up in total chaos (not to mention that a DB could be accessed by different applications). One simply cannot tell apart the converted values from the "real" ones. And why should setByte() store -10 as 246 in an INT column? And why should a really positive value stored in an INT column be read back into a negative Java value when using getByte()? Why should a Java application even bother with implicit data conversion, at all? If I store a Java byte I would expect that exact value to end up in the DB if possible, or to get an error if not.
The mentioned approach of simply mapping value ranges between Java byte and unsigned TINYINT has been introduced in the jTDS driver years ago, long before I joined the project, but I don't think we should stay with it. The API-docs state that setByte()/getByte() will convert the given value to a TINYINT in the DBs native format. I could imagine that the ambiguity of the JDBC specs has lead to the perception that a value transformation would be required here. But I highly doubt that such conversion was meant to actually change the value. As stated above, such approach simply doesn't fit into the Java world. The JDBC standard is meant to abstract from a concrete DBMS so why should a programmer care about (and even expect) "random" data conversion?
All that being said, I definitely wouldn't return to the former state of implicit data conversion as you proposed in 2., but I'd neither directly limit setByte()/getByte() to values from 0 to 127 as you proposed in 1. because negative values are okay when storing values in columns of other types.
I'd expect the following behavior:
1. setByte()/getByte() can be used for storing values from -128 to 127 in all column types with appropriate value range
2. setByte() can be used for storing values from 0 to 127 in TINYINT columns and throws a DataTruncation for negative values
3. getByte() can be used for storing values from 0 to 127 in TINYINT columns and throws a DataTruncation for values 128 to 255
4. setShort()/getShort() and "above" can be used for storing/reading values 0 to 255 in TINYINT columns
That way all the setter/getter methods would behave the same in that they simply prevent you from reading values that are not in the corresponding Java-type's value range and leaving data truncation problems for write operations up to the DBMS.
Conclusion1:
Avoid using setByte()/getByte(). They are simply not appropriate for handling all possible TINYINT values.
Conclusion2:
When implementing the overflow check I missed the fact, that setByte() still performs an implicit data transformation. I'll have to fix it, but that will further break backward compatibility with jTDS 1.2.2.
Cheers,
momo
That's OK for me, provided that this behavior change will be documented appropriately.
I completely agree with you.
Since I'm using Hibernate, for me it means: never use java.lang.byte as persistent property type.
Generally for Hibernate Users it will mean: only positive java.lang.byte values can be transported
best regards
Guenther
I'm glad you understand what I was trying to say - I just recognized I had some copy&paste errors in my description of the expected behavior ;-)
Just for the recods - I meant:
1. setByte()/getByte() can be used for storing/reading values from -128 to 127 in all column types with appropriate value range
2. setByte() can be used for storing values from 0 to 127 in TINYINT columns and throws a DataTruncation for negative values
3. getByte() can be used for reading values from 0 to 127 from TINYINT columns and throws an overflow error for values 128 to 255
4. setShort()/getShort() and "above" can be used for storing/reading values 0 to 255 in/from TINYINT columns
I'm not really familiar with Hibernate, but isn't it possible to specify the DB data type used for a Java type? I cannot imagine TINYINT to be hard-coded in Hibernate, so using SMALLINT should be a sufficient mapping for byte values.
Cheers,
momo
I don't agree completely with you in that point, as there's also another point of view:
Actually jTDS 1.2.2 and SQLServer do not convert any data
as the 8 bits are transported without modification:
(byte)-10 in java and TinyInt 246 on db is stored as 11110110 on both sides in binary format.
-10 and 246 could even be interpreted as just 2 different presentations the same stored value.
As byte is usually not used as a scalar value,
users will hardly use such columns in where-conditions or aggregate functions.
In fact I did not find any posting on the Internet complaining about this problem (except skotinin).
So normally users are satisfied by getting the same value back again from db, which they stored before.
Anyway for me your proposal is OK.
According hibernate docu, java.lang.Byte and byte are always mapped to TINYINT !
(But you can define custom types in Hibernate)
Okay, there is no binary difference, but values just get interpreted differently.
The whole problem comes down to the fact, that the JDBC specs do not specify exactly what one can expect when using getByte()/setByte(). I clearly see the point of just using that methods as if Java-byte was unsigned, but that still leaves the problem of detecting "misuse". Unfortunately there is no way of detecting whether the user really meant 246 or if he was trying to insert the invalid -10 he provided (maybe even downcast from an int, so its meaning would be obvious). And since we still are in the Java world, I'd prefer using the Java types in their natural meaning.
Anyway, I'll have to do some further research on that topic. TINYINT being hardwired in Hibernate is a strong argument for reverting back to the former behavior (even if no one complained about not being able to handle negative values until today).
Cheers,
momo
Hi Guenther & momo!
but:
I want get byte value from SMALLINT column without a number overflow
I totally agree with momo solution (1,2,3,4). For MSSQL TINYINT column this is right resolve. (P.S. thank momo for resolve getByte(-1))
Guenther wrote:
quote
YES! Normally users use getInt/setInt and happy :-( But I want use getByte for check constant values (e.g. -1,0,1)
Best regards,
Andrey
and thank very much for the great jTDS
Hi Momo,
after conversation with skotinin, I came to the conclusion that all he wants is:
getByte("a_smallint_column") should not throw exception if the database type is SMALLINT
and if the stored value is -1.
So I propose the driver on getByte() should :
- accept all SMALLINT values within the byte range -128;+127
- have a extra handling for TINYINT values
I guess the byte type generally is more used as 8bit-array than as scalar value,
therefore it would be very pity to reduce TINYINT's usage to 7 bits.
I propose to introduce a new property which allows the use to choose between a strict TINYINT check
(only values between 0 and 127) or a not strict TINYINT check (values between 0 and 255).
best regards
Guenther
Hi Guenther, hi Andrey,
thank you for all your input.
[quote]have a extra handling for TINYINT values[/quote]
This is no option at all. The driver doesn't (and, under certain circumstances, even cannot) know the column/parameter type a value is read from/written to. In any case, this solution would heavily impact performance because of additional database round-trips for gathering type information. I wouldn't consider that a viable solution, anyway. The way getByte()/setByte() interpret byte values has to be consistent, at the very least.
Furthermore, I found the following in the Java JDBC guide:
[quote]
The recommended Java mapping for the JDBC TINYINT type is as either a Java byte or a Java short. The 8-bit Java byte type represents a signed value from -128 to 127, so it may not always be appropriate for larger TINYINT values, whereas the 16-bit Java short will always be able to hold all TINYINT values.
[/quote]
To me this looks quite unambiguous. Why should they explicitly mention that a byte-mapping may be inappropriate for the 8-bit TINYINT if they ever considered getByte()/setByte to interpret Java bytes as unsigned values?
So, to follow this idea, I'll have to adapt byte handling the way I described below.
If neccessary, we could even define an additional property to allow the users to choose between current and previous behaviour (would affect implicit data conversion and range checks), but I really dislike the idea of adding more and more properties for each and every "bagatelle" (that just would force jTDS to violate JDBC specs here, to make it even worse).
Cheers,
momo
Hi Momo,
[quote]
I'll have to adapt byte handling the way I described below.
...
3. getByte() can be used for storing values from 0 to 127 in TINYINT columns and throws a DataTruncation for values 128 to 255
[/quote]
on the other hand JDBC-specs clearly states that TINYINT type has 8-bits.
Reducing it's usage to 7-bits would therefore also violate the JDBC specs IMHO.
[quote]
TINYINT being hardwired in Hibernate is a strong argument for reverting back to the
former behavior (even if no one complained about not being able to handle
negative values until today).[/quote]
Remember that version DTS 1.2.5 is of relative young age.
Be aware that It will only be a question of time that other (hibernate) users will run into same problem.
Hi Guenther,
I cannot agree with that argument. Yes, the specs define TINYINT to be an 8-bit value. But they also state that a Java byte may not be sufficient to handle all TINYINT values. Please don't mix up Java types (and their corresponding setters/getters) with the JDBC data types. JDBC defines its own data types on the basis of the SQL standard and then suggests a mapping to native Java types. The only question was whether the JDBC standard requires "misinterpretation" of a Java byte to allow a better mapping to TINYINT (and IMHO that question has been answered).
On the other hand, I definitely see the problem with Hibernate. But maybe one should file a bug report there, instead of trying to keep jTDS "compatible". As I understand the JDBC specs, Hibernate shouldn't have used a TINYINT mapping in the first place because of its limitations.
Cheers,
momo