Skip to content

Commit

Permalink
ISPN-14138 SQL Store fail to persist numbers with fractional portion
Browse files Browse the repository at this point in the history 
* Added support for Numeric
* Added support for Decimal
* Fixed Numeric to support decimals
** Numeric with 0 scale is still Integer
  • Loading branch information
@wburns
wburns committed Sep 27, 2022
1 parent 6e179aa commit 03e4af6
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 19 deletions.
20 changes: 14 additions & 6 deletions core/src/test/java/org/infinispan/persistence/BaseStoreFunctionalTest.java
View file
Expand Up @@ -164,10 +164,10 @@ public void testPreloadStoredAsBinary() {

byte[] pictureBytes = new byte[]{1, 82, 123, 19};

cache.put("k1", new Person("1"));
cache.put("k2", new Person("2", null, pictureBytes, null, null, false), 111111, TimeUnit.MILLISECONDS);
cache.put("k3", new Person("3", null, null, Sex.MALE, null, false), -1, TimeUnit.MILLISECONDS, 222222, TimeUnit.MILLISECONDS);
cache.put("k4", new Person("4", new Address("Street", "City", 12345), null, null, null, true), 333333, TimeUnit.MILLISECONDS, 444444, TimeUnit.MILLISECONDS);
cache.put("k1", createEmptyPerson("1"));
cache.put("k2", new Person("2", null, pictureBytes, null, null, false, 4.6, 5.6f, 8.4, 9.2f), 111111, TimeUnit.MILLISECONDS);
cache.put("k3", new Person("3", null, null, Sex.MALE, null, false, 4.7, 5.7f, 8.5, 9.3f), -1, TimeUnit.MILLISECONDS, 222222, TimeUnit.MILLISECONDS);
cache.put("k4", new Person("4", new Address("Street", "City", 12345), null, null, null, true, 4.8, 5.8f, 8.6, 9.4f), 333333, TimeUnit.MILLISECONDS, 444444, TimeUnit.MILLISECONDS);
Calendar calendar = Calendar.getInstance(TimeZone.getTimeZone("EST"));
calendar.set(2009, Calendar.MARCH, 18, 3, 22, 57);
// Chop off the last milliseconds as some databases don't have that high of accuracy
Expand All @@ -184,11 +184,19 @@ public void testPreloadStoredAsBinary() {
assertEquals(createEmptyPerson("1"), cache.get("k1"));
Person person2 = createEmptyPerson("2");
person2.setPicture(pictureBytes);
person2.setMoneyOwned(4.6);
person2.setMoneyOwed(5.6f);
person2.setDecimalField(8.4);
person2.setRealField(9.2f);
assertEquals(person2, cache.get("k2"));
Person person3 = createEmptyPerson("3");
person3.setSex(Sex.MALE);
person3.setMoneyOwned(4.7);
person3.setMoneyOwed(5.7f);
person3.setDecimalField(8.5);
person3.setRealField(9.3f);
assertEquals(person3, cache.get("k3"));
assertEquals(new Person("4", new Address("Street", "City", 12345), null, null, null, true), cache.get("k4"));
assertEquals(new Person("4", new Address("Street", "City", 12345), null, null, null, true, 4.8, 5.8f, 8.6, 9.4f), cache.get("k4"));
assertEquals(infinispanPerson, cache.get("Infinispan"));

cache.stop();
Expand All @@ -199,7 +207,7 @@ public void testPreloadStoredAsBinary() {
assertEquals(createEmptyPerson("1"), cache.get("k1"));
assertEquals(person2, cache.get("k2"));
assertEquals(person3, cache.get("k3"));
assertEquals(new Person("4", new Address("Street", "City", 12345), null, null, null, true), cache.get("k4"));
assertEquals(new Person("4", new Address("Street", "City", 12345), null, null, null, true, 4.8, 5.8f, 8.6, 9.4f), cache.get("k4"));
assertEquals(infinispanPerson, cache.get("Infinispan"));
}

Expand Down
71 changes: 68 additions & 3 deletions core/src/test/java/org/infinispan/test/data/Person.java
View file
Expand Up @@ -18,23 +18,36 @@ public class Person implements Serializable, JsonSerialization {
Sex sex;
Date birthDate;
boolean acceptedToS;
double moneyOwned;
float moneyOwed;
double decimalField;
float realField;

public Person() {
// Needed for serialization
}

public Person(String name) {
this(name, null, null, null, null, false);
this(name, null);
}

public Person(String name, Address address) {
this(name, address, null, null, null, false, 1.1, 0.4f, 10.3, 4.7f);
}

@ProtoFactory
public Person(String name, Address address, byte[] picture, Sex sex, Date birthDate, boolean acceptedToS) {
public Person(String name, Address address, byte[] picture, Sex sex, Date birthDate, boolean acceptedToS, double moneyOwned,
float moneyOwed, double decimalField, float realField) {
this.name = name;
this.address = address;
this.picture = picture;
this.sex = sex;
this.birthDate = birthDate;
this.acceptedToS = acceptedToS;
this.moneyOwned = moneyOwned;
this.moneyOwed = moneyOwed;
this.decimalField = decimalField;
this.realField = realField;
}

@ProtoField(1)
Expand Down Expand Up @@ -91,6 +104,42 @@ public void setAcceptedToS(boolean acceptedToS) {
this.acceptedToS = acceptedToS;
}

@ProtoField(value = 7, defaultValue = "1.1")
public double getMoneyOwned() {
return moneyOwned;
}

public void setMoneyOwned(double moneyOwned) {
this.moneyOwned = moneyOwned;
}

@ProtoField(value = 8, defaultValue = "0.4")
public float getMoneyOwed() {
return moneyOwed;
}

public void setMoneyOwed(float moneyOwed) {
this.moneyOwed = moneyOwed;
}

@ProtoField(value = 9, defaultValue = "10.3")
public double getDecimalField() {
return decimalField;
}

public void setDecimalField(double decimalField) {
this.decimalField = decimalField;
}

@ProtoField(value = 10, defaultValue = "4.7")
public float getRealField() {
return realField;
}

public void setRealField(float realField) {
this.realField = realField;
}

@Override
public String toString() {
return "Person{" +
Expand All @@ -100,6 +149,10 @@ public String toString() {
", sex=" + sex +
", birthDate=" + birthDate +
", acceptedToS=" + acceptedToS +
", moneyOwned=" + moneyOwned +
", moneyOwed=" + moneyOwed +
", decimalField=" + decimalField +
", realField=" + realField +
'}';
}

Expand All @@ -116,6 +169,10 @@ public boolean equals(Object o) {
if (sex != null ? !sex.equals(person.sex) : person.sex != null) return false;
if (birthDate != null ? !birthDate.equals(person.birthDate) : person.birthDate != null) return false;
if (acceptedToS != person.acceptedToS) return false;
if (moneyOwned != person.moneyOwned) return false;
if (moneyOwed != person.moneyOwed) return false;
if (decimalField != person.decimalField) return false;
if (realField != person.realField) return false;

return true;
}
Expand All @@ -129,6 +186,10 @@ public int hashCode() {
result = 29 * result + (sex != null ? sex.hashCode() : 0);
result = 29 * result + (birthDate != null ? birthDate.hashCode() : 0);
result = 29 * result + Boolean.hashCode(acceptedToS);
result = 29 * result + Double.hashCode(moneyOwned);
result = 29 * result + Float.hashCode(moneyOwed);
result = 29 * result + Double.hashCode(decimalField);
result = 29 * result + Float.hashCode(realField);
return result;
}

Expand All @@ -140,6 +201,10 @@ public Json toJson() {
.set("picture", picture)
.set("sex", sex)
.set("birthDate", birthDate == null ? 0 : birthDate.getTime())
.set("acceptedToS", acceptedToS);
.set("acceptedToS", acceptedToS)
.set("moneyOwned", moneyOwned)
.set("moneyOwed", moneyOwed)
.set("decimalField", decimalField)
.set("realField", realField);
}
}
2 changes: 1 addition & 1 deletion ...c/test/java/org/infinispan/persistence/jdbc/stringbased/TwoWayPersonKey2StringMapper.java
View file
Expand Up @@ -22,6 +22,6 @@ public Object getKeyMapping(String key) {
String street = tkz.nextToken();
String city = tkz.nextToken();
String zip = tkz.nextToken();
return new Person(name, new Address(street, city, Integer.parseInt(zip)), null, null, null, false);
return new Person(name, new Address(street, city, Integer.parseInt(zip)));
}
}
9 changes: 7 additions & 2 deletions persistence/sql/src/main/java/org/infinispan/persistence/sql/AbstractSchemaJdbcStore.java
View file
Expand Up @@ -101,7 +101,7 @@ protected Parameter[] determinePrimaryParameters(C config, Parameter[] allParame
*/
abstract Parameter[] generateParameterInformation(C config, ConnectionFactory connectionFactory) throws SQLException;

int typeWeUse(int sqlType, String typeName) {
int typeWeUse(int sqlType, String typeName, int scale) {
if (sqlType == Types.VARCHAR) {
// Some DBs store VARBINARY as VARCHAR FOR BIT DATA (ahem... DB2)
if (typeName.contains("BIT") || typeName.contains("BINARY")) {
Expand All @@ -110,6 +110,9 @@ int typeWeUse(int sqlType, String typeName) {
} else if (typeName.toUpperCase().startsWith("BOOL")) {
// Some databases store as int32 or something similar but have the typename as BOOLEAN or some derivation
return Types.BOOLEAN;
} else if (sqlType == Types.NUMERIC && scale == 0) {
// If scale is 0 we don't want to use float or double types
return Types.INTEGER;
}
return sqlType;
}
Expand Down Expand Up @@ -360,13 +363,15 @@ protected enum ProtostreamFieldType {
protected static ProtostreamFieldType from(int sqlType) {
switch (sqlType) {
case Types.INTEGER:
case Types.NUMERIC:
return INT_32;
case Types.BIGINT:
return INT_64;
case Types.FLOAT:
case Types.REAL:
return FLOAT;
case Types.DOUBLE:
case Types.NUMERIC:
case Types.DECIMAL:
return DOUBLE;
case Types.BIT:
case Types.BOOLEAN:
Expand Down
3 changes: 2 additions & 1 deletion persistence/sql/src/main/java/org/infinispan/persistence/sql/QueriesJdbcStore.java
View file
Expand Up @@ -97,7 +97,8 @@ Parameter[] generateParameterInformation(QueriesJdbcStoreConfiguration config, C
for (int i = 1; i <= rsMetadata.getColumnCount(); ++i) {
int columnType = rsMetadata.getColumnType(i);
String name = rsMetadata.getColumnName(i);
int actualType = typeWeUse(columnType, rsMetadata.getColumnTypeName(i));
int scale = rsMetadata.getScale(i);
int actualType = typeWeUse(columnType, rsMetadata.getColumnTypeName(i), scale);
ProtostreamFieldType type = ProtostreamFieldType.from(actualType);
String lowerCaseName = name.toLowerCase();
// Make sure to reuse same parameter instance just with different offset
Expand Down
3 changes: 2 additions & 1 deletion persistence/sql/src/main/java/org/infinispan/persistence/sql/TableJdbcStore.java
View file
Expand Up @@ -134,7 +134,8 @@ Parameter[] generateParameterInformation(TableJdbcStoreConfiguration config, Con
while (rs.next()) {
String name = rs.getString("COLUMN_NAME");
int sqlColumnType = rs.getInt("DATA_TYPE");
int actualType = typeWeUse(sqlColumnType, rs.getString("TYPE_NAME"));
int scale = rs.getInt("DECIMAL_DIGITS");
int actualType = typeWeUse(sqlColumnType, rs.getString("TYPE_NAME"), scale);

ProtostreamFieldType schemaType = ProtostreamFieldType.from(actualType);
boolean isPrimary = primaryKeyList.contains(name.toUpperCase());
Expand Down
6 changes: 5 additions & 1 deletion ...ence/sql/src/test/java/org/infinispan/persistence/sql/AbstractSQLStoreFunctionalTest.java
View file
Expand Up @@ -106,7 +106,7 @@ protected void clearTempDir() {
// DB table is denormalized when read
@Override
protected Person createEmptyPerson(String name) {
return new Person(name, new Address(), null, null, null, false);
return new Person(name, new Address());
}

@Override
Expand Down Expand Up @@ -432,6 +432,10 @@ protected void createTable(String cacheName, String tableName, ConnectionFactory
"accepted_tos " + booleanType() + ", " +
"sex VARCHAR(255), " +
"birthdate " + dateTimeType() + ", " +
"moneyOwned NUMERIC(10, 4), " +
"moneyOwed FLOAT, " +
"decimalField DECIMAL(10, 4), " +
"realField REAL, " +
"PRIMARY KEY (keycolumn))";
} else if (cacheName.equalsIgnoreCase("testStoreByteArrays")) {
tableCreation = "CREATE TABLE " + tableName + " (" +
Expand Down
7 changes: 6 additions & 1 deletion persistence/sql/src/test/java/org/infinispan/persistence/sql/QueriesJdbcJoinTest.java
View file
Expand Up @@ -234,7 +234,12 @@ private Person samplePerson(String name) {
address.setCity("London");
address.setStreet("Cool Street");
address.setZip(1321);
return new Person(name, address, new byte[]{0x1, 0x12}, Sex.MALE, new java.util.Date(1629495308), true);
Person person = new Person(name, address);
person.setPicture(new byte[]{0x1, 0x12});
person.setSex(Sex.MALE);
person.setBirthDate(new java.util.Date(1629495308));
person.setAcceptedToS(true);
return person;
}

private String insertTable1Statement(boolean idJoin, boolean namedParams) {
Expand Down
6 changes: 3 additions & 3 deletions ...ence/sql/src/test/java/org/infinispan/persistence/sql/QueriesJdbcStoreFunctionalTest.java
View file
Expand Up @@ -60,10 +60,10 @@ protected PersistenceConfigurationBuilder createCacheStoreConfig(PersistenceConf
storeBuilder.keyColumns(KEY_COLUMN);
if (cacheName.equalsIgnoreCase("testPreloadStoredAsBinary")) {
storeBuilder.queries()
.select("SELECT " + KEY_COLUMN + ", name, STREET, city, ZIP, picture, sex, birthdate, accepted_tos FROM " + tableName + " WHERE " + KEY_COLUMN + " = :" + KEY_COLUMN)
.selectAll("SELECT " + KEY_COLUMN + ", name, street, city, zip, picture, sex, birthdate, accepted_tos FROM " + tableName)
.select("SELECT " + KEY_COLUMN + ", name, STREET, city, ZIP, picture, sex, birthdate, accepted_tos, moneyOwned, moneyOwed, decimalField, realField FROM " + tableName + " WHERE " + KEY_COLUMN + " = :" + KEY_COLUMN)
.selectAll("SELECT " + KEY_COLUMN + ", name, street, city, zip, picture, sex, birthdate, accepted_tos, moneyOwned, moneyOwed, decimalField, realField FROM " + tableName)
.upsert(manager.getUpsertStatement(Collections.singletonList(KEY_COLUMN),
Arrays.asList(KEY_COLUMN, "name", "street", "CITY", "zip", "picture", "sex", "birthdate", "accepted_tos")))
Arrays.asList(KEY_COLUMN, "name", "street", "CITY", "zip", "picture", "sex", "birthdate", "accepted_tos", "moneyOwned", "moneyOwed", "decimalField", "realField")))
.delete("DELETE FROM " + tableName + " WHERE " + KEY_COLUMN + " = :" + KEY_COLUMN);
} else if (cacheName.equalsIgnoreCase("testStoreByteArrays")) {
storeBuilder.queries()
Expand Down

0 comments on commit 03e4af6

Please sign in to comment.