From 56550a953436d36dcc839ab2a703e69b0ffb62a5 Mon Sep 17 00:00:00 2001 From: Rob Bygrave Date: Mon, 21 Aug 2023 23:16:55 +1200 Subject: [PATCH] #3166 Remove InTuples literal mode --- .../api/SpiExpressionRequest.java | 5 -- .../expression/DefaultExpressionRequest.java | 5 -- .../server/expression/InLiterals.java | 73 ------------------ .../server/expression/InTuplesExpression.java | 75 +------------------ .../server/expression/InLiteralsTest.java | 58 -------------- .../expression/InTuplesExpressionTest.java | 51 ------------- .../expression/TDSpiExpressionRequest.java | 5 -- .../TestInTuplesWithLocalDate.java | 25 +------ .../cache/TestCacheViaComplexNaturalKey3.java | 31 -------- 9 files changed, 2 insertions(+), 326 deletions(-) delete mode 100644 ebean-core/src/main/java/io/ebeaninternal/server/expression/InLiterals.java delete mode 100644 ebean-core/src/test/java/io/ebeaninternal/server/expression/InLiteralsTest.java delete mode 100644 ebean-core/src/test/java/io/ebeaninternal/server/expression/InTuplesExpressionTest.java diff --git a/ebean-core/src/main/java/io/ebeaninternal/api/SpiExpressionRequest.java b/ebean-core/src/main/java/io/ebeaninternal/api/SpiExpressionRequest.java index cf53a0131..1b90140ee 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/api/SpiExpressionRequest.java +++ b/ebean-core/src/main/java/io/ebeaninternal/api/SpiExpressionRequest.java @@ -31,11 +31,6 @@ public interface SpiExpressionRequest { */ SpiOrmQueryRequest queryRequest(); - /** - * Return the underling buffer. - */ - StringBuilder buffer(); - /** * Append to the expression sql without any parsing. */ diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionRequest.java b/ebean-core/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionRequest.java index 28468b834..6f05e31a5 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionRequest.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/expression/DefaultExpressionRequest.java @@ -109,11 +109,6 @@ public final class DefaultExpressionRequest implements SpiExpressionRequest { return queryRequest; } - @Override - public StringBuilder buffer() { - return sql; - } - @Override public SpiExpressionRequest append(String expression) { sql.append(expression); diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/expression/InLiterals.java b/ebean-core/src/main/java/io/ebeaninternal/server/expression/InLiterals.java deleted file mode 100644 index f67000ffc..000000000 --- a/ebean-core/src/main/java/io/ebeaninternal/server/expression/InLiterals.java +++ /dev/null @@ -1,73 +0,0 @@ -package io.ebeaninternal.server.expression; - -import io.ebean.annotation.Platform; - -import java.time.LocalDate; -import java.util.UUID; - -/** - * Adds values as literals to SQL string. - */ -interface InLiterals { - - /** - * Add the value as a SQL literal to the buffer. - */ - void append(StringBuilder buffer, Object value); - - /** - * Return the InLiterals for the type of the given value. - */ - static InLiterals of(Object val, Platform platform) { - if (val instanceof Number) { - return NumLiteral.INSTANCE; - } - if (val instanceof String || val instanceof UUID) { - return StrLiteral.INSTANCE; - } - if (val instanceof LocalDate) { - return platform.base() == Platform.MYSQL ? DateEscapeLiteral.INSTANCE : DateLiteral.INSTANCE; - } - throw new UnsupportedOperationException(); - } - - final class NumLiteral implements InLiterals { - - static NumLiteral INSTANCE = new NumLiteral(); - - @Override - public void append(StringBuilder buffer, Object value) { - buffer.append(value); - } - } - - final class StrLiteral implements InLiterals { - - static StrLiteral INSTANCE = new StrLiteral(); - - @Override - public void append(StringBuilder buffer, Object value) { - buffer.append('\'').append(value).append('\''); - } - } - - final class DateLiteral implements InLiterals { - - static DateLiteral INSTANCE = new DateLiteral(); - - @Override - public void append(StringBuilder buffer, Object value) { - buffer.append("date ").append('\'').append(value).append('\''); - } - } - - final class DateEscapeLiteral implements InLiterals { - - static DateEscapeLiteral INSTANCE = new DateEscapeLiteral(); - - @Override - public void append(StringBuilder buffer, Object value) { - buffer.append("{d '").append(value).append("'}"); - } - } -} diff --git a/ebean-core/src/main/java/io/ebeaninternal/server/expression/InTuplesExpression.java b/ebean-core/src/main/java/io/ebeaninternal/server/expression/InTuplesExpression.java index 8c46e236d..8fa2fb8cb 100644 --- a/ebean-core/src/main/java/io/ebeaninternal/server/expression/InTuplesExpression.java +++ b/ebean-core/src/main/java/io/ebeaninternal/server/expression/InTuplesExpression.java @@ -1,8 +1,6 @@ package io.ebeaninternal.server.expression; import io.ebean.InTuples; -import io.ebean.annotation.Platform; -import io.ebean.event.BeanQueryRequest; import io.ebean.service.SpiInTuples; import io.ebeaninternal.api.BindValuesKey; import io.ebeaninternal.api.NaturalKeyQueryData; @@ -10,7 +8,6 @@ import io.ebeaninternal.api.SpiExpression; import io.ebeaninternal.api.SpiExpressionRequest; import java.util.List; -import java.util.UUID; import static java.util.Objects.requireNonNull; @@ -19,8 +16,6 @@ final class InTuplesExpression extends AbstractExpression { private final boolean not; private final String[] properties; private final List entries; - private InLiterals[] literals; - private boolean literalMode; InTuplesExpression(InTuples pairs, boolean not) { super(""); @@ -31,44 +26,6 @@ final class InTuplesExpression extends AbstractExpression { this.not = not; } - @Override - public void prepareExpression(BeanQueryRequest request) { - if (entries.size() > 50) { - // check if this should go into literal mode - final int maxInBinding = request.database().pluginApi().databasePlatform().maxInBinding(); - final int threshold = literalThreshold(maxInBinding, properties.length); - if (entries.size() > threshold) { - literals = initLiterals(request.database().platform()); - literalMode = literals != null; - } - } - } - - private InLiterals[] initLiterals(Platform platform) { - try { - Object[] firstData = entries.get(0); - final InLiterals[] literals = new InLiterals[firstData.length]; - for (int i = 0; i < firstData.length; i++) { - literals[i] = InLiterals.of(firstData[i], platform); - } - return literals; - } catch (UnsupportedOperationException e) { - // one of the value types is not supported as a SQL literal - // so stick to binding only - return null; - } - } - - /** - * Return the threshold in number of entries before literal mode is used. - */ - static int literalThreshold(int maxInBinding, int propertyCount) { - if (maxInBinding == 0) { - return 5000 / propertyCount; - } - return (maxInBinding / propertyCount) - 200; - } - @Override public boolean naturalKey(NaturalKeyQueryData data) { return false; @@ -81,9 +38,6 @@ final class InTuplesExpression extends AbstractExpression { @Override public void addBindValues(SpiExpressionRequest request) { - if (literalMode) { - return; - } for (Object[] entry : entries) { for (Object value : entry) { requireNonNull(value); @@ -106,32 +60,10 @@ final class InTuplesExpression extends AbstractExpression { request.property(properties[i]); } request.append(") in ("); - if (literalMode) { - addSqlLiterals(request); - } else { - addSqlBinding(request); - } + addSqlBinding(request); request.append(')'); } - private void addSqlLiterals(SpiExpressionRequest request) { - final var buffer = request.buffer(); - for (int i = 0; i < entries.size(); i++) { - if (i > 0) { - buffer.append(','); - } - buffer.append('('); - Object[] values = entries.get(i); - for (int v = 0; v < values.length; v++) { - if (v > 0) { - buffer.append(','); - } - literals[v].append(buffer, values[v]); - } - buffer.append(')'); - } - } - private void addSqlBinding(SpiExpressionRequest request) { final String eb = entryBinding(); for (int i = 0; i < entries.size(); i++) { @@ -159,11 +91,6 @@ final class InTuplesExpression extends AbstractExpression { */ @Override public void queryPlanHash(StringBuilder builder) { - if (literalMode) { - builder.delete(0, builder.length()); - builder.append("$NoCache/").append(UUID.randomUUID()).append('/'); - return; - } if (not) { builder.append("Not"); } diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/expression/InLiteralsTest.java b/ebean-core/src/test/java/io/ebeaninternal/server/expression/InLiteralsTest.java deleted file mode 100644 index 6a993f67f..000000000 --- a/ebean-core/src/test/java/io/ebeaninternal/server/expression/InLiteralsTest.java +++ /dev/null @@ -1,58 +0,0 @@ -package io.ebeaninternal.server.expression; - -import io.ebean.annotation.Platform; -import org.junit.jupiter.api.Test; - -import java.math.BigDecimal; -import java.math.BigInteger; -import java.time.LocalDate; -import java.util.UUID; - -import static org.assertj.core.api.Assertions.assertThat; - -class InLiteralsTest { - - private static String using(Object value) { - return using(value, Platform.GENERIC); - } - - private static String using(Object value, Platform platform) { - InLiterals litInt = InLiterals.of(value, platform); - StringBuilder sb = new StringBuilder(); - litInt.append(sb, value); - return sb.toString(); - } - - @Test - void of_numbers() { - assertThat(using(42)).isEqualTo("42"); - assertThat(using(42d)).isEqualTo("42.0"); - assertThat(using(42L)).isEqualTo("42"); - assertThat(using(new BigDecimal("42.34"))).isEqualTo("42.34"); - assertThat(using(new BigInteger("42"))).isEqualTo("42"); - assertThat(using(Integer.valueOf(42))).isEqualTo("42"); - assertThat(using(Long.valueOf(42))).isEqualTo("42"); - } - - @Test - void of_str() { - assertThat(using("hi")).isEqualTo("'hi'"); - assertThat(using("there")).isEqualTo("'there'"); - } - - @Test - void of_uuid() { - UUID uuid = UUID.randomUUID(); - assertThat(using(uuid)).isEqualTo("'" + uuid + "'"); - } - - @Test - void of_localDate() { - assertThat(using(LocalDate.of(2023, 3, 7))).isEqualTo("date '2023-03-07'"); - } - - @Test - void of_localDate_mysql() { - assertThat(using(LocalDate.of(2023, 3, 7), Platform.MYSQL)).isEqualTo("{d '2023-03-07'}"); - } -} diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/expression/InTuplesExpressionTest.java b/ebean-core/src/test/java/io/ebeaninternal/server/expression/InTuplesExpressionTest.java deleted file mode 100644 index 196952ca7..000000000 --- a/ebean-core/src/test/java/io/ebeaninternal/server/expression/InTuplesExpressionTest.java +++ /dev/null @@ -1,51 +0,0 @@ -package io.ebeaninternal.server.expression; - -import org.junit.jupiter.api.Test; - -import static org.assertj.core.api.Assertions.assertThat; - -class InTuplesExpressionTest { - - @Test - void literalThreshold_maxInBindingZero_2properties() { - int threshold = InTuplesExpression.literalThreshold(0, 2); - assertThat(threshold).isEqualTo(2500); - } - - @Test - void literalThreshold_maxInBindingZero_3properties() { - int threshold = InTuplesExpression.literalThreshold(0, 3); - assertThat(threshold).isEqualTo(1666); - } - - @Test - void literalThreshold_maxInBindingZero_4properties() { - int threshold = InTuplesExpression.literalThreshold(0, 4); - assertThat(threshold).isEqualTo(1250); - } - - @Test - void literalThreshold_sqlServer_2properties() { - int threshold = InTuplesExpression.literalThreshold(2000, 2); - assertThat(threshold).isEqualTo(800); - } - - @Test - void literalThreshold_sqlServer_3properties() { - int threshold = InTuplesExpression.literalThreshold(2000, 3); - assertThat(threshold).isEqualTo(466); - } - - @Test - void literalThreshold_sqlServer_4properties() { - int threshold = InTuplesExpression.literalThreshold(2000, 4); - assertThat(threshold).isEqualTo(300); - } - - @Test - void literalThreshold_5000() { - int threshold = InTuplesExpression.literalThreshold(5000, 2); - assertThat(threshold).isEqualTo(2300); - } - -} diff --git a/ebean-core/src/test/java/io/ebeaninternal/server/expression/TDSpiExpressionRequest.java b/ebean-core/src/test/java/io/ebeaninternal/server/expression/TDSpiExpressionRequest.java index 74ee2ef76..f4000fd3b 100644 --- a/ebean-core/src/test/java/io/ebeaninternal/server/expression/TDSpiExpressionRequest.java +++ b/ebean-core/src/test/java/io/ebeaninternal/server/expression/TDSpiExpressionRequest.java @@ -43,11 +43,6 @@ public class TDSpiExpressionRequest implements SpiExpressionRequest { return null; } - @Override - public StringBuilder buffer() { - return sql; - } - @Override public SpiExpressionRequest append(String expression) { sql.append(expression); diff --git a/ebean-test/src/test/java/org/tests/model/aggregation/TestInTuplesWithLocalDate.java b/ebean-test/src/test/java/org/tests/model/aggregation/TestInTuplesWithLocalDate.java index 7ca3036ac..96a190b52 100644 --- a/ebean-test/src/test/java/org/tests/model/aggregation/TestInTuplesWithLocalDate.java +++ b/ebean-test/src/test/java/org/tests/model/aggregation/TestInTuplesWithLocalDate.java @@ -54,7 +54,6 @@ class TestInTuplesWithLocalDate extends BaseTestCase { .add(today.plusDays(1), 12) .add(today.plusDays(2), 15); - LoggedSql.start(); List result = DB.find(DMachineStats.class) @@ -65,31 +64,9 @@ class TestInTuplesWithLocalDate extends BaseTestCase { assertThat(result).hasSize(5); - var in2 = InTuples.of("date", "hours") - .add(today, 0); - - for (int i = 0; i < 3_000; i++) { - in2.add(today, 100 + i); - } - - List result2 = DB.find(DMachineStats.class) - .where() - .inTuples(in2) - //.raw("(t0.edate,t0.hours) in ((date '2023-08-16',0),(date '2023-08-16',100))") - .eq("machine.name", "inTuple") - .findList(); - List sql = LoggedSql.stop(); - - assertThat(sql).hasSize(2); + assertThat(sql).hasSize(1); assertThat(sql.get(0)).contains("where (t0.edate,t0.hours) in ((?,?),(?,?),(?,?),(?,?),(?,?)) and t1.name = ?"); - if (isMySql()) { - assertThat(sql.get(1)).contains("where (t0.edate,t0.hours) in (({d '2023-08-16'},0),({d '2023-08-16'},100),({d '2023-08-16'},101),("); - } else if (isPostgresCompatible()) { - assertThat(sql.get(1)).contains("where (t0.edate,t0.hours) in ((?,?),(?,?),(?,?),("); - } else { - assertThat(sql.get(1)).contains("where (t0.edate,t0.hours) in ((date '2023-08-16',0),(date '2023-08-16',100),(date '2023-08-16',101),("); - } DB.deleteAll(allStats); DB.delete(machine); diff --git a/ebean-test/src/test/java/org/tests/model/basic/cache/TestCacheViaComplexNaturalKey3.java b/ebean-test/src/test/java/org/tests/model/basic/cache/TestCacheViaComplexNaturalKey3.java index 01d12b476..8cf5617f1 100644 --- a/ebean-test/src/test/java/org/tests/model/basic/cache/TestCacheViaComplexNaturalKey3.java +++ b/ebean-test/src/test/java/org/tests/model/basic/cache/TestCacheViaComplexNaturalKey3.java @@ -454,35 +454,4 @@ public class TestCacheViaComplexNaturalKey3 extends BaseTestCase { assertThat(list).hasSize(3); assertSql(sql.get(0)).contains("from o_cached_natkey3 t0 where t0.store = ? and (t0.sku,t0.code) in ((?,?),(?,?),(?,?)) order by t0.sku desc;"); } - - @IgnorePlatform({Platform.SQLSERVER, Platform.DB2}) - @Test - void inTuples_literalMode() { - InTuples tuples = InTuples.of("sku", "code"); - tuples.add("hi", 123); - tuples.add("bye", 121); - // add more entries than threshold triggers literal mode - for (int i = 0; i < 5_000; i++) { - tuples.add("x", i); - } - - LoggedSql.start(); - - DB.find(OCachedNatKeyBean3.class) - .where() - .eq("store", "def") - .inTuples(tuples) - .setUseCache(false) - .orderBy("sku desc") - .findList(); - - List sql = LoggedSql.stop(); - - if (isPostgresCompatible()) { - // didn't exceed postgres threshold - assertSql(sql.get(0)).contains("from o_cached_natkey3 t0 where t0.store = ? and (t0.sku,t0.code) in ((?,?),(?,?),("); - } else { - assertSql(sql.get(0)).contains("from o_cached_natkey3 t0 where t0.store = ? and (t0.sku,t0.code) in (('hi',123),('bye',121),('"); - } - } }