Skip to content

Commit

Permalink
Sqlite precision/driver issues
Browse files Browse the repository at this point in the history
Using ROUND(...,33) in sqlite causes more issues than it solves for exact to inexact conversion
  for ceil & floor.  So allow the SqlHooks to turn off the rounding and control the precision
Have special hooks for testing without the standard math functions, since the sqlite-jdbc
  doesn't use the standard math functions (see xerial/sqlite-jdbc#763)
Turn off binding because the default driver uses numbers for date binding when testing
More fixes for sqlite for date math
  • Loading branch information
steventamm committed Oct 23, 2022
1 parent 2fb8c0f commit 4cfd980
Show file tree
Hide file tree
Showing 17 changed files with 340 additions and 98 deletions.
14 changes: 12 additions & 2 deletions api/src/main/java/com/force/formula/util/BigDecimalHelper.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,12 @@

package com.force.formula.util;

import java.math.*;
import java.sql.*;
import java.math.BigDecimal;
import java.math.MathContext;
import java.math.RoundingMode;
import java.sql.CallableStatement;
import java.sql.ResultSet;
import java.sql.SQLException;

/**
* BigDecimal helper class
Expand All @@ -31,6 +35,12 @@ public class BigDecimalHelper {
public static final int NUMBER_PRECISION_EXTERNAL = 33;
public static final MathContext MC_PRECISION_EXTERNAL = new MathContext(NUMBER_PRECISION_EXTERNAL, RoundingMode.HALF_UP);

/**
* For low-precision DBs, use this constant to prevent ROUND(,33) from causing Round/Ceil to get confused with
* integers
*/
public static final int DOUBLE_PRECISION = 16;

public static final BigDecimal getNonNullFromRs(ResultSet rs, String colName) throws SQLException {
BigDecimal bd = rs.getBigDecimal(colName);
return bd == null ? BigDecimal.ZERO : bd;
Expand Down
14 changes: 13 additions & 1 deletion api/src/main/java/com/force/formula/util/FormulaDateUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
import java.math.RoundingMode;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.util.*;
import java.util.Calendar;
import java.util.Date;
import java.util.TimeZone;
import java.util.function.Supplier;

import com.force.i18n.BaseLocalizer;
Expand All @@ -29,6 +31,7 @@ private FormulaDateUtil() {}


private static final ThreadLocal<SimpleDateFormat> ISO8601_FORMATTER = ThreadLocal.withInitial(sdfWithGMT("yyyy-MM-dd'T'HH:mm:ss'Z'"));
private static final ThreadLocal<SimpleDateFormat> SQL_TIMESTAMP_FORMATTER = ThreadLocal.withInitial(sdfWithGMT("yyyy-MM-dd HH:mm:ss"));
private static final ThreadLocal<SimpleDateFormat> ISO8601_MILLISECOND_FORMATTER = ThreadLocal.withInitial(sdfWithGMT("yyyy-MM-dd'T'HH:mm:ss.SSS'Z'"));

private static final ThreadLocal<SimpleDateFormat> SQL_FORMATTER = ThreadLocal.withInitial(() -> new SimpleDateFormat("dd-MM-yyyy"));
Expand Down Expand Up @@ -204,6 +207,15 @@ public static String formatDatetimeToISO8601(Date date) {
return ISO8601_FORMATTER.get().format(date);
}

/**
* Returns a string in SQL Literal format, with both date and time
* e.g. 2011-01-31 22:59:48
* @param date the date to format
* @return the value of the date formatted with ISO8601 format.
*/
public static String formatDatetimeToSqlLiteral(Date date) {
return SQL_TIMESTAMP_FORMATTER.get().format(date);
}

public static Calendar translateCal(Calendar from, Calendar to, Date date, boolean toMidnight) {
from.setTime(date);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ public ContextualFormulaFieldInfo lookup(String name, boolean isDynamicRefBase)
return originDateTime_PRESTO;
} else if (style != null && style.isGoogleStyle()) {
return originDateTime_GOOGLE;
} else if (style != null && style.isSqliteStyle()) {
return originDateTime_SQLITE;
}
return originDateTime;
} else {
Expand Down Expand Up @@ -143,6 +145,8 @@ public SQLPair getSQL(ITableAliasRegistry registry) {
FormulaEngine.getHooks().getDataTypeByName("DateTime"), new SQLPair("DATEFROMPARTS(1900,1,1)", null));
private static final SystemFormulaFieldInfo originDateTime_GOOGLE = new SystemFormulaFieldInfo(ORIGIN_DATE_TIME,
FormulaEngine.getHooks().getDataTypeByName("DateTime"), new SQLPair("DATE(1900,1,1)", null));
private static final SystemFormulaFieldInfo originDateTime_SQLITE = new SystemFormulaFieldInfo(ORIGIN_DATE_TIME,
FormulaEngine.getHooks().getDataTypeByName("DateTime"), new SQLPair("'1900-01-01 00:00:00'", null));
private static DisplayField[] displayFields = new DisplayField[] { new DisplayField(SYSTEM_NAMESPACE, SYSTEM_NAMESPACE, originDateTime) };

static {
Expand Down
5 changes: 5 additions & 0 deletions docs/coverage/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,11 @@
<artifactId>formula-engine-sqlserver-test</artifactId>
<version>${project.version}</version>
</dependency>
<dependency>
<groupId>com.salesforce.formula</groupId>
<artifactId>formula-engine-google-test</artifactId>
<version>${project.version}</version>
</dependency>
</dependencies>
</profile>
</profiles>
Expand Down
16 changes: 13 additions & 3 deletions impl/src/main/java/com/force/formula/commands/FunctionCeiling.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package com.force.formula.commands;

import java.math.*;
import java.math.BigDecimal;
import java.math.MathContext;
import java.math.RoundingMode;

import com.force.formula.FormulaCommandType.AllowedContext;
import com.force.formula.FormulaCommandType.SelectorSection;
import com.force.formula.FormulaContext;
import com.force.formula.impl.FormulaAST;
import com.force.formula.impl.FormulaSqlHooks;
import com.force.formula.impl.JsValue;
import com.force.formula.sql.SQLPair;
import com.force.formula.util.BigDecimalHelper;
Expand Down Expand Up @@ -41,8 +44,15 @@ protected BigDecimal execute(BigDecimal value) {

@Override
public SQLPair getSQL(FormulaAST node, FormulaContext context, String[] args, String[] guards) {
String ceil = context.getSqlStyle().isTransactSqlStyle() ? "CEILING" : "CEIL";
String sql = "CASE WHEN " + args[0] + ">=0 THEN "+ceil+"(ROUND(" + args[0] + ","+BigDecimalHelper.NUMBER_PRECISION_EXTERNAL+")) ELSE FLOOR(ROUND(" + args[0] + ","+BigDecimalHelper.NUMBER_PRECISION_EXTERNAL+")) END";
FormulaSqlHooks hooks = (FormulaSqlHooks)context.getSqlStyle();
String ceil = hooks.isTransactSqlStyle() ? "CEILING" : "CEIL";
int precision = hooks.getExternalPrecision();
String sql;
if (precision >= 0) { // If external precision is -1 don't reound before Ceil/Floor
sql = "CASE WHEN " + args[0] + ">=0 THEN "+ceil+"(ROUND(" + args[0] + ","+precision+")) ELSE FLOOR(ROUND(" + args[0] + ","+precision+")) END";
} else {
sql = "CASE WHEN " + args[0] + ">=0 THEN "+ceil+"(" + args[0] + ") ELSE FLOOR(" + args[0] + ") END";
}
return new SQLPair(sql, guards[0]);
}

Expand Down
16 changes: 13 additions & 3 deletions impl/src/main/java/com/force/formula/commands/FunctionFloor.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,14 @@
package com.force.formula.commands;

import java.math.*;
import java.math.BigDecimal;
import java.math.MathContext;
import java.math.RoundingMode;

import com.force.formula.FormulaCommandType.AllowedContext;
import com.force.formula.FormulaCommandType.SelectorSection;
import com.force.formula.FormulaContext;
import com.force.formula.impl.FormulaAST;
import com.force.formula.impl.FormulaSqlHooks;
import com.force.formula.impl.JsValue;
import com.force.formula.sql.SQLPair;
import com.force.formula.util.BigDecimalHelper;
Expand Down Expand Up @@ -37,8 +40,15 @@ protected BigDecimal execute(BigDecimal value) {

@Override
public SQLPair getSQL(FormulaAST node, FormulaContext context, String[] args, String[] guards) {
String ceil = context.getSqlStyle().isTransactSqlStyle() ? "CEILING" : "CEIL";
String sql = "CASE WHEN " + args[0] + ">=0 THEN FLOOR(ROUND(" + args[0] + ","+BigDecimalHelper.NUMBER_PRECISION_EXTERNAL+")) ELSE "+ceil+"(ROUND(" + args[0] + ","+BigDecimalHelper.NUMBER_PRECISION_EXTERNAL+")) END";
FormulaSqlHooks hooks = (FormulaSqlHooks)context.getSqlStyle();
String ceil = hooks.isTransactSqlStyle() ? "CEILING" : "CEIL";
int precision = hooks.getExternalPrecision();
String sql;
if (precision >= 0) { // If external precision is -1 don't reound before Ceil/Floor
sql = "CASE WHEN " + args[0] + ">=0 THEN FLOOR(ROUND(" + args[0] + ","+precision+")) ELSE "+ceil+"(ROUND(" + args[0] + ","+precision+")) END";
} else {
sql = "CASE WHEN " + args[0] + ">=0 THEN FLOOR(" + args[0] + ") ELSE "+ceil+"(" + args[0] + ") END";
}
return new SQLPair(sql, guards[0]);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@

import static com.force.formula.commands.FormulaCommandInfoImpl.jsMathPkg;

import java.math.*;
import java.math.BigDecimal;
import java.math.MathContext;
import java.math.RoundingMode;

import com.force.formula.FormulaCommandType.AllowedContext;
import com.force.formula.FormulaCommandType.SelectorSection;
import com.force.formula.FormulaContext;
import com.force.formula.impl.FormulaAST;
import com.force.formula.impl.FormulaSqlHooks;
import com.force.formula.impl.JsValue;
import com.force.formula.sql.SQLPair;
import com.force.formula.util.BigDecimalHelper;
Expand Down Expand Up @@ -43,8 +46,15 @@ protected BigDecimal execute(BigDecimal value) {

@Override
public SQLPair getSQL(FormulaAST node, FormulaContext context, String[] args, String[] guards) {
String ceil = context.getSqlStyle().isTransactSqlStyle() ? "CEILING" : "CEIL";
String sql = ceil + "(ROUND(" + args[0] + ","+BigDecimalHelper.NUMBER_PRECISION_EXTERNAL+"))";
FormulaSqlHooks hooks = (FormulaSqlHooks)context.getSqlStyle();
String ceil = hooks.isTransactSqlStyle() ? "CEILING" : "CEIL";
int precision = hooks.getExternalPrecision();
String sql;
if (precision >= 0) { // If external precision is -1 don't reound before Ceil/Floor
sql = ceil + "(ROUND(" + args[0] + ","+precision+"))";
} else {
sql = ceil + "(" + args[0] + ")";
}
return new SQLPair(sql, guards[0]);
}

Expand Down
16 changes: 13 additions & 3 deletions impl/src/main/java/com/force/formula/commands/FunctionMFloor.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@

import static com.force.formula.commands.FormulaCommandInfoImpl.jsMathPkg;

import java.math.*;
import java.math.BigDecimal;
import java.math.MathContext;
import java.math.RoundingMode;

import com.force.formula.*;
import com.force.formula.FormulaCommandType.AllowedContext;
import com.force.formula.FormulaCommandType.SelectorSection;
import com.force.formula.FormulaContext;
import com.force.formula.impl.FormulaAST;
import com.force.formula.impl.FormulaSqlHooks;
import com.force.formula.impl.JsValue;
import com.force.formula.sql.SQLPair;
import com.force.formula.util.BigDecimalHelper;
Expand Down Expand Up @@ -40,7 +43,14 @@ protected BigDecimal execute(BigDecimal value) {

@Override
public SQLPair getSQL(FormulaAST node, FormulaContext context, String[] args, String[] guards) {
String sql = "FLOOR(ROUND(" + args[0] + ","+BigDecimalHelper.NUMBER_PRECISION_EXTERNAL+"))";
FormulaSqlHooks hooks = (FormulaSqlHooks)context.getSqlStyle();
int precision = hooks.getExternalPrecision();
String sql;
if (precision >= 0) { // If external precision is -1 don't reound before Ceil/Floor
sql = "FLOOR(ROUND(" + args[0] + ","+precision+"))";
} else {
sql = "FLOOR(" + args[0] + ")";
}
return new SQLPair(sql, guards[0]);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@
import com.force.formula.FormulaCommandType.AllowedContext;
import com.force.formula.FormulaCommandType.SelectorSection;
import com.force.formula.FormulaContext;
import com.force.formula.impl.*;
import com.force.formula.impl.FormulaAST;
import com.force.formula.impl.FormulaSqlHooks;
import com.force.formula.impl.JsValue;
import com.force.formula.sql.SQLPair;
import com.force.formula.util.BigDecimalHelper;

Expand Down Expand Up @@ -37,7 +39,7 @@ protected BigDecimal execute(BigDecimal lhs, BigDecimal rhs) {
@Override
public SQLPair getSQL(FormulaAST node, FormulaContext context, String[] args, String[] guards) {
FormulaSqlHooks hooks = (FormulaSqlHooks) context.getSqlStyle();
String sql = "ROUND(" + args[0] + ", " + hooks.sqlRoundScaleArg(args[1]) + ")";
String sql = hooks.sqlRound(args[0], hooks.sqlRoundScaleArg(args[1]));
String guard = SQLPair.generateGuard(guards, null);
return new SQLPair(sql, guard);
}
Expand Down
23 changes: 22 additions & 1 deletion impl/src/main/java/com/force/formula/impl/FormulaSqlHooks.java
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

import com.force.formula.sql.FormulaSqlStyle;
import com.force.formula.sql.SQLPair;
import com.force.formula.util.BigDecimalHelper;
import com.force.formula.util.FormulaDateUtil;
import com.google.common.collect.Multimap;
import com.google.common.collect.Multimaps;
Expand All @@ -25,7 +26,16 @@
* @since 0.1.0
*/
public interface FormulaSqlHooks extends FormulaSqlStyle {

/**
* Precision to use rounding for ceil/floor to prevent issues with inexact to exact transitions
* like 1/3 or 1/11. Return -1 if you don't want any rounding.
* @return 33 by default
* @see BigDecimalHelper#NUMBER_PRECISION_EXTERNAL
*/
default int getExternalPrecision() {
return BigDecimalHelper.NUMBER_PRECISION_EXTERNAL;
}

// Handle plsql regexp differences (where oracle needs regexp_like and postgres wants ~ or similar to)
/**
* @return how to do "not regexp_like", where the %s is used to represent the value to guard against for DateTime Value
Expand Down Expand Up @@ -682,6 +692,16 @@ default String sqlTrunc(String argument) {
default String sqlTrunc(String argument, String scale) {
return "TRUNC(" + argument + ", " + scale + ")";
}

/**
* Parameterized for those DBs that don't handle negative rounding
* @param argument the value to round
* @param scale the scale to truncate to
* @return how to call round to drop to the given number of decimal places
*/
default String sqlRound(String argument, String scale) {
return "ROUND(" + argument + ", " + scale + ")";
}


/**
Expand Down Expand Up @@ -781,4 +801,5 @@ default String sqlRpad(String str, String amount, String pad) {
return "RPAD(" + str + ", " + amount + ")";
}
}

}
Loading

0 comments on commit 4cfd980

Please sign in to comment.