Skip to content

Commit

Permalink
Range finding methods should operate on a double type.
Browse files Browse the repository at this point in the history
  • Loading branch information
@broneill
broneill committed Sep 5, 2024
1 parent 417a7f2 commit ee11dec
Show file tree
Hide file tree
Showing 2 changed files with 102 additions and 42 deletions.
98 changes: 69 additions & 29 deletions src/main/java/org/cojen/tupl/table/expr/WindowBuffer.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
import org.cojen.maker.MethodMaker;
import org.cojen.maker.Variable;

import org.cojen.tupl.table.ColumnInfo;
import org.cojen.tupl.table.CompareUtils;
import org.cojen.tupl.table.Converter;

Expand Down Expand Up @@ -201,7 +202,7 @@ public static int clampCapacity(long capacity) {
* @return a frame position relative to the current row (which is zero), or MAX_VALUE if
* out of bounds (effective range is empty)
*/
public abstract long findRangeStartAsc(V delta, long startPos);
public abstract long findRangeStartAsc(double delta, long startPos);

/**
* Finds a frame row position whose value matches the current row value plus a delta value.
Expand All @@ -217,7 +218,7 @@ public static int clampCapacity(long capacity) {
* @return a frame position relative to the current row (which is zero), or MIN_VALUE if
* out of bounds (effective range is empty)
*/
public abstract long findRangeEndAsc(V delta, long endPos);
public abstract long findRangeEndAsc(double delta, long endPos);

/**
* Finds a frame row position whose value matches the current row value plus a delta value.
Expand All @@ -233,7 +234,7 @@ public static int clampCapacity(long capacity) {
* @return a frame position relative to the current row (which is zero), or MAX_VALUE if
* out of bounds (effective range is empty)
*/
public abstract long findRangeStartDesc(V delta, long startPos);
public abstract long findRangeStartDesc(double delta, long startPos);

/**
* Finds a frame row position whose value matches the current row value plus a delta value.
Expand All @@ -249,7 +250,7 @@ public static int clampCapacity(long capacity) {
* @return a frame position relative to the current row (which is zero), or MIN_VALUE if
* out of bounds (effective range is empty)
*/
public abstract long findRangeEndDesc(V delta, long endPos);
public abstract long findRangeEndDesc(double delta, long endPos);

/**
* Returns the number of non-null values which are available over the given range.
Expand Down Expand Up @@ -803,9 +804,9 @@ private static void makeFindGroupCompare(Variable a, Variable b, Label equal) {
private static void makeFindRange(Type type, ClassMaker cm,
boolean forStart, boolean ascending)
{
Class clazz = type.clazz();
String name = "findRange" + (forStart ? "Start" : "End") + (ascending ? "Asc" : "Desc");
MethodMaker mm = cm.addMethod(long.class, name, clazz, long.class).public_().final_();
MethodMaker mm = cm.addMethod(long.class, name, double.class, long.class);
mm.public_().final_();

var deltaVar = mm.param(0);
var posVar = mm.param(1);
Expand Down Expand Up @@ -836,19 +837,29 @@ private static void makeFindRange(Type type, ClassMaker cm,
return index + start;
*/

int cmpOp = forStart ? (ascending ? OP_LT : OP_GT) : (ascending ? OP_LE : OP_GE);

var startVar = mm.field("start");
var indexVar = mm.var(long.class).set(posVar.sub(startVar));

var zeroVar = mm.var(clazz);
Arithmetic.zero(zeroVar);
//int deltaOp = forStart ? (ascending ? OP_GT : OP_LT) : (ascending ? OP_GE : OP_LE);

Label reverse = mm.label();
Label forward = mm.label();
int deltaOp = forStart ? (ascending ? OP_GT : OP_LT) : (ascending ? OP_GE : OP_LE);
CompareUtils.compare(mm, type, deltaVar, type, zeroVar, deltaOp, forward, reverse);
forward.here();

if (forStart) {
if (ascending) {
deltaVar.ifLe(0, reverse);
} else {
deltaVar.ifGe(0, reverse);
}
} else {
if (ascending) {
deltaVar.ifLt(0, reverse);
} else {
deltaVar.ifGt(0, reverse);
}
}

int cmpOp = forStart ? (ascending ? OP_LT : OP_GT) : (ascending ? OP_LE : OP_GE);

makeFindRangeLoop(type, mm, deltaVar, indexVar, true, flipOperator(cmpOp));
Label cont = mm.label().goto_();
reverse.here();
Expand All @@ -858,20 +869,25 @@ private static void makeFindRange(Type type, ClassMaker cm,
mm.return_(indexVar.add(startVar));
}

/**
* @param deltaVar variable of double type
*/
private static void makeFindRangeLoop(Type type, MethodMaker mm,
Variable deltaVar, Variable indexVar,
boolean forward, int cmpOp)
{
Label done = mm.label();

var doubleType = doubleType(type);

if (forward) {
/*
long endIndex = end - start;
if (index < endIndex) {
V find = get((int) index) + delta;
double find = get((int) index) + delta;
do {
index++;
V value = get((int) index);
double value = get((int) index);
if (value <cmpOp> find) {
index--; // only when cmpOp doesn't have '='
goto done;
Expand All @@ -885,15 +901,15 @@ private static void makeFindRangeLoop(Type type, MethodMaker mm,
Label loopEnd = mm.label();
indexVar.ifGe(endIndexVar, loopEnd);

var findVar = makeFindVar(type, indexVar, deltaVar);
var findVar = makeFindVar(type, doubleType, indexVar, deltaVar);

Label doStart = mm.label().here();
indexVar.inc(1);
var valueVar = mm.invoke("get", indexVar.cast(int.class));
var valueVar = convert(type, mm.invoke("get", indexVar.cast(int.class)), doubleType);

Label stop = mm.label();
Label cont = mm.label();
CompareUtils.compare(mm, type, valueVar, type, findVar, cmpOp, stop, cont);
CompareUtils.compare(mm, doubleType, valueVar, doubleType, findVar, cmpOp, stop, cont);
stop.here();

if (!hasEqualComponent(cmpOp)) {
Expand All @@ -912,10 +928,10 @@ private static void makeFindRangeLoop(Type type, MethodMaker mm,
} else {
/*
if (index > 0) {
V find = get((int) index) + delta;
double find = get((int) index) + delta;
do {
index--;
V value = get((int) index);
double value = get((int) index);
if (value <cmpOp> find) {
index++; // only when cmpOp doesn't have '='
goto done;
Expand All @@ -928,15 +944,15 @@ private static void makeFindRangeLoop(Type type, MethodMaker mm,
Label loopEnd = mm.label();
indexVar.ifLe(0, loopEnd);

var findVar = makeFindVar(type, indexVar, deltaVar);
var findVar = makeFindVar(type, doubleType, indexVar, deltaVar);

Label doStart = mm.label().here();
indexVar.dec(1);
var valueVar = mm.invoke("get", indexVar.cast(int.class));
var valueVar = convert(type, mm.invoke("get", indexVar.cast(int.class)), doubleType);

Label stop = mm.label();
Label cont = mm.label();
CompareUtils.compare(mm, type, valueVar, type, findVar, cmpOp, stop, cont);
CompareUtils.compare(mm, doubleType, valueVar, doubleType, findVar, cmpOp, stop, cont);
stop.here();

if (!hasEqualComponent(cmpOp)) {
Expand All @@ -957,22 +973,46 @@ private static void makeFindRangeLoop(Type type, MethodMaker mm,
done.here();
}

private static Variable makeFindVar(Type type, Variable indexVar, Variable deltaVar) {
/**
* @param deltaVar variable of double type
* @return variable of type double or Double
*/
private static Variable makeFindVar(Type type, Type doubleType,
Variable indexVar, Variable deltaVar)
{
MethodMaker mm = indexVar.methodMaker();
var findVar = mm.invoke("get", indexVar.cast(int.class));
final var findVar = convert(type, mm.invoke("get", indexVar.cast(int.class)), doubleType);

if (findVar.classType().isPrimitive()) {
findVar = Arithmetic.eval(type, Token.T_PLUS, findVar, deltaVar);
if (!doubleType.isNullable()) {
findVar.set(findVar.add(deltaVar));
} else {
Label skip = mm.label();
findVar.ifEq(null, skip);
findVar.set(Arithmetic.eval(type, Token.T_PLUS, findVar, deltaVar));
findVar.set(findVar.add(deltaVar));
skip.here();
}

return findVar;
}

private static Variable convert(ColumnInfo srcInfo, Variable srcVar, Type dstType) {
MethodMaker mm = srcVar.methodMaker();
Variable dstVar = mm.var(dstType.clazz());
Converter.convertLossy(mm, srcInfo, srcVar, dstType, dstVar);
return dstVar;
}

/**
* Returns a double or nullable Double type.
*/
private static Type doubleType(ColumnInfo info) {
if (info.isNullable()) {
return BasicType.make(Double.class, Type.TYPE_DOUBLE | Type.TYPE_NULLABLE);
} else {
return BasicType.make(double.class, Type.TYPE_DOUBLE);
}
}

private static void addNumericalMethods(ClassMaker cm, Type type) {
Class clazz = type.clazz();
Class unboxed = type.unboxedType();
Expand Down
46 changes: 33 additions & 13 deletions src/main/java/org/cojen/tupl/table/expr/WindowFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,9 +103,9 @@ private String orderStr() {
// True if the range start/end is a compile-time or run-time constant.
private boolean mIsStartConstant, mIsEndConstant;

// Is set if the range start/end is a compile-time constant.
// FIXME: Type might be different when using MODE_RANGE.
private Long mStartConstant, mEndConstant;
// Is set if the range start/end is a compile-time constant. Type is Long for MODE_ROWS and
// MODE_GROUPS, and is Double for MODE_RANGE.
private Number mStartConstant, mEndConstant;

private int mMode;

Expand Down Expand Up @@ -168,19 +168,19 @@ public void init(GroupContext context) {
mIsStartConstant = true;
mIsEndConstant = true;
var range = (Range) rangeVal.constantValue();
mStartConstant = range.start_long();
mEndConstant = range.end_long();
mStartConstant = start(range);
mEndConstant = end(range);
} else if (rangeVal.expr() instanceof RangeExpr re) {
if (re.start().isConstant()) {
mIsStartConstant = true;
if (re.start() instanceof ConstantExpr c) {
mStartConstant = Range.make((Number) c.value(), null).start_long();
mStartConstant = start(Range.make((Number) c.value(), null));
}
}
if (re.end().isConstant()) {
mIsEndConstant = true;
if (re.end() instanceof ConstantExpr c) {
mEndConstant = Range.make(null, (Number) c.value()).end_long();
mEndConstant = end(Range.make(null, (Number) c.value()));
}
}
}
Expand Down Expand Up @@ -226,7 +226,7 @@ public void init(GroupContext context) {
mm.field(mBufferFieldName).set(mm.new_(bufferType, DEFAULT_CAPACITY));
} else if (mStartConstant != null && mEndConstant != null) {
bufferFieldMaker.final_();
int capacity = WindowBuffer.capacityFor(mStartConstant, mEndConstant);
int capacity = WindowBuffer.capacityFor((Long) mStartConstant, (Long) mEndConstant);
if (mMode == MODE_GROUPS) {
capacity = WindowBuffer.clampCapacity(capacity * (long) GROUP_SIZE);
}
Expand All @@ -241,7 +241,7 @@ public void init(GroupContext context) {
if (mMode != MODE_GROUPS || !mIsEndConstant) {
checkField.set(DEFAULT_CAPACITY);
} else if (mEndConstant != null) {
checkField.set(WindowBuffer.capacityForGroup(GROUP_SIZE, mEndConstant));
checkField.set(WindowBuffer.capacityForGroup(GROUP_SIZE, (Long) mEndConstant));
} else {
var capacityVar = mm.var(WindowBuffer.class)
.invoke("capacityFor", GROUP_SIZE, mm.field(mEndFieldName));
Expand Down Expand Up @@ -337,7 +337,7 @@ public void check(GroupContext context) {
remainingVar.ifGt(0L, ready);

if (!isOpenEnd()) {
if (mEndConstant == null || mEndConstant < 0) {
if (mEndConstant == null || ((Number) mEndConstant).doubleValue() < 0) {
// The current row isn't guaranteed to be included, so prevent the remaining
// amount from decrementing below zero. This check isn't necessary when the end
// is known to always include the current row because then the buffer ready
Expand Down Expand Up @@ -466,7 +466,7 @@ public Variable step(GroupContext context) {
} else if (mMode != MODE_ROWS) {
bufferVar.invoke("trimStart", start);
bufferVar.invoke("advance");
} else if (mStartConstant != null && mStartConstant >= 0) {
} else if (mStartConstant != null && ((Number) mStartConstant).doubleValue() >= 0) {
bufferVar.invoke("advanceAndRemove");
} else {
bufferVar.invoke("advanceAndRemove", start);
Expand Down Expand Up @@ -505,12 +505,32 @@ protected Class<?> bufferType() {
*/
protected abstract Variable compute(Variable bufferVar, Object frameStart, Object frameEnd);

private Number start(Range range) {
if (mFrame.mode() == MODE_RANGE) {
return range.start_double();
} else {
return range.start_long();
}
}

private Number end(Range range) {
if (mFrame.mode() == MODE_RANGE) {
return range.end_double();
} else {
return range.end_long();
}
}

private boolean isOpenStart() {
return mStartConstant != null && mStartConstant == Long.MIN_VALUE;
return mStartConstant != null &&
(mStartConstant == (Number) Long.MIN_VALUE ||
mStartConstant == (Number) Double.NEGATIVE_INFINITY);
}

private boolean isOpenEnd() {
return mEndConstant != null && mEndConstant == Long.MAX_VALUE;
return mEndConstant != null &&
(mEndConstant == (Number) Long.MAX_VALUE ||
mEndConstant == (Number) Double.POSITIVE_INFINITY);
}

private boolean isStartVariable() {
Expand Down

0 comments on commit ee11dec

Please sign in to comment.