Skip to content

A bare decimal literal is compared at FLOAT precision, so >= 0.05 on a DOUBLE column silently drops every row equal to 0.05 while < 0.05 returns them #7609

Description

@tae898

Summary

A floating-point literal written without a type suffix (0.05, not 0.05D) is parsed into a Float, and the comparison operators then widen that Float to double with .doubleValue(), which preserves the single-precision rounding error instead of removing it. Against a DOUBLE or DECIMAL column the literal is therefore not the number the user typed, and every row sitting exactly on the boundary is decided the wrong way, with no error and no warning.

>= 0.05 returns exactly what > 0.05 returns. = 0.05 returns nothing. < 0.05 returns the rows that are equal to 0.05. Which side loses depends only on the sign of the rounding error for that literal, so it is not always the lower bound: at 0.7 it is <= that fails and >= that is correct.

BETWEEN, a bound parameter, an explicit D suffix, and an indexed column are all correct on the same data, so the same query answers differently depending on whether an index happens to exist on the column.

Found on a TPC-H Q6 shape (l_discount BETWEEN :d - 0.01 AND :d + 0.01 written as two comparisons), where it summed two of the three discount buckets and reported revenue about 30% low.

Version: 26.9.1 (build 9cea8e848fa57d275b8d614328245508cd76614a, branch main), from the official arcadedata/arcadedb:26.9.1 image, JDK Temurin 21.0.12. Both code sites named below are unchanged on main as of d3d7497286a021a3b38332fe2f443e41e950995d.

Reproduction

One self-contained class, no Maven, no test harness. It builds and runs inside the published image:

docker run --rm -v "$PWD":/work -w /work --entrypoint sh arcadedata/arcadedb:26.9.1 -c \
  'javac -cp "/home/arcadedb/lib/*" DecimalLiteralComparisonRepro.java && \
   java -cp "/home/arcadedb/lib/*:." DecimalLiteralComparisonRepro'

It creates fifty documents, ten each at 0.04, 0.05, 0.06, 0.07, and 0.08, storing the same five values three times: on a DOUBLE property, on a FLOAT property, and on a DECIMAL property. It then prints returned versus expected row counts for each comparison. Full source is attached below.

Observed output

A. DOUBLE property, the shape TPC-H Q6 hits
  query                                      returned   expected
  disc = 0.05                                       0         10   WRONG
  disc > 0.05                                      30         30   ok
  disc >= 0.05                                     30         40   WRONG
  disc < 0.05                                      20         10   WRONG
  disc <= 0.05                                     20         20   ok
  disc IN [0.05]                                    0         10   WRONG
  disc >= 0.05 AND disc <= 0.07                    20         30   WRONG
  disc BETWEEN 0.05 AND 0.07                       30         30   ok
  sum(disc) over the >= / <= window            1.3000        1.8
  sum(disc) over the BETWEEN window            1.8000        1.8

Note disc < 0.05 returning 20. That is not a missing row, it is a false positive: a strict less-than returns the rows that are equal to the literal.

Which declared type is affected

B. The same comparisons on each declared property type, at 0.05
  DOUBLE: = 0.05                                    0         10   WRONG
  DOUBLE: >= 0.05                                  30         40   WRONG
  DOUBLE: <= 0.05                                  20         20   ok
  DOUBLE: BETWEEN 0.05 AND 0.07                    30         30   ok
  FLOAT: = 0.05                                    10         10   ok
  FLOAT: >= 0.05                                   40         40   ok
  FLOAT: <= 0.05                                   20         20   ok
  FLOAT: BETWEEN 0.05 AND 0.07                     30         30   ok
  DECIMAL: = 0.05                                   0         10   WRONG
  DECIMAL: >= 0.05                                 30         40   WRONG
  DECIMAL: <= 0.05                                 20         20   ok
  DECIMAL: BETWEEN 0.05 AND 0.07                   30         30   ok

FLOAT is correct because the stored value carries the same rounding error as the literal. DOUBLE and DECIMAL are both wrong.

Which side loses depends on the literal

C. Other literals on the DOUBLE property
  Sw: = 0.1        0 / 10   WRONG    Sw: >= 0.1   40 / 50   WRONG    Sw: <= 0.1   10 / 10   ok
      float(0.1) as double = 0.10000000149011612, double(0.1) = 0.1 -> literal is ABOVE the stored value
  Sw: = 0.2        0 / 10   WRONG    Sw: >= 0.2   30 / 40   WRONG    Sw: <= 0.2   20 / 20   ok
      float(0.2) as double = 0.20000000298023224, double(0.2) = 0.2 -> literal is ABOVE the stored value
  Sw: = 0.3        0 / 10   WRONG    Sw: >= 0.3   20 / 30   WRONG    Sw: <= 0.3   30 / 30   ok
      float(0.3) as double = 0.30000001192092896, double(0.3) = 0.3 -> literal is ABOVE the stored value
  Sw: = 0.7        0 / 10   WRONG    Sw: >= 0.7   20 / 20   ok       Sw: <= 0.7   30 / 40   WRONG
      float(0.7) as double = 0.699999988079071,  double(0.7) = 0.7 -> literal is BELOW the stored value

At 0.7 the error has the other sign, so it is the upper bound that loses its rows. A range written as x >= a AND x <= b can therefore lose either end, both ends, or neither, depending only on the two constants.

What is correct on the same data

D. Bound parameter and explicit D suffix
  dDouble = :d  (Double parameter)                 10         10   ok
  dDouble >= :d (Double parameter)                 40         40   ok
  dDouble = 0.05D  (D suffix)                      10         10   ok
  dDouble >= 0.05D (D suffix)                      40         40   ok

E. The SAME fifty rows on a DOUBLE property that carries a NOTUNIQUE index
  Ix: v = 0.05                                     10         10   ok
  Ix: v > 0.05                                     30         30   ok
  Ix: v >= 0.05                                    40         40   ok
  Ix: v < 0.05                                     10         10   ok
  Ix: v <= 0.05                                    20         20   ok
  Ix: v BETWEEN 0.05 AND 0.07                      30         30   ok
      plan for `Ix: v >= 0.05` : + FETCH FROM INDEX Ix[v] ()
      plan for `disc >= 0.05`  : + FETCH FROM TYPE Li WITH FILTER ()

The index path is right and the filter path is wrong, on identical data and identical SQL. Adding or dropping an index changes the answer.

Where it comes from

Two sites, both on the filter path only.

FloatingPoint.getValue() downcasts a suffix-less literal to float:

// engine/src/main/java/com/arcadedb/query/sql/parser/FloatingPoint.java:52
final double returnValue = Double.parseDouble(stringValue) * sign;
if (Math.abs(returnValue) < Float.MAX_VALUE) {
  finalValue = (float) returnValue;     // 0.05 becomes 0.05f
} else {
  finalValue = returnValue;
}

Type.castComparableNumber() then widens that Float back with .doubleValue(), which reproduces the float's error as a double rather than undoing it:

// engine/src/main/java/com/arcadedb/schema/Type.java:1326
} else if (left instanceof Double) {
  if (right instanceof BigDecimal)
    left = BigDecimal.valueOf(left.doubleValue());
  else if (right instanceof Byte || right instanceof Short || right instanceof Integer || right instanceof Long
      || right instanceof Float)
    right = right.doubleValue();        // 0.05f -> 0.05000000074505806

So 0.05d >= 0.05000000074505806 is false, and the boundary row is dropped. GeOperator, GtOperator, LeOperator, LtOperator, and QueryOperatorEquals.equals() all reach the comparison through castComparableNumber.

The DECIMAL case fails the same way eight lines down, because BigDecimal.valueOf has no float overload and the argument widens through double first:

// engine/src/main/java/com/arcadedb/schema/Type.java:1334
} else if (left instanceof BigDecimal) {
  ...
  else if (right instanceof Float float1)
    right = BigDecimal.valueOf(float1);   // 0.05f -> 0.05000000074505806

BETWEEN is correct because it takes the other route, converting each bound to the field's own class before comparing:

// engine/src/main/java/com/arcadedb/query/sql/parser/BetweenCondition.java:57
secondValue = Type.convertOrNull(context.getDatabase(), secondValue, firstValue.getClass());

and Type.convert() already re-parses rather than widening, with a comment that says exactly why:

// engine/src/main/java/com/arcadedb/schema/Type.java:577
} else if (targetClass.equals(Double.TYPE) || targetClass.equals(Double.class)) {
  ...
  else if (value instanceof Float)
    // THIS IS NECESSARY DUE TO A BUG/STRANGE BEHAVIOR OF JAVA BY LOSING PRECISION
    return Double.parseDouble(value.toString());

The fix for the comparison path already exists in the codebase, one call away.

Suggested fix

Either of these, or both:

  1. In Type.castComparableNumber(), widen a Float to Double with Double.parseDouble(value.toString()) rather than .doubleValue(), matching what Type.convert() already does, and build the BigDecimal from new BigDecimal(value.toString()) rather than BigDecimal.valueOf(float). This is the narrower change and it fixes every operator at once.

  2. In FloatingPoint.getValue(), stop downcasting a suffix-less literal to float. A literal with no suffix is most naturally a double (the F suffix is there for anyone who wants single precision), and that removes the rounding error at its source. This is the larger change because other code may depend on the current type.

I would expect a regression test for each of: =, IN, >= and < at a literal whose float form rounds up (0.05, 0.1, 0.2, 0.3), <= and > at one that rounds down (0.7), on a DOUBLE column and a DECIMAL column, with and without an index, asserting that the indexed and non-indexed plans return the same rows.

Related

Same family as #5900 (BinaryComparator narrows the wider operand) and #5919 (unchecked numeric narrowing, follow-ups #5905, #5906, #5924), both closed. This one is the parser half rather than the comparator half, and unlike #5900 it is a demonstrated wrong SQL result, not only a broken comparator contract.

Full reproduction source

DecimalLiteralComparisonRepro.java
/*
 * Reproduction: a bare decimal literal on the right-hand side of =, >, >=, <, <=
 * is compared at FLOAT precision against a DOUBLE/DECIMAL property, so the
 * boundary value is lost. BETWEEN, which converts the bound to the property's
 * class first, is correct.
 *
 * Build and run (no Maven needed, jars from the official image):
 *   javac -cp '/home/arcadedb/lib/*' DecimalLiteralComparisonRepro.java
 *   java  -cp '/home/arcadedb/lib/*:.' DecimalLiteralComparisonRepro
 */

import com.arcadedb.Constants;
import com.arcadedb.database.Database;
import com.arcadedb.database.DatabaseFactory;
import com.arcadedb.database.MutableDocument;
import com.arcadedb.query.sql.executor.ResultSet;

import java.io.File;
import java.math.BigDecimal;
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.Comparator;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.stream.Stream;

public class DecimalLiteralComparisonRepro {

  /** ten rows at each of these five discounts, exactly the TPC-H Q6 shape */
  private static final String[] DISCOUNTS = { "0.04", "0.05", "0.06", "0.07", "0.08" };
  private static final int      PER_VALUE = 10;

  private static Database db;
  private static int      failures = 0;

  public static void main(final String[] args) throws Exception {
    final Path dir = Files.createTempDirectory("decimal-literal-repro");
    final String path = dir.resolve("repro").toString();

    System.out.println("ArcadeDB " + Constants.getVersion()
        + "  (build " + Constants.getBuildNumber() + ", branch " + Constants.getBranch()
        + ", " + Constants.getTimestamp() + ")");
    System.out.println("JVM      " + System.getProperty("java.vm.name") + " " + System.getProperty("java.version"));
    System.out.println();

    try (final DatabaseFactory factory = new DatabaseFactory(path)) {
      db = factory.create();
      load();

      System.out.println("Fifty documents, ten each at disc = 0.04, 0.05, 0.06, 0.07, 0.08.");
      System.out.println("The same five values are stored three times, on a DOUBLE, a FLOAT and a DECIMAL property.");
      System.out.println();

      partA();
      partB();
      partC();
      partD();
      partE();

      System.out.println();
      System.out.println(failures == 0
          ? "ALL ROWS CORRECT - the defect did not reproduce."
          : failures + " row(s) above returned the wrong number of records.");
    } finally {
      if (db != null && db.isOpen())
        db.drop();
      delete(dir.toFile());
    }
    System.exit(failures == 0 ? 0 : 1);
  }

  // --------------------------------------------------------------------------------------------

  private static void partA() {
    header("A. DOUBLE property, the shape TPC-H Q6 hits");
    row("disc = 0.05", count("SELECT FROM Li WHERE dDouble = 0.05"), 10);
    row("disc > 0.05", count("SELECT FROM Li WHERE dDouble > 0.05"), 30);
    row("disc >= 0.05", count("SELECT FROM Li WHERE dDouble >= 0.05"), 40);
    row("disc < 0.05", count("SELECT FROM Li WHERE dDouble < 0.05"), 10);
    row("disc <= 0.05", count("SELECT FROM Li WHERE dDouble <= 0.05"), 20);
    row("disc IN [0.05]", count("SELECT FROM Li WHERE dDouble IN [0.05]"), 10);
    row("disc >= 0.05 AND disc <= 0.07", count("SELECT FROM Li WHERE dDouble >= 0.05 AND dDouble <= 0.07"), 30);
    row("disc BETWEEN 0.05 AND 0.07", count("SELECT FROM Li WHERE dDouble BETWEEN 0.05 AND 0.07"), 30);
    System.out.printf("  %-40s %10s %10s%n", "sum(disc) over the >= / <= window",
        fmt(sum("SELECT sum(dDouble) AS s FROM Li WHERE dDouble >= 0.05 AND dDouble <= 0.07")), "1.8");
    System.out.printf("  %-40s %10s %10s%n", "sum(disc) over the BETWEEN window",
        fmt(sum("SELECT sum(dDouble) AS s FROM Li WHERE dDouble BETWEEN 0.05 AND 0.07")), "1.8");
  }

  private static void partB() {
    header("B. The same comparisons on each declared property type, at 0.05");
    for (final String col : new String[] { "dDouble", "dFloat", "dDecimal" }) {
      final String t = col.equals("dDouble") ? "DOUBLE" : col.equals("dFloat") ? "FLOAT" : "DECIMAL";
      row(t + ": = 0.05", count("SELECT FROM Li WHERE " + col + " = 0.05"), 10);
      row(t + ": >= 0.05", count("SELECT FROM Li WHERE " + col + " >= 0.05"), 40);
      row(t + ": <= 0.05", count("SELECT FROM Li WHERE " + col + " <= 0.05"), 20);
      row(t + ": BETWEEN 0.05 AND 0.07", count("SELECT FROM Li WHERE " + col + " BETWEEN 0.05 AND 0.07"), 30);
    }
  }

  private static void partC() {
    header("C. Other literals on the DOUBLE property: which side loses the boundary depends on the literal");
    final Map<String, Integer> below = new LinkedHashMap<>();   // rows strictly below the literal
    below.put("0.05", 10);
    below.put("0.1", 0);
    below.put("0.2", 10);
    below.put("0.3", 20);
    below.put("0.7", 30);
    // Sweep type Sw holds ten rows at each of 0.1 0.2 0.3 0.7 0.8, plus the Li values are untouched.
    for (final String lit : new String[] { "0.1", "0.2", "0.3", "0.7" }) {
      final int lower = below.get(lit);
      row("Sw: = " + lit, count("SELECT FROM Sw WHERE v = " + lit), 10);
      row("Sw: >= " + lit, count("SELECT FROM Sw WHERE v >= " + lit), 50 - lower);
      row("Sw: <= " + lit, count("SELECT FROM Sw WHERE v <= " + lit), lower + 10);
      System.out.printf("      float(%s) as double = %s, double(%s) = %s -> literal is %s the stored value%n",
          lit, Double.toString((double) Float.parseFloat(lit)), lit, Double.toString(Double.parseDouble(lit)),
          (double) Float.parseFloat(lit) > Double.parseDouble(lit) ? "ABOVE" :
              (double) Float.parseFloat(lit) < Double.parseDouble(lit) ? "BELOW" : "EQUAL TO");
    }
  }

  private static void partD() {
    header("D. The same comparisons with a bound parameter, and with an explicit D suffix");
    final Map<String, Object> p = Map.of("d", 0.05d);
    row("dDouble = :d  (Double parameter)", count("SELECT FROM Li WHERE dDouble = :d", p), 10);
    row("dDouble >= :d (Double parameter)", count("SELECT FROM Li WHERE dDouble >= :d", p), 40);
    row("dDouble = 0.05D  (D suffix)", count("SELECT FROM Li WHERE dDouble = 0.05D"), 10);
    row("dDouble >= 0.05D (D suffix)", count("SELECT FROM Li WHERE dDouble >= 0.05D"), 40);
  }

  private static void partE() {
    header("E. The SAME fifty rows on a DOUBLE property that carries a NOTUNIQUE index (type Ix)");
    row("Ix: v = 0.05", count("SELECT FROM Ix WHERE v = 0.05"), 10);
    row("Ix: v > 0.05", count("SELECT FROM Ix WHERE v > 0.05"), 30);
    row("Ix: v >= 0.05", count("SELECT FROM Ix WHERE v >= 0.05"), 40);
    row("Ix: v < 0.05", count("SELECT FROM Ix WHERE v < 0.05"), 10);
    row("Ix: v <= 0.05", count("SELECT FROM Ix WHERE v <= 0.05"), 20);
    row("Ix: v BETWEEN 0.05 AND 0.07", count("SELECT FROM Ix WHERE v BETWEEN 0.05 AND 0.07"), 30);
    System.out.println("      plan for `Ix: v >= 0.05` : " + explain("SELECT FROM Ix WHERE v >= 0.05"));
    System.out.println("      plan for `disc >= 0.05`  : " + explain("SELECT FROM Li WHERE dDouble >= 0.05"));

    System.out.println();
    System.out.println("F. Plain Java, for the arithmetic behind it");
    System.out.println("  " + "-".repeat(41));
    System.out.println("      (double) 0.05f  = " + (double) 0.05f);
    System.out.println("      0.05d           = " + 0.05d);
    System.out.println("      0.05d >= (double) 0.05f ? " + (0.05d >= (double) 0.05f));
  }

  // --------------------------------------------------------------------------------------------

  private static void load() {
    db.command("sql", "CREATE DOCUMENT TYPE Li");
    db.command("sql", "CREATE PROPERTY Li.dDouble DOUBLE");
    db.command("sql", "CREATE PROPERTY Li.dFloat FLOAT");
    db.command("sql", "CREATE PROPERTY Li.dDecimal DECIMAL");

    db.command("sql", "CREATE DOCUMENT TYPE Sw");
    db.command("sql", "CREATE PROPERTY Sw.v DOUBLE");

    db.command("sql", "CREATE DOCUMENT TYPE Ix");
    db.command("sql", "CREATE PROPERTY Ix.v DOUBLE");
    db.command("sql", "CREATE INDEX ON Ix (v) NOTUNIQUE");

    db.transaction(() -> {
      for (final String d : DISCOUNTS)
        for (int i = 0; i < PER_VALUE; i++) {
          final MutableDocument doc = db.newDocument("Li");
          doc.set("dDouble", Double.parseDouble(d));
          doc.set("dFloat", Float.parseFloat(d));
          doc.set("dDecimal", new BigDecimal(d));
          doc.save();
        }
      for (final String d : new String[] { "0.1", "0.2", "0.3", "0.7", "0.8" })
        for (int i = 0; i < PER_VALUE; i++)
          db.newDocument("Sw").set("v", Double.parseDouble(d)).save();
      for (final String d : DISCOUNTS)
        for (int i = 0; i < PER_VALUE; i++)
          db.newDocument("Ix").set("v", Double.parseDouble(d)).save();
    });
  }

  private static long count(final String sql) {
    return count(sql, Map.of());
  }

  private static long count(final String sql, final Map<String, Object> params) {
    long n = 0;
    try (final ResultSet rs = params.isEmpty() ? db.query("sql", sql) : db.query("sql", sql, params)) {
      while (rs.hasNext()) {
        rs.next();
        n++;
      }
    }
    return n;
  }

  private static double sum(final String sql) {
    try (final ResultSet rs = db.query("sql", sql)) {
      if (!rs.hasNext())
        return 0;
      final Object v = rs.next().getProperty("s");
      return v == null ? 0 : ((Number) v).doubleValue();
    }
  }

  private static String fmt(final double d) {
    return String.format("%.4f", d);
  }

  /** first line of the execution plan, so the reader can see whether an index was used */
  private static String explain(final String sql) {
    try (final ResultSet rs = db.query("sql", "EXPLAIN " + sql)) {
      if (!rs.hasNext())
        return "?";
      final Object p = rs.next().getProperty("executionPlanAsString");
      if (p == null)
        return "?";
      for (final String line : p.toString().split("\n"))
        if (line.contains("FETCH") || line.contains("SCAN"))
          return line.trim();
      return p.toString().replace('\n', ' ');
    }
  }

  private static void header(final String s) {
    System.out.println();
    System.out.println(s);
    System.out.println("  " + "-".repeat(s.length()));
    System.out.printf("  %-40s %10s %10s   %s%n", "query", "returned", "expected", "");
  }

  private static void row(final String label, final long actual, final long expected) {
    final boolean ok = actual == expected;
    if (!ok)
      failures++;
    System.out.printf("  %-40s %10d %10d   %s%n", label, actual, expected, ok ? "ok" : "WRONG");
  }

  private static void delete(final File f) {
    if (f.isDirectory())
      try (final Stream<Path> s = Files.list(f.toPath())) {
        s.sorted(Comparator.reverseOrder()).map(Path::toFile).forEach(DecimalLiteralComparisonRepro::delete);
      } catch (final Exception ignore) {
        // best effort
      }
    f.delete();
  }
}

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

bugengineCore engine: document/schema APIsql

Type

No type

Projects

No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions