diff --git a/DB/src/main/java/io/deephaven/db/v2/select/DhFormulaColumn.java b/DB/src/main/java/io/deephaven/db/v2/select/DhFormulaColumn.java index f52a9352006..69353cac5ce 100644 --- a/DB/src/main/java/io/deephaven/db/v2/select/DhFormulaColumn.java +++ b/DB/src/main/java/io/deephaven/db/v2/select/DhFormulaColumn.java @@ -322,8 +322,18 @@ private CodeGenerator generateAppropriateGetMethod(TypeAnalyzer ta, boolean useP final CodeGenerator g = CodeGenerator.create( "@Override", "public [[RETURN_TYPE]] [[GETTER_NAME]](final long k)", CodeGenerator.block( - CodeGenerator.optional("maybeCreateI", "final int i = __intSize(__index.find(k));"), - CodeGenerator.optional("maybeCreateII", "final long ii = __index.find(k);"), + (usePrev + ? CodeGenerator.optional("maybeCreateIorII", + "final long findResult;", + "try (final Index prev = __index.getPrevIndex())", CodeGenerator.block( + "findResult = prev.find(k);" + )) + : CodeGenerator.optional("maybeCreateIorII", + "final long findResult = __index.find(k);")), + CodeGenerator.optional("maybeCreateI", + "final int i = __intSize(findResult);"), + CodeGenerator.optional("maybeCreateII", + "final long ii = findResult;"), CodeGenerator.repeated("cacheColumnSourceGet", "final [[TYPE]] [[VAR]] = [[GET_EXPRESSION]];"), "if ([[LAZY_RESULT_CACHE_NAME]] != null)", CodeGenerator.block( "final Object __lazyKey = [[C14NUTIL_CLASSNAME]].maybeMakeSmartKey([[FORMULA_ARGS]]);", @@ -345,6 +355,9 @@ private CodeGenerator generateAppropriateGetMethod(TypeAnalyzer ta, boolean useP g.replace("RESULT_TYPE", resultTypeString); g.replace("GETTER_NAME", getterName); + if (usesI || usesII) { + g.activateOptional("maybeCreateIorII"); + } if (usesI) { g.activateOptional("maybeCreateI"); } @@ -472,7 +485,7 @@ private CodeGenerator generateFillChunk(boolean usePrev) { "final [[CHUNK_TYPE]] __chunk__col__[[COL_SOURCE_NAME]] = this.[[COL_SOURCE_NAME]].[[GET_CURR_OR_PREV_CHUNK]](" + "__typedContext.__subContext[[COL_SOURCE_NAME]], __orderedKeys).[[AS_CHUNK_METHOD]]();" ), - "fillChunkHelper(__typedContext, __destination, __orderedKeys[[ADDITIONAL_CHUNK_ARGS]]);" + "fillChunkHelper(" + Boolean.toString(usePrev) + ", __typedContext, __destination, __orderedKeys[[ADDITIONAL_CHUNK_ARGS]]);" ) ); @@ -497,17 +510,22 @@ private CodeGenerator generateFillChunk(boolean usePrev) { @NotNull private CodeGenerator generateFillChunkHelper(TypeAnalyzer ta) { final CodeGenerator g = CodeGenerator.create( - "private void fillChunkHelper(final FormulaFillContext __context,", CodeGenerator.indent( + "private void fillChunkHelper(final boolean __usePrev, final FormulaFillContext __context,", CodeGenerator.indent( "final WritableChunk __destination,", "final OrderedKeys __orderedKeys[[ADDITIONAL_CHUNK_ARGS]])"), CodeGenerator.block( "final [[DEST_CHUNK_TYPE]] __typedDestination = __destination.[[DEST_AS_CHUNK_METHOD]]();", - CodeGenerator.optional("maybeCreateI", - "__context.__iChunk.setSize(0);", - "__index.invert(__orderedKeys.asIndex()).forAllLongs(l -> __context.__iChunk.add(__intSize(l)));" - ), - CodeGenerator.optional("maybeCreateII", - "__index.invert(__orderedKeys.asIndex()).fillKeyIndicesChunk(__context.__iiChunk);" - ), + CodeGenerator.optional("maybeCreateIOrII", + "try (final Index prev = __usePrev ? __index.getPrevIndex() : null;", CodeGenerator.indent( + "final Index inverted = ((prev != null) ? prev : __index).invert(__orderedKeys.asIndex()))"), CodeGenerator.block( + CodeGenerator.optional("maybeCreateI", + "__context.__iChunk.setSize(0);", + "inverted.forAllLongs(l -> __context.__iChunk.add(__intSize(l)));" + ), + CodeGenerator.optional("maybeCreateII", + "inverted.fillKeyIndicesChunk(__context.__iiChunk);" + ) + ) + ), CodeGenerator.repeated("getChunks", "final [[CHUNK_TYPE]] __chunk__col__[[COL_SOURCE_NAME]] = __sources[[[SOURCE_INDEX]]].[[AS_CHUNK_METHOD]]();" ), @@ -547,6 +565,9 @@ private CodeGenerator generateFillChunkHelper(TypeAnalyzer ta) { null); final String additionalChunkArgs = chunkArgs.isEmpty() ? "" : ", " + makeCommaSeparatedList(chunkArgs); g.replace("ADDITIONAL_CHUNK_ARGS", additionalChunkArgs); + if (usesI || usesII) { + g.activateOptional("maybeCreateIOrII"); + } if (usesI) { g.activateAllOptionals("maybeCreateI"); } diff --git a/DB/src/main/java/io/deephaven/db/v2/utils/codegen/CodeGenerator.java b/DB/src/main/java/io/deephaven/db/v2/utils/codegen/CodeGenerator.java index 9c68786a9f4..932d3db19d2 100644 --- a/DB/src/main/java/io/deephaven/db/v2/utils/codegen/CodeGenerator.java +++ b/DB/src/main/java/io/deephaven/db/v2/utils/codegen/CodeGenerator.java @@ -371,7 +371,7 @@ final class Optional extends CodeGenerator { this(tag, inner, false); } - Optional(String tag, CodeGenerator inner, boolean active) { + private Optional(String tag, CodeGenerator inner, boolean active) { this.tag = tag; this.inner = inner; this.active = active; diff --git a/DB/src/test/java/io/deephaven/db/v2/QueryTableTest.java b/DB/src/test/java/io/deephaven/db/v2/QueryTableTest.java index bc848a6663a..a795de626e6 100644 --- a/DB/src/test/java/io/deephaven/db/v2/QueryTableTest.java +++ b/DB/src/test/java/io/deephaven/db/v2/QueryTableTest.java @@ -39,6 +39,7 @@ import junit.framework.TestCase; import org.apache.commons.lang3.mutable.MutableObject; import org.junit.Assert; +import org.junit.Test; import java.io.File; import java.io.IOException; @@ -2861,4 +2862,30 @@ public void testNotifyListenersReleasesUpdateShiftAwareChildListener() { Assert.assertNull(update.added); } + + public void testRegressionIssue544() { + // The expression that fails in the console is: + // x = merge(newTable(byteCol("Q", (byte)0)), timeTable("00:00:01").view("Q=(byte)(i%2)")) + // .tail(1) + // .view("Q=Q*i") + // .sumBy() + // + // The exception we were getting was: java.lang.IllegalArgumentException: keys argument has elements not in the index + // + final Table t0 = newTable(byteCol("Q", (byte) 0)); + final QueryTable t1 = TstUtils.testRefreshingTable(i(), intCol("T")); + final Table t2 = t1.view("Q=(byte)(i%2)"); + final Table result = merge(t0, t2).tail(1).view("Q=Q*i").sumBy(); + int i = 1; + for (int step = 0; step < 2; ++step) { + final int key = i++; + LiveTableMonitor.DEFAULT.runWithinUnitTestCycle(() -> { + final Index addIndex = i(key); + addToTable(t1, addIndex, intCol("T", key)); + t1.notifyListeners(addIndex, i(), i()); + TableTools.show(result); + }); + } + assertEquals(0, getUpdateErrors().size()); + } } diff --git a/DB/src/test/java/io/deephaven/db/v2/select/FormulaSample.java b/DB/src/test/java/io/deephaven/db/v2/select/FormulaSample.java index fd0cf66b60a..9284ac6b347 100644 --- a/DB/src/test/java/io/deephaven/db/v2/select/FormulaSample.java +++ b/DB/src/test/java/io/deephaven/db/v2/select/FormulaSample.java @@ -98,8 +98,9 @@ public FormulaSample(final Index index, @Override public long getLong(final long k) { - final int i = __intSize(__index.find(k)); - final long ii = __index.find(k); + final long findResult = __index.find(k); + final int i = __intSize(findResult); + final long ii = findResult; final long __temp0 = II.getLong(k); final int __temp1 = I.getInt(k); if (__lazyResultCache != null) { @@ -111,8 +112,12 @@ public long getLong(final long k) { @Override public long getPrevLong(final long k) { - final int i = __intSize(__index.find(k)); - final long ii = __index.find(k); + final long findResult; + try (final Index prev = __index.getPrevIndex()) { + findResult = prev.find(k); + } + final int i = __intSize(findResult); + final long ii = findResult; final long __temp0 = II.getPrevLong(k); final int __temp1 = I.getPrevInt(k); if (__lazyResultCache != null) { @@ -142,7 +147,7 @@ public void fillChunk(final FillContext __context, final WritableChunk __chunk__col__II = this.II.getChunk(__typedContext.__subContextII, __orderedKeys).asLongChunk(); final IntChunk __chunk__col__I = this.I.getChunk(__typedContext.__subContextI, __orderedKeys).asIntChunk(); - fillChunkHelper(__typedContext, __destination, __orderedKeys, __chunk__col__II, __chunk__col__I); + fillChunkHelper(false, __typedContext, __destination, __orderedKeys, __chunk__col__II, __chunk__col__I); } @Override @@ -150,16 +155,19 @@ public void fillPrevChunk(final FillContext __context, final WritableChunk __chunk__col__II = this.II.getPrevChunk(__typedContext.__subContextII, __orderedKeys).asLongChunk(); final IntChunk __chunk__col__I = this.I.getPrevChunk(__typedContext.__subContextI, __orderedKeys).asIntChunk(); - fillChunkHelper(__typedContext, __destination, __orderedKeys, __chunk__col__II, __chunk__col__I); + fillChunkHelper(true, __typedContext, __destination, __orderedKeys, __chunk__col__II, __chunk__col__I); } - private void fillChunkHelper(final FormulaFillContext __context, + private void fillChunkHelper(final boolean __usePrev, final FormulaFillContext __context, final WritableChunk __destination, final OrderedKeys __orderedKeys, LongChunk __chunk__col__II, IntChunk __chunk__col__I) { final WritableLongChunk __typedDestination = __destination.asWritableLongChunk(); - __context.__iChunk.setSize(0); - __index.invert(__orderedKeys.asIndex()).forAllLongs(l -> __context.__iChunk.add(__intSize(l))); - __index.invert(__orderedKeys.asIndex()).fillKeyIndicesChunk(__context.__iiChunk); + try (final Index prev = __usePrev ? __index.getPrevIndex() : null; + final Index inverted = ((prev != null) ? prev : __index).invert(__orderedKeys.asIndex())) { + __context.__iChunk.setSize(0); + inverted.forAllLongs(l -> __context.__iChunk.add(__intSize(l))); + inverted.fillKeyIndicesChunk(__context.__iiChunk); + } final int[] __chunkPosHolder = new int[] {0}; if (__lazyResultCache != null) { __orderedKeys.forAllLongs(k -> { diff --git a/DB/src/test/java/io/deephaven/db/v2/select/TestFormulaColumnGeneration.java b/DB/src/test/java/io/deephaven/db/v2/select/TestFormulaColumnGeneration.java index 2c767101e68..3af19a31610 100644 --- a/DB/src/test/java/io/deephaven/db/v2/select/TestFormulaColumnGeneration.java +++ b/DB/src/test/java/io/deephaven/db/v2/select/TestFormulaColumnGeneration.java @@ -20,10 +20,10 @@ public class TestFormulaColumnGeneration { // code only needs to be run when the formula generation code changes, and only after // some human believes that the code is correct. When that happens the process // is: - // 1. Once the formula generation code is believed to be correct, uncomment this "Test" - // 2. Run it once to generate new "golden master" files. + // 1. Once the formula generation code is believed to be correct, uncomment @Test for this test case below. + // 2. Run this test case once to generate new "golden master" files. // 3. Comment this "test" back out - // 4. Confirm that the modified files pass "validateFiles". + // 4. Confirm that the modified files pass the "validateFiles" case. // 4. Check in the modified "golden master" files. // @Test public void generateFiles() throws FileNotFoundException {