Skip to content

Commit

Permalink
Merge pull request #3985 from katzyn/nan
Browse files Browse the repository at this point in the history
Reject invalid ASIN and ACOS arguments and fix PK index for other numeric data types
  • Loading branch information
katzyn committed Jan 26, 2024
2 parents caa35aa + 52d0501 commit 6f727b5
Show file tree
Hide file tree
Showing 12 changed files with 911 additions and 59 deletions.
2 changes: 2 additions & 0 deletions h2/src/docsrc/html/changelog.html
Expand Up @@ -21,6 +21,8 @@ <h1>Change Log</h1>

<h2>Next Version (unreleased)</h2>
<ul>
<li>Issue #3981: Unexpected result when using trigonometric functions
</li>
<li>Issue #3983: INVISIBLE columns should be ignored in INSERT statement without explicit column list
</li>
<li>Issue #3960: NullPointerException when executing batch insert
Expand Down
6 changes: 6 additions & 0 deletions h2/src/main/org/h2/expression/function/MathFunction1.java
Expand Up @@ -156,9 +156,15 @@ public Value getValue(SessionLocal session) {
d = Math.tanh(d);
break;
case ASIN:
if (d < -1d || d > 1d) {
throw DbException.getInvalidValueException("ASIN() argument", d);
}
d = Math.asin(d);
break;
case ACOS:
if (d < -1d || d > 1d) {
throw DbException.getInvalidValueException("ACOS() argument", d);
}
d = Math.acos(d);
break;
case ATAN:
Expand Down
5 changes: 3 additions & 2 deletions h2/src/main/org/h2/index/RangeIndex.java
Expand Up @@ -95,8 +95,9 @@ public Cursor findFirstOrLast(SessionLocal session, boolean first) {
if (step == 0L) {
throw DbException.get(ErrorCode.STEP_SIZE_MUST_NOT_BE_ZERO);
}
return new SingleRowCursor((step > 0 ? min <= max : min >= max)
? Row.get(new Value[]{ ValueBigint.get(first ^ min >= max ? min : max) }, 1) : null);
return (step > 0 ? min <= max : min >= max)
? new SingleRowCursor(Row.get(new Value[] { ValueBigint.get(first ^ min >= max ? min : max) }, 1))
: SingleRowCursor.EMPTY;
}

@Override
Expand Down
6 changes: 6 additions & 0 deletions h2/src/main/org/h2/index/SingleRowCursor.java
Expand Up @@ -13,6 +13,12 @@
* A cursor with at most one row.
*/
public class SingleRowCursor implements Cursor {

/**
* An empty cursor.
*/
public static final SingleRowCursor EMPTY = new SingleRowCursor(null);

private Row row;
private boolean end;

Expand Down
124 changes: 94 additions & 30 deletions h2/src/main/org/h2/mvstore/db/MVPrimaryIndex.java
Expand Up @@ -5,6 +5,7 @@
*/
package org.h2.mvstore.db;

import java.math.BigDecimal;
import java.util.List;
import java.util.Map.Entry;
import java.util.concurrent.atomic.AtomicLong;
Expand All @@ -29,8 +30,8 @@
import org.h2.table.IndexColumn;
import org.h2.table.TableFilter;
import org.h2.value.Value;
import org.h2.value.ValueDecfloat;
import org.h2.value.ValueLob;
import org.h2.value.ValueNull;
import org.h2.value.VersionedValue;

/**
Expand Down Expand Up @@ -230,31 +231,101 @@ private Row lockRow(TransactionMap<Long,SearchRow> map, long key, int timeoutMil

@Override
public Cursor find(SessionLocal session, SearchRow first, SearchRow last) {
long min = extractPKFromRow(first, Long.MIN_VALUE);
long max = extractPKFromRow(last, Long.MAX_VALUE);
return find(session, min, max);
}

private long extractPKFromRow(SearchRow row, long defaultValue) {
long result;
if (row == null) {
result = defaultValue;
} else if (mainIndexColumn == SearchRow.ROWID_INDEX) {
result = row.getKey();
Long min, max;
Value v;
if (first == null) {
min = null;
} else if (mainIndexColumn == SearchRow.ROWID_INDEX || (v = first.getValue(mainIndexColumn)) == null) {
min = first.getKey();
} else {
Value v = row.getValue(mainIndexColumn);
if (v == null) {
result = row.getKey();
} else if (v == ValueNull.INSTANCE) {
result = 0L;
} else {
result = v.getLong();
switch (v.getValueType()) {
case Value.NULL:
return SingleRowCursor.EMPTY;
case Value.REAL:
case Value.DOUBLE: {
double d = v.getDouble();
if (Double.isNaN(d)) {
return SingleRowCursor.EMPTY;
} else {
min = (long) d;
}
break;
}
case Value.DECFLOAT:
if (!((ValueDecfloat) v).isFinite()) {
if (v == ValueDecfloat.NEGATIVE_INFINITY) {
min = null;
} else {
return SingleRowCursor.EMPTY;
}
break;
}
//$FALL-THROUGH$
case Value.NUMERIC: {
BigDecimal bd = v.getBigDecimal();
if (bd.compareTo(Value.MAX_LONG_DECIMAL) > 0) {
return SingleRowCursor.EMPTY;
} else if (bd.compareTo(Value.MIN_LONG_DECIMAL) < 0) {
min = null;
} else {
min = bd.longValue();
}
break;
}
default:
min = v.getLong();
}
}
return result;
if (last == null) {
max = null;
} else if (mainIndexColumn == SearchRow.ROWID_INDEX || (v = last.getValue(mainIndexColumn)) == null) {
max = last.getKey();
} else {
switch (v.getValueType()) {
case Value.NULL:
return SingleRowCursor.EMPTY;
case Value.REAL:
case Value.DOUBLE: {
double d = v.getDouble();
if (Double.isNaN(d)) {
max = null;
} else {
max = (long) d;
}
break;
}
case Value.DECFLOAT:
if (!((ValueDecfloat) v).isFinite()) {
if (v == ValueDecfloat.NEGATIVE_INFINITY) {
return SingleRowCursor.EMPTY;
} else {
max = null;
}
break;
}
//$FALL-THROUGH$
case Value.NUMERIC: {
BigDecimal bd = v.getBigDecimal();
if (bd.compareTo(Value.MAX_LONG_DECIMAL) > 0) {
max = null;
} else if (bd.compareTo(Value.MIN_LONG_DECIMAL) < 0) {
return SingleRowCursor.EMPTY;
} else {
max = bd.longValue();
}
break;
}
default:
max = v.getLong();
}
}
TransactionMap<Long,SearchRow> map = getMap(session);
if (min != null && max != null && min.longValue() == max.longValue()) {
return new SingleRowCursor(setRowKey((Row) map.getFromSnapshot(min), min));
}
return new MVStoreCursor(map.entryIterator(min, max));
}


@Override
public MVTable getTable() {
return mvTable;
Expand Down Expand Up @@ -319,7 +390,8 @@ public boolean canGetFirstOrLast() {
public Cursor findFirstOrLast(SessionLocal session, boolean first) {
TransactionMap<Long, SearchRow> map = getMap(session);
Entry<Long, SearchRow> entry = first ? map.firstEntry() : map.lastEntry();
return new SingleRowCursor(entry != null ? setRowKey((Row) entry.getValue(), entry.getKey()) : null);
return entry != null ? new SingleRowCursor(setRowKey((Row) entry.getValue(), entry.getKey()))
: SingleRowCursor.EMPTY;
}

@Override
Expand Down Expand Up @@ -360,14 +432,6 @@ public void addBufferedRows(List<String> bufferNames) {
throw new UnsupportedOperationException();
}

private Cursor find(SessionLocal session, Long first, Long last) {
TransactionMap<Long,SearchRow> map = getMap(session);
if (first != null && last != null && first.longValue() == last.longValue()) {
return new SingleRowCursor(setRowKey((Row) map.getFromSnapshot(first), first));
}
return new MVStoreCursor(map.entryIterator(first, last));
}

@Override
public boolean isRowIdIndex() {
return true;
Expand Down
2 changes: 1 addition & 1 deletion h2/src/main/org/h2/mvstore/db/MVSecondaryIndex.java
Expand Up @@ -334,7 +334,7 @@ public Cursor findFirstOrLast(SessionLocal session, boolean first) {
return new SingleRowCursor(mvTable.getRow(session, key.getKey()));
}
}
return new SingleRowCursor(null);
return SingleRowCursor.EMPTY;
}

@Override
Expand Down
48 changes: 26 additions & 22 deletions h2/src/main/org/h2/res/help.csv
Expand Up @@ -4845,8 +4845,10 @@ ABS(CAST(I AS BIGINT))
ACOS(numeric)
","
Calculate the arc cosine.
See also Java ""Math.acos"".
This method returns a double.

Argument must be between -1 and 1 inclusive.

This function returns a double precision value.
","
ACOS(D)
"
Expand All @@ -4855,8 +4857,10 @@ ACOS(D)
ASIN(numeric)
","
Calculate the arc sine.
See also Java ""Math.asin"".
This method returns a double.

Argument must be between -1 and 1 inclusive.

This function returns a double precision value.
","
ASIN(D)
"
Expand All @@ -4865,8 +4869,8 @@ ASIN(D)
ATAN(numeric)
","
Calculate the arc tangent.
See also Java ""Math.atan"".
This method returns a double.

This function returns a double precision value.
","
ATAN(D)
"
Expand All @@ -4875,8 +4879,8 @@ ATAN(D)
COS(numeric)
","
Calculate the trigonometric cosine.
See also Java ""Math.cos"".
This method returns a double.

This function returns a double precision value.
","
COS(ANGLE)
"
Expand All @@ -4885,8 +4889,8 @@ COS(ANGLE)
COSH(numeric)
","
Calculate the hyperbolic cosine.
See also Java ""Math.cosh"".
This method returns a double.

This function returns a double precision value.
","
COSH(X)
"
Expand All @@ -4895,8 +4899,8 @@ COSH(X)
@h2@ COT(numeric)
","
Calculate the trigonometric cotangent (""1/TAN(ANGLE)"").
See also Java ""Math.*"" functions.
This method returns a double.

This function returns a double precision value.
","
COT(ANGLE)
"
Expand All @@ -4905,8 +4909,8 @@ COT(ANGLE)
SIN(numeric)
","
Calculate the trigonometric sine.
See also Java ""Math.sin"".
This method returns a double.

This function returns a double precision value.
","
SIN(ANGLE)
"
Expand All @@ -4915,8 +4919,8 @@ SIN(ANGLE)
SINH(numeric)
","
Calculate the hyperbolic sine.
See also Java ""Math.sinh"".
This method returns a double.

This function returns a double precision value.
","
SINH(ANGLE)
"
Expand All @@ -4925,8 +4929,8 @@ SINH(ANGLE)
TAN(numeric)
","
Calculate the trigonometric tangent.
See also Java ""Math.tan"".
This method returns a double.

This function returns a double precision value.
","
TAN(ANGLE)
"
Expand All @@ -4935,8 +4939,8 @@ TAN(ANGLE)
TANH(numeric)
","
Calculate the hyperbolic tangent.
See also Java ""Math.tanh"".
This method returns a double.

This function returns a double precision value.
","
TANH(X)
"
Expand All @@ -4945,8 +4949,8 @@ TANH(X)
@h2@ ATAN2(numeric, numeric)
","
Calculate the angle when converting the rectangular coordinates to polar coordinates.
See also Java ""Math.atan2"".
This method returns a double.

This function returns a double precision value.
","
ATAN2(X, Y)
"
Expand Down
7 changes: 5 additions & 2 deletions h2/src/main/org/h2/value/Value.java
Expand Up @@ -378,10 +378,13 @@ public abstract class Value extends VersionedValue<Value> implements HasSQL, Typ

private static SoftReference<Value[]> softCache;

static final BigDecimal MAX_LONG_DECIMAL = BigDecimal.valueOf(Long.MAX_VALUE);
/**
* The largest BIGINT value, as a BigDecimal.
*/
public static final BigDecimal MAX_LONG_DECIMAL = BigDecimal.valueOf(Long.MAX_VALUE);

/**
* The smallest Long value, as a BigDecimal.
* The smallest BIGINT value, as a BigDecimal.
*/
public static final BigDecimal MIN_LONG_DECIMAL = BigDecimal.valueOf(Long.MIN_VALUE);

Expand Down
4 changes: 2 additions & 2 deletions h2/src/test/org/h2/test/db/TestTableEngines.java
Expand Up @@ -1007,8 +1007,8 @@ public boolean canGetFirstOrLast() {

@Override
public Cursor findFirstOrLast(SessionLocal session, boolean first) {
return new SingleRowCursor((Row)
(set.isEmpty() ? null : first ? set.first() : set.last()));
return set.isEmpty() ? SingleRowCursor.EMPTY
: new SingleRowCursor((Row) (first ? set.first() : set.last()));
}

@Override
Expand Down
9 changes: 9 additions & 0 deletions h2/src/test/org/h2/test/scripts/functions/numeric/acos.sql
Expand Up @@ -8,3 +8,12 @@ select acos(null) vn, acos(-1) r1;
> ---- -----------------
> null 3.141592653589793
> rows: 1

SELECT ACOS(-1.1);
> exception INVALID_VALUE_2

SELECT ACOS(1.1);
> exception INVALID_VALUE_2

SELECT ACOS(CAST('Infinity' AS DOUBLE PRECISION));
> exception INVALID_VALUE_2

0 comments on commit 6f727b5

Please sign in to comment.