Eclipse Community Forums
Forum Search:

Search      Help    Register    Login    Home
Home » Eclipse Projects » scout » AbstractSqlStyle returns different data types in the same column
AbstractSqlStyle returns different data types in the same column [message #1108335] Fri, 13 September 2013 18:08 Go to next message
Ken Lee is currently offline Ken LeeFriend
Messages: 97
Registered: March 2012
Member
I'd like to discuss a subject regarding our AbstractSqlStyle implementation about binding data read from the DB to Java data types.
The reason for this discussion was bug 416905.

In summary, a Scout project used SQL.select() to fetch some data from the DB. The result was stored in a Java Object[][]. For each row a CompositeObject was created. These objects were later used in Set / List data structures which led to ClassCastExceptions.

Analysis:
The compareTo() method in org.eclipse.scout.commons.CompositeObject is implemented to compare array elements located at the same position to each other. For table rows used as values in the CompositeObject this means that values of a specific column are compared to each other.
The above mentioned ClassCastException occured e.g. when a value of type Long was compared to another value of type Double because the contract of Long.compareTo expects a parameter of the same type. Converting a Double value to Long leads to a ClassCastException.

After having a deeper look at the SqlService, we found out that the implementation of AbstractSqlStyle.readBind() processes the ResultSet row per row, inside a row each column is processed.
If the meta data for the column type is a general number, the value is converted into a BigDecimal type and then checked if the value contains further digits after the decimal point. If not, the value is represented as a Long type, otherwise a Double object is created.

 switch (type) {
 // General Number
 case Types.DECIMAL:
 case Types.NUMERIC: {
   BigDecimal bd = rs.getBigDecimal(jdbcBindIndex);
   if (bd != null) {
     if (bd.scale() == 0) {
       o = new Long(bd.longValue());
     }
     else {
       o = new Double(bd.doubleValue());
     }
   }
   break;
  }
  ...


In the Scout project the DB table was created with a column of type Number(15,5). If a value like 1.111 is read, a Double is created. However, if the number 1 is read, a Long will be created. As a consequence there are different data types inside a single column that cannot be compared to each other, thus leading to a ClassCastException when they are compared inside the CompositeObject.

The question now is if we should have the same data type for all values inside a single column or not. Assume we create / convert all values inside a column with the data type that has the highest necessary precision, the problem regarding ClassCastException would never arise. However, this conversion can only be done during a prefetching or in a postprocessing step of the result step which could be quite heavy.

So the current solutions I could think of are:

  • Don't do anything in AbstractSqlStyle. This implies that data rows cannot be compared inside a CompositeObject.
  • Provide Utilities to convert the Object[][] values after they are read from the DB. This means that the programmer has to call the conversion utility before comparing the rows.
  • Implement a prefetching phase in AbstractSqlStyle that finds the type with the highest precision (for numbers) in the result set first. After that the values can be created with the known types.
  • Implement a postprocessing phase in AbstractSqlStyle to convert the data types of a single column.
  • Change CompositeObject.compareTo to be able to compare different Number types (long 1 == double 1.00, e.g.)


What do you think about the solutions? Are there any other solutions?
The conversion needs to be done for Numbers and probably for Date / Timestamp types as well. Are there other data types which need to be considered?

[Updated on: Fri, 13 September 2013 18:09]

Report message to a moderator

Re: AbstractSqlStyle returns different data types in the same column [message #1108386 is a reply to message #1108335] Fri, 13 September 2013 19:45 Go to previous messageGo to next message
Ivan Motsch is currently offline Ivan MotschFriend
Messages: 100
Registered: March 2010
Senior Member
I vote for the first bullet:
Unless a higher level jpa is used such as hibernate or eclipse link, there is no way scout can reasonably detect the runtime type of a jdbc driver.
Note also that different databases handle NUMERIC differently. So a comparison of two database fetched rows should never be done.

A better solution would be that scout offers the possibility - similar to hibernates native SQL queries - that the caller can declare the column jdbc types.

Something like
SQL.selectTyped(queryString, new int[]{Types.BIGINT, Types.VARCHAR, ...}, bindMap)

where Types=java.sql.Types and queryString is a native SQL select statement.

That way it is absolutely clear what result values are to be fetched by the AbstractSqlStyle.
This would not fix the reported bug automatically but give the developer a decent chance to tell scout what types should be returned without guessing fractional numbers.

[Updated on: Fri, 13 September 2013 19:47]

Report message to a moderator

Re: AbstractSqlStyle returns different data types in the same column [message #1109490 is a reply to message #1108335] Sun, 15 September 2013 13:51 Go to previous messageGo to next message
Lukas Huser is currently offline Lukas HuserFriend
Messages: 42
Registered: March 2010
Member
Hi Ken

There is another forum post and corresponding bugzilla ticket on the same topic. Here the ClassCastException occurs when using MatrixUtility.sort().
http://www.eclipse.org/forums/index.php/m/968564/#msg_968564
https://bugs.eclipse.org/bugs/show_bug.cgi?id=394984

The above bugzilla ticket suggests to determine the appropriate Java type (Long, Double) based on the column meta data.
I guess this could be done through ResultSetMetaData.getScale(). At least the JavaDoc sounds promising, whether it works for actual JDBC implementations for different DB engines is probably another question.
It is highly desireable to have consistent data types within a single column, as it helps avoiding many follow-up problems. If it can be achieved by evaluating the provided meta data, this would certainly be my preferred solution.

Quote:
Change CompositeObject.compareTo to be able to compare different Number types (long 1 == double 1.00, e.g.)

I would strictly vote against such a change. CompositeObject can be used in a much broader context than comparing database rows, in fact, it is not even particularly suited for this purpose, as you need to select the columns in the desired sort order.
Following the principle of least surprise, the "expected" behavior of a composite object is to simply delegate to the compareTo method of its components (as it does today).

However, I agree with Marco Bigolins comment in Bug 416905: The equals method should not delegate to the compareTo method. Instead it should also delegate to the equals method of its components. Reasons are: compareTo is allowed to throw ClassCastException and NullPointerException, while equals must not. compareTo and equals are not necessarily consistent with each other for all component objects, which can lead to unexpected results.
Re: AbstractSqlStyle returns different data types in the same column [message #1110015 is a reply to message #1108335] Mon, 16 September 2013 08:38 Go to previous messageGo to next message
Matthias Villiger is currently offline Matthias VilligerFriend
Messages: 83
Registered: September 2011
Member
+1 for Ivans solution
Re: AbstractSqlStyle returns different data types in the same column [message #1110036 is a reply to message #1110015] Mon, 16 September 2013 09:17 Go to previous messageGo to next message
André Wegmüller is currently offline André WegmüllerFriend
Messages: 8
Registered: September 2012
Junior Member
As Ken pointed out correctly the problem is that AbstractSqlStyle looks at each row in order to create a concrete Number instance. In my opinion the simplest solution would be to look at the meta-data of the result-set instead. An instance of ResultSetMetaData is already passed to the readBind() method, but it is never used there. In our project we use a modified version of AbstractSqlStyle which solves the Number instance problem as described in the Bugzilla bug and in the forum post:

      
      case Types.DECIMAL:
      case Types.NUMERIC: {
        BigDecimal bd = rs.getBigDecimal(jdbcBindIndex);
        if (meta.getScale(jdbcBindIndex) == 0) {
          if (bd != null) {
            o = bd.longValue();
          }
        }
        else {
          o = bd;
        }
        break;


JDBC by default returns a BigDecimal for numeric types with a fixed scale. The current AbstractSqlStyle impl. always transforms a BigDecimal into a Double, which is wrong in my opinion, since Double does not have a fixed scale but is a floating point number. Our change also deals with that issue and returns BigDecimal in favor of Double.

Thus: the current impl. always returns Long or Double, so a change as suggested above could possibly break existing application-code. Whatever the correct solution is, there should be a way to activate/configure the current behavior for applications built with older versions of Scout.

I'd rather not have to set data-types explicitly for each query. Today Scout already does some type-guessing, which is convenient and works quite good (most of the time). So in my opinion the type-guessing should be improved by looking at the metadata instead of the row.
Re: AbstractSqlStyle returns different data types in the same column [message #1111001 is a reply to message #1109490] Tue, 17 September 2013 16:23 Go to previous messageGo to next message
Ken Lee is currently offline Ken LeeFriend
Messages: 97
Registered: March 2012
Member
Lukas Huser wrote on Sun, 15 September 2013 09:51
There is another forum post and corresponding bugzilla ticket on the same topic. Here the ClassCastException occurs when using MatrixUtility.sort().
http://www.eclipse.org/forums/index.php/m/968564/#msg_968564
https://bugs.eclipse.org/bugs/show_bug.cgi?id=394984


Thanks for the links, Lukas. Yes, the other forum post and bugzilla does also target the same issue.
Using the meta data of the resultset was also my first advice. However, I was told that the meta data were not always reliable depending on the underlying DB. E.g. it doesn't work reliably with an Oracle 11 DB. So using meta data is not an option.

Quote:

I would strictly vote against such a change. CompositeObject can be used in a much broader context than comparing database rows, in fact, it is not even particularly suited for this purpose, as you need to select the columns in the desired sort order.
Following the principle of least surprise, the "expected" behavior of a composite object is to simply delegate to the compareTo method of its components (as it does today).


I agree.

Quote:

However, I agree with Marco Bigolins comment in Bug 416905: The equals method should not delegate to the compareTo method. Instead it should also delegate to the equals method of its components. Reasons are: compareTo is allowed to throw ClassCastException and NullPointerException, while equals must not. compareTo and equals are not necessarily consistent with each other for all component objects, which can lead to unexpected results.


I'm affirming your statement about the equals / compareTo methods. However, the equals implementation should use the same logic as compareTo since this is strongly recommended in the Javadoc of the Comparable interface.


André Wegmüller wrote on Mon, 16 September 2013 05:17
In our project we use a modified version of AbstractSqlStyle which solves the Number instance problem as described in the Bugzilla bug and in the forum post:
...
JDBC by default returns a BigDecimal for numeric types with a fixed scale. The current AbstractSqlStyle impl. always transforms a BigDecimal into a Double, which is wrong in my opinion, since Double does not have a fixed scale but is a floating point number. Our change also deals with that issue and returns BigDecimal in favor of Double.


Does the above change really solve the problem with the ClassCastException? I think you could end up having Long and Decimal data types in a single column in your case, couldn't you?

Quote:

Thus: the current impl. always returns Long or Double, so a change as suggested above could possibly break existing application-code. Whatever the correct solution is, there should be a way to activate/configure the current behavior for applications built with older versions of Scout.


Wouldn't it be cleaner to always return a BigDecimal value even if the scale is 0? That would solve the problem with the mixed up data types inside a single column. With respect to memory management however, we would end up having a lot of BigDecimal data types. I don't know how that would effect the performance.

Re: AbstractSqlStyle returns different data types in the same column [message #1111401 is a reply to message #1111001] Wed, 18 September 2013 06:53 Go to previous messageGo to next message
André Wegmüller is currently offline André WegmüllerFriend
Messages: 8
Registered: September 2012
Junior Member
Quote:
Does the above change really solve the problem with the ClassCastException?

Yes it does. For a single column the meta-data of the result-set is the same for all rows, so the method produces either Long or BD for a single column, no matter what the column-value of the individual row is.

Quote:
Wouldn't it be cleaner to always return a BigDecimal value even if the scale is 0?

Yes, I'd prefer that, since BD is what JDBC returns anyway, which means you don't have to do type-conversion anymore. With the current impl. you'll get Long for big numbers with scale 0, which is Ok in my opinion. So I thought, the impact on existing code is smaller when only numbers with scale > 0 are returned as BD and numbers with scale 0 are still returned as Long.
The impact on performance should be easy to measure with a profiler. My guess is, that it does not make a big difference. Maybe there's even a small improvement when type conversion is skipped. But I'd rather not say anything about performance, because my assumptions regarding that subject are wrong most of the time Smile
Re: AbstractSqlStyle returns different data types in the same column [message #1111462 is a reply to message #1111401] Wed, 18 September 2013 08:50 Go to previous messageGo to next message
Ken Lee is currently offline Ken LeeFriend
Messages: 97
Registered: March 2012
Member
André Wegmüller wrote on Wed, 18 September 2013 02:53
Yes it does. For a single column the meta-data of the result-set is the same for all rows, so the method produces either Long or BD for a single column, no matter what the column-value of the individual row is.


I'm not sure if your statement is really correct. The difference I see between your modified code for AbstractSqlStyle.readBind is that you avoid the conversion to Double and return the BigDecimal data type, which I also prefer since this is the type read from the result set.
However, if you have a look at the related forum post, they had a Oracle DB with a column declared as Number(15,5). Both values 1.5 and 1.0 are stored in the DB. Both are identified as NUMERIC types but for the value 1.5 a Double (in your case a BigDecimal) type is returned whereas for the value 1.0 a Long is given back. As a consequence we would end up having the same issue of mixed data types Long/Double or Long/BigDecimal respectively, wouldn't we?

Quote:

Yes, I'd prefer that, since BD is what JDBC returns anyway, which means you don't have to do type-conversion anymore.


+1

Quote:

With the current impl. you'll get Long for big numbers with scale 0, which is Ok in my opinion. So I thought, the impact on existing code is smaller when only numbers with scale > 0 are returned as BD and numbers with scale 0 are still returned as Long.


Cf. my response to the first quote. However, if I made a misinterpretation about the mixed up data types, then we may only change the return type for NUMERIC types with a scale > 0.

Quote:

But I'd rather not say anything about performance, because my assumptions regarding that subject are wrong most of the time Smile


I'm also very careful what to say when it comes to performance issues Smile
Re: AbstractSqlStyle returns different data types in the same column [message #1111583 is a reply to message #1111001] Wed, 18 September 2013 12:17 Go to previous messageGo to next message
Lukas Huser is currently offline Lukas HuserFriend
Messages: 42
Registered: March 2010
Member
Ken Lee
However, I was told that the meta data were not always reliable depending on the underlying DB. E.g. it doesn't work reliably with an Oracle 11 DB. So using meta data is not an option.

Ok. Would have been too easy, wouldn't it? Smile

André Wegmüller
In our project we use a modified version of AbstractSqlStyle which solves the Number instance problem as described in the Bugzilla bug and in the forum post

A quick search on Google returns many questions/problems regarding ResultSetMetaData.getScale(). Especially when your select contains numeric expressions (COLUMN * 100) or aggregation functions (SUM(COLUMN)) etc.
See for example:
http://stackoverflow.com/questions/11567099/resultsetmetadata-getscale-returns-0-when-using-aggregate-functions-like-min-o
http://stackoverflow.com/questions/5354176/resultsetmetadata-getscale-returns-0

So I guess Ken and Ivan are right that we cannot reliably determine the runtime type of a result set column. André, maybe you did not run into such problems with your specific setup, but I would now reject the meta data approach as a general solution.

André Wegmüller
JDBC by default returns a BigDecimal for numeric types with a fixed scale. The current AbstractSqlStyle impl. always transforms a BigDecimal into a Double, which is wrong in my opinion, since Double does not have a fixed scale but is a floating point number.

I totally agree. BigDecimal instead of Double would be a much better choice in this situation.

Ken Lee
Wouldn't it be cleaner to always return a BigDecimal value even if the scale is 0? That would solve the problem with the mixed up data types inside a single column.

Yes, I think this would be a reasonable behavior for the default implementation in AbstractSqlStyle. Backwards compatibility is also an issue of course (pure integer columns have consistently returned Long values, but they won't anymore with the new default).

Ivans solution, where we provide data types explicitly, provides the most flexibility, but is also a bit cumbersome if I simply want to force a single column to be of type Long in a large statement...

Certainly beyond a quick fix: Often the Object[][] data will be sent to the client and displayed in a table page, where the Java types of the columns are known. So this information could also be used on server side -- maybe by means of a typed TableDataHolder or similar -- to convert the retrieved data to the appropriate types...

Ken Lee
I'm affirming your statement about the equals / compareTo methods. However, the equals implementation should use the same logic as compareTo since this is strongly recommended in the Javadoc of the Comparable interface.

Ok, we're going a bit off-topic here Smile
Yes, compareTo() should be consistent with equals() whenever possible. The problem here is that we break the contract of equals() and hashCode() of CompositeObject if we do so (and as the current implementation actually does), which is far worse.
We cannot know whether compareTo() and equals() are consistent for all component objects. If they are not (for example if a CompositeObject contains a BigDecimal) we break the contract of CompositeObject.hashCode(), because two CompositeObjects are "equal" but won't return the same hash code!
Bottom line: CompositeObject should simply delegate to the hashCode(), equals() and compareTo() methods of its components.
Re: AbstractSqlStyle returns different data types in the same column [message #1112264 is a reply to message #1111583] Thu, 19 September 2013 10:29 Go to previous messageGo to next message
Ken Lee is currently offline Ken LeeFriend
Messages: 97
Registered: March 2012
Member
Lukas Huser
A quick search on Google returns many questions/problems regarding ResultSetMetaData.getScale(). Especially when your select contains numeric expressions (COLUMN * 100) or aggregation functions (SUM(COLUMN)) etc.
See for example:
http://stackoverflow.com/questions/11567099/resultsetmetadata-getscale-returns-0-when-using-aggregate-functions-like-min-o
http://stackoverflow.com/questions/5354176/resultsetmetadata-getscale-returns-0

So I guess Ken and Ivan are right that we cannot reliably determine the runtime type of a result set column. André, maybe you did not run into such problems with your specific setup, but I would now reject the meta data approach as a general solution.


I tried André's proposed solution by using the meta data of the result set to check for the scale of the bigdecimal.
I created an Oracle table with the values

CREATE TABLE test (id integer primary key not null, amount number(15,5))
INSERT INTO test VALUES (1, 1)
INSERT INTO test VALUES (2, 1.5)
INSERT INTO test VALUES (3, 1.12345)


and executed the query

SELECT id, amount*5 FROM test


and compared the resulting data types between the current Scout implementation and André's proposed one:

--- Current Scout Impl ---
rs.getBigDecimal() --> 5
Current impl. does: 5      class=class java.lang.Long
rs.getBigDecimal() --> 7.5
Current impl. does: 7.5    class=class java.lang.Double
rs.getBigDecimal() --> 5.61725
Current impl. does: 5.61725       class=class java.lang.Double

--- André's proposed Impl ---
rs.getBigDecimal() --> 5
André's impl. does: 5      class=class java.lang.Long
rs.getBigDecimal() --> 7.5
André's impl. does: 7      class=class java.lang.Long
rs.getBigDecimal() --> 5.61725
André's impl. does: 5      class=class java.lang.Long


It really seems that using the meta data of the result set for conversion leads to undesired precision loss. This does only happen if you calculate a value in the select statement that was already mentioned by Lukas. If you just select the amount column, the returned data type is the correct one.


So I've come to the conclusion where I would like to avoid the conversion from BigDecimal to Double/Long and return the BigDecimal data type instead.


Are there any other opinions or even objections?

[Updated on: Thu, 19 September 2013 10:32]

Report message to a moderator

Re: AbstractSqlStyle returns different data types in the same column [message #1115655 is a reply to message #1112264] Tue, 24 September 2013 11:47 Go to previous messageGo to next message
Ken Lee is currently offline Ken LeeFriend
Messages: 97
Registered: March 2012
Member
After some deeper analysis and several discussions I propose the following solution:


  1. org.eclipse.scout.commons.CompositeObject will be left unchanged.
  2. We will target the method readBindorg.eclipse.scout.rt.server.services.common.jdbc.style.AbstractSqlStyle


// current code
Object o = null;
switch (type) {
  // General Number
  case Types.DECIMAL:
  case Types.NUMERIC: {
    BigDecimal bd = rs.getBigDecimal(jdbcBindIndex);
    if (bd != null) {
      if (bd.scale() == 0) {
        o = new Long(bd.longValue());
      }
      else {
        o = new Double(bd.doubleValue());
      }
    }
    break;
  }
  ...


Instead of converting the BigDecimal data type depending on the scale to Long or Double, we will always return the BigDecimal data type as it was read from the DB.

As the code analysis has shown so far, returning a BigDecimal data type shouldn't have any impact on filling the FormData/ValueHolder on server-side nor on importing the FormData on client-side since org.eclipse.scout.commons.TypeCastUtility is used for conversion, e.g. it is able to convert a BigDecimal value to be displayed Long/Double column on the client.

However, if the data is read into an Object[][] and then further processed on the client, the programmer should be aware that the Numeric column will always be of type BigDecimal with the new implementation.

To provide backward compatibility we will introduce a property on AbstractSqlService that will be delegated to AbstractSqlStyle and defines if the new implementation (always return a BigDecimal data type) should be used or if the legacy implementation (i.e. the current one that potentially mixes up Long/Double) should be used in AbstractSqlStyle.
The new implementation will be used by default in Scout 3.10.

This change will be tracked in bug 394984.

Bug 416905 regarding the changes in CompositeObject will not be fixed.

Re: AbstractSqlStyle returns different data types in the same column [message #1124339 is a reply to message #1115655] Thu, 03 October 2013 12:41 Go to previous message
Urs Beeli is currently offline Urs BeeliFriend
Messages: 346
Registered: October 2012
Location: Bern, Switzerland
Senior Member
Is it possible that these changes break AbstractListBox<Long> and AbstractTreeBox<Long> as I describe in this thread: http://www.eclipse.org/forums/index.php/mv/msg/538261/1124335/#msg_1124335 ?
Previous Topic:Target file problems
Next Topic:Problem after deploying to tomcat
Goto Forum:
  


Current Time: Wed Dec 17 21:43:23 GMT 2014

Powered by FUDForum. Page generated in 0.18414 seconds
.:: Contact :: Home ::.

Powered by: FUDforum 3.0.2.
Copyright ©2001-2010 FUDforum Bulletin Board Software