Refactor SpiExpressionRequest split parse() into property() and path() to have fast path

property() uses a fast path for the common case that the expression is a bean property path. This will then fall back to using parse() when that isn't the case.
2024-04-21 10:51:47 +00:00 · 2023-03-24 19:35:11 +13:00
parent 9982fa3682
commit 9ff240dc10
29 changed files with 99 additions and 84 deletions
@@ -32,14 +32,22 @@ public interface SpiExpressionRequest {
  SpiOrmQueryRequest<?> getQueryRequest();

  /**
-   * Append to the expression sql.
+   * Append to the expression sql without any parsing.
   */
-  SpiExpressionRequest append(String sql);
+  SpiExpressionRequest append(String expression);
+
+  /**
+   * Append to the expression sql with logical property parsing to db columns with logical path prefix.
+   * <p>
+   * This is a fast path case when expression is a bean property path and falls back to using parse()
+   * when that isn't the case.
+   */
+  SpiExpressionRequest property(String expression);

  /**
   * Append to the expression sql with logical property parsing to db columns with logical path prefix.
   */
-  SpiExpressionRequest parse(String sql);
+  SpiExpressionRequest parse(String expression);

  /**
   * Add an encryption key to bind to this request.
@@ -46,7 +46,7 @@ public abstract class DeployParser {

  protected abstract String convertWord();

-  public abstract String getDeployWord(String expression);
+  public abstract String deployWord(String expression);

  /**
   * Return the join includes.
@@ -57,14 +57,26 @@ public abstract class DeployParser {
    this.encrypted = encrypted;
  }

-  public String parse(String source) {
-    if (source == null) {
+  /**
+   * Fast path parse() which first checks that the source is a bean property.
+   */
+  public String property(String expression) {
+    String deployWord = deployWord(expression);
+    if (deployWord != null) {
+      return deployWord;
+    }
+    // fallback to use parse()
+    return parse(expression);
+  }
+
+  public String parse(String expression) {
+    if (expression == null) {
      return null;
    }
    pos = -1;
-    this.source = source;
-    this.sourceLength = source.length();
-    this.sb = new StringBuilder(source.length() + 20);
+    this.source = expression;
+    this.sourceLength = expression.length();
+    this.sb = new StringBuilder(expression.length() + 20);
    while (nextWord()) {
      if (skipWordConvert()) {
        sb.append(word);
@@ -55,7 +55,7 @@ public final class DeployPropertyParser extends DeployParser {
  }

  @Override
-  public String getDeployWord(String expression) {
+  public String deployWord(String expression) {
    ElPropertyDeploy elProp = beanDescriptor.elPropertyDeploy(expression);
    if (elProp == null) {
      return null;
@@ -70,7 +70,7 @@ public final class DeployPropertyParser extends DeployParser {

  @Override
  public String convertWord() {
-    String r = getDeployWord(word);
+    String r = deployWord(word);
    return r == null ? word : r;
  }

@@ -25,12 +25,12 @@ public class DeployPropertyParserMap extends DeployParser {

  @Override
  public String convertWord() {
-    String r = getDeployWord(word);
+    String r = deployWord(word);
    return r == null ? word : r;
  }

  @Override
-  public String getDeployWord(String expression) {
+  public String deployWord(String expression) {
    return map.get(expression);
  }

@@ -25,7 +25,7 @@ public final class DeployUpdateParser extends DeployParser {

  @Override
  public String convertWord() {
-    String dbWord = getDeployWord(word);
+    String dbWord = deployWord(word);
    if (dbWord != null) {
      return dbWord;
    }
@@ -62,7 +62,7 @@ public final class DeployUpdateParser extends DeployParser {
      start = dotPos + 1;
      String remainder = currentWord.substring(start);

-      String dbWord = getDeployWord(remainder);
+      String dbWord = deployWord(remainder);
      if (dbWord != null) {
        // we have found a match for the remainder
        localBuffer.append(dbWord);
@@ -72,7 +72,7 @@ public final class DeployUpdateParser extends DeployParser {
  }

  @Override
-  public String getDeployWord(String expression) {
+  public String deployWord(String expression) {
    if (expression.equalsIgnoreCase(beanDescriptor.name())) {
      return beanDescriptor.baseTable();
    }
@@ -8,31 +8,14 @@ import io.ebean.config.DatabaseConfig;
 import io.ebean.config.TableName;
 import io.ebean.config.dbplatform.IdType;
 import io.ebean.config.dbplatform.PlatformIdGenerator;
-import io.ebean.event.BeanFindController;
-import io.ebean.event.BeanPersistController;
-import io.ebean.event.BeanPersistListener;
-import io.ebean.event.BeanPostConstructListener;
-import io.ebean.event.BeanPostLoad;
-import io.ebean.event.BeanQueryAdapter;
+import io.ebean.event.*;
 import io.ebean.event.changelog.ChangeLogFilter;
 import io.ebean.text.PathProperties;
 import io.ebean.util.SplitName;
 import io.ebeaninternal.api.ConcurrencyMode;
 import io.ebeaninternal.server.core.CacheOptions;
 import io.ebeaninternal.server.deploy.BeanDescriptor.EntityType;
-import io.ebeaninternal.server.deploy.BeanDescriptorManager;
-import io.ebeaninternal.server.deploy.ChainedBeanPersistController;
-import io.ebeaninternal.server.deploy.ChainedBeanPersistListener;
-import io.ebeaninternal.server.deploy.ChainedBeanPostConstructListener;
-import io.ebeaninternal.server.deploy.ChainedBeanPostLoad;
-import io.ebeaninternal.server.deploy.ChainedBeanQueryAdapter;
-import io.ebeaninternal.server.deploy.DeployPropertyParserMap;
-import io.ebeaninternal.server.deploy.IdentityMode;
-import io.ebeaninternal.server.deploy.IndexDefinition;
-import io.ebeaninternal.server.deploy.InheritInfo;
-import io.ebeaninternal.server.deploy.PartitionMeta;
-import io.ebeaninternal.server.deploy.TableJoin;
-import io.ebeaninternal.server.deploy.TablespaceMeta;
+import io.ebeaninternal.server.deploy.*;
 import io.ebeaninternal.server.deploy.parse.DeployBeanInfo;
 import io.ebeaninternal.server.idgen.UuidV1IdGenerator;
 import io.ebeaninternal.server.idgen.UuidV1RndIdGenerator;
@@ -41,13 +24,7 @@ import io.ebeaninternal.server.rawsql.SpiRawSql;

 import javax.persistence.Entity;
 import javax.persistence.MappedSuperclass;
-import java.util.ArrayList;
-import java.util.Collection;
-import java.util.Comparator;
-import java.util.HashMap;
-import java.util.LinkedHashMap;
-import java.util.List;
-import java.util.Map;
+import java.util.*;

 /**
 * Describes Beans including their deployment information.
@@ -1102,7 +1079,7 @@ public class DeployBeanDescriptor<T> {
    }

    @Override
-    public String getDeployWord(String expression) {
+    public String deployWord(String expression) {
      return descriptor.getDeployWord(expression);
    }
  }
@@ -76,7 +76,7 @@ final class AllEqualsExpression extends NonPrepareExpression {
      if (count > 0) {
        request.append("and ");
      }
-      request.parse(name(propName));
+      request.property(name(propName));
      if (value == null) {
        request.append(" is null ");
      } else {
@@ -52,7 +52,7 @@ final class BetweenExpression extends AbstractExpression {
    if (prop != null && prop.isDbEncrypted()) {
      pname = prop.beanProperty().decryptProperty(propName);
    }
-    request.parse(pname).append(_BETWEEN);
+    request.property(pname).append(_BETWEEN);
  }

  @Override
@@ -81,7 +81,7 @@ final class BetweenPropertyExpression extends NonPrepareExpression {

  @Override
  public void addSql(SpiExpressionRequest request) {
-    request.append(" ?").append(BETWEEN).parse(name(lowProperty)).append(" and ").parse(name(highProperty));
+    request.append(" ?").append(BETWEEN).property(name(lowProperty)).append(" and ").property(name(highProperty));
  }

  @Override
@@ -52,9 +52,9 @@ final class CaseInsensitiveEqualExpression extends AbstractValueExpression {
      pname = prop.beanProperty().decryptProperty(propName);
    }
    if (not) {
-      request.append("lower(").parse(pname).append(") != ?");
+      request.append("lower(").property(pname).append(") != ?");
    } else {
-      request.append("lower(").parse(pname).append(") = ?");
+      request.append("lower(").property(pname).append(") = ?");
    }
  }

@@ -73,7 +73,7 @@ public final class DefaultExpressionRequest implements SpiExpressionRequest {

  @Override
  public String parseDeploy(String logicalProp) {
-    String s = deployParser.getDeployWord(logicalProp);
+    String s = deployParser.deployWord(logicalProp);
    return s == null ? logicalProp : s;
  }

@@ -114,8 +114,18 @@ public final class DefaultExpressionRequest implements SpiExpressionRequest {
   * Append text the underlying sql expression.
   */
  @Override
-  public SpiExpressionRequest append(String sqlExpression) {
-    sql.append(sqlExpression);
+  public SpiExpressionRequest append(String expression) {
+    sql.append(expression);
+    return this;
+  }
+
+  @Override
+  public SpiExpressionRequest property(String expression) {
+    if (deployParser == null) {
+      sql.append(expression);
+    } else {
+      sql.append(deployParser.property(expression));
+    }
    return this;
  }

@@ -82,9 +82,8 @@ public final class IdInExpression extends NonPrepareExpression implements IdInCo
    if (idCollection.isEmpty()) {
      request.append(SQL_FALSE); // append false for this stage
    } else {
-      request.parse(descriptor.idBinder().getBindIdInSql(null));
-      String inClause = idBinder.getIdInValueExpr(false, idCollection.size());
-      request.append(inClause);
+      request.property(descriptor.idBinder().getBindIdInSql(null));
+      request.append(idBinder.getIdInValueExpr(false, idCollection.size()));
    }
  }

@@ -99,7 +98,7 @@ public final class IdInExpression extends NonPrepareExpression implements IdInCo
        request.parse(descriptor.idBinderInLHSSql());
        request.append(idBinder.getIdInValueExpr(false, idCollection.size()));
      } else {
-        request.parse(idBinder.getBeanProperty().name());
+        request.property(idBinder.getBeanProperty().name());
        request.appendInExpression(false, idCollection);
      }
    }
@@ -169,7 +169,7 @@ public final class InExpression extends AbstractExpression implements IdInCommon
        return;
      }
    }
-    request.parse(propName);
+    request.property(propName);
    request.appendInExpression(not, bindValues);
  }

@@ -55,7 +55,7 @@ final class InRangeExpression extends AbstractExpression {
    if (prop != null && prop.isDbEncrypted()) {
      pname = prop.beanProperty().decryptProperty(propName);
    }
-    request.append("(").parse(pname).append(" >= ? and ").parse(pname).append(" < ?)");
+    request.append("(").property(pname).append(" >= ? and ").property(pname).append(" < ?)");
  }

  @Override
@@ -44,9 +44,9 @@ final class LikeExpression extends AbstractValueExpression {
      pname = prop.beanProperty().decryptProperty(propName);
    }
    if (caseInsensitive) {
-      request.append("lower(").parse(pname).append(")");
+      request.append("lower(").property(pname).append(")");
    } else {
-      request.parse(pname);
+      request.property(pname);
    }
    if (type == LikeType.EQUAL_TO) {
      request.append(" = ?");
@@ -41,7 +41,7 @@ final class NativeILikeExpression extends AbstractExpression {
    if (prop != null && prop.isDbEncrypted()) {
      pname = prop.beanProperty().decryptProperty(propName);
    }
-    request.parse(pname).append(" ilike ?");
+    request.property(pname).append(" ilike ?");
  }

  /**
@@ -66,7 +66,7 @@ final class NullExpression extends AbstractExpression {
    if (elProperty != null && elProperty.isAssocId()) {
      request.parse(elProperty.assocIdExpression(propName, nullExpr));
    } else {
-      request.parse(propName).append(nullExpr);
+      request.property(propName).append(nullExpr);
    }
  }

@@ -109,7 +109,7 @@ public final class SimpleExpression extends AbstractValueExpression {
        return;
      }
    }
-    request.parse(propName).append(type.bind());
+    request.property(propName).append(type.bind());
  }

  /**
@@ -90,7 +90,7 @@ final class SubQueryExpression extends AbstractExpression implements Unsupported

  @Override
  public void addSql(SpiExpressionRequest request) {
-    request.parse(propName).append(op.expression).append("(").parse(sql).append(")");
+    request.property(propName).append(op.expression).append("(").parse(sql).append(")");
  }

  @Override
@@ -10,8 +10,8 @@ abstract class BaseDbExpression implements DbExpressionHandler {

  @Override
  public void bitwise(SpiExpressionRequest request, String propName, BitwiseOp operator, long flags, String compare, long match) {
-    String bitOp = bitOp(operator);
-    request.append("(").parse(propName).append(" ").append(bitOp).append(" ? ").append(compare).append(" ?)");
+    final String bitOp = bitOp(operator);
+    request.append("(").property(propName).append(" ").append(bitOp).append(" ? ").append(compare).append(" ?)");
  }

  private String bitOp(BitwiseOp operator) {
@@ -29,8 +29,8 @@ abstract class BaseDbExpression implements DbExpressionHandler {
   * Common alternative where the bitwise operation is a function (specifically bitand is used - H2 and Oracle).
   */
  protected void bitwiseFunction(SpiExpressionRequest request, String propName, BitwiseOp operator, String compare) {
-    String funcName = functionName(operator);
-    request.append(funcName).append("(").parse(propName).append(", ?) ").append(compare).append(" ?");
+    final String funcName = functionName(operator);
+    request.append(funcName).append("(").property(propName).append(", ?) ").append(compare).append(" ?");
  }

  protected String functionName(BitwiseOp operator) {
@@ -10,7 +10,7 @@ final class H2DbExpression extends BasicDbExpression {

  @Override
  public void bitwise(SpiExpressionRequest request, String propName, BitwiseOp operator, long flags, String compare, long match) {
-    String funcName = functionName(operator);
-    request.append(funcName).append("(").parse(propName).append(", cast(? as long)) ").append(compare).append(" cast(? as long)");
+    final String funcName = functionName(operator);
+    request.append(funcName).append("(").property(propName).append(", cast(? as long)) ").append(compare).append(" cast(? as long)");
  }
 }
@@ -16,12 +16,12 @@ final class HanaDbExpression extends BaseDbExpression {

  @Override
  public void json(SpiExpressionRequest request, String propName, String path, Op operator, Object value) {
-    request.append("json_value(").parse(propName).append(", '$.").append(path).append("')").append(operator.bind());
+    request.append("json_value(").property(propName).append(", '$.").append(path).append("')").append(operator.bind());
  }

  @Override
  public void arrayIsEmpty(SpiExpressionRequest request, String propName, boolean empty) {
-    request.append("cardinality(").parse(propName).append(")");
+    request.append("cardinality(").property(propName).append(")");
    if (empty) {
      request.append(" = 0");
    } else {
@@ -50,7 +50,7 @@ final class HanaDbExpression extends BaseDbExpression {
      if (!contains) {
        request.append(" not ");
      }
-      request.append(" member of ").parse(propName).append(")");
+      request.append(" member of ").property(propName).append(")");
    }
  }

@@ -10,6 +10,6 @@ final class MariaDbExpression extends BasicDbExpression {

  @Override
  public void json(SpiExpressionRequest request, String propName, String path, Op operator, Object value) {
-    request.append("(").parse(propName).append(" ->> '$.").append(path).append("')").append(operator.bind());
+    request.append("(").property(propName).append(" ->> '$.").append(path).append("')").append(operator.bind());
  }
 }
@@ -10,7 +10,7 @@ final class MySqlDbExpression extends BasicDbExpression {

  @Override
  public void json(SpiExpressionRequest request, String propName, String path, Op operator, Object value) {
-    request.append("(").parse(propName).append(" ->> '$.").append(path).append("')").append(operator.bind());
+    request.append("(").property(propName).append(" ->> '$.").append(path).append("')").append(operator.bind());
  }

 }
@@ -17,11 +17,11 @@ final class OracleDbExpression extends BaseDbExpression {
  @Override
  public void json(SpiExpressionRequest request, String propName, String path, Op operator, Object value) {
    if (operator == Op.EXISTS) {
-      request.append("json_exists(").parse(propName).append(", '$.").append(path).append("')");
+      request.append("json_exists(").property(propName).append(", '$.").append(path).append("')");
    } else if (operator == Op.NOT_EXISTS) {
-      request.append("not json_exists(").parse(propName).append(", '$.").append(path).append("')");
+      request.append("not json_exists(").property(propName).append(", '$.").append(path).append("')");
    } else {
-      request.append("json_value(").parse(propName).append(", '$.").append(path).append("')").append(operator.bind());
+      request.append("json_value(").property(propName).append(", '$.").append(path).append("')").append(operator.bind());
    }
  }

@@ -18,10 +18,10 @@ final class PostgresDbExpression extends BaseDbExpression {
    String[] paths = path.split("\\.");
    if (paths.length == 1) {
      // (t0.content ->> 'title') = 'Some value'
-      request.append("(").parse(propName).append(" ->> '").append(path).append("')");
+      request.append("(").property(propName).append(" ->> '").append(path).append("')");
    } else {
      // (t0.content #>> '{path,inner}') = 'Some value'
-      request.append("(").parse(propName).append(" #>> '{");
+      request.append("(").property(propName).append(" #>> '{");
      for (int i = 0; i < paths.length; i++) {
        if (i > 0) {
          request.append(",");
@@ -38,7 +38,7 @@ final class PostgresDbExpression extends BaseDbExpression {
    if (!contains) {
      request.append("not (");
    }
-    request.parse(propName).append(" @> array[?");
+    request.property(propName).append(" @> array[?");
    for (int i = 1; i < values.length; i++) {
      request.append(",?");
    }
@@ -50,7 +50,7 @@ final class PostgresDbExpression extends BaseDbExpression {

  @Override
  public void arrayIsEmpty(SpiExpressionRequest request, String propName, boolean empty) {
-    request.append("coalesce(cardinality(").parse(propName).append("),0)");
+    request.append("coalesce(cardinality(").property(propName).append("),0)");
    if (empty) {
      request.append(" = 0");
    } else {
@@ -11,7 +11,7 @@ final class SqlServerDbExpression extends BaseDbExpression {
  @Override
  public void json(final SpiExpressionRequest request, final String propName,
                   final String path, final Op operator, final Object value) {
-    request.append("json_value(").parse(propName).append(", '$.").append(path).append("')").append(operator.bind());
+    request.append("json_value(").property(propName).append(", '$.").append(path).append("')").append(operator.bind());
  }

  @Override
@@ -26,11 +26,13 @@ public class DeployPropertyParserTest extends BaseTest {

  @Test
  public void depth1_path() {
+    Assertions.assertThat(parser().property("billingAddress.city")).isEqualTo("${billingAddress}city");
    Assertions.assertThat(parser().parse("billingAddress.city")).isEqualTo("${billingAddress}city");
  }

  @Test
  public void depth2_path() {
+    Assertions.assertThat(parser().property("max(billingAddress.country.name)")).isEqualTo("max(${billingAddress.country}name)");
    Assertions.assertThat(parser().parse("max(billingAddress.country.name)")).isEqualTo("max(${billingAddress.country}name)");
  }

@@ -69,6 +71,7 @@ public class DeployPropertyParserTest extends BaseTest {

  @Test
  public void unknown_path() {
+    Assertions.assertThat(parser().property(" foo ")).isEqualTo(" foo ");
    Assertions.assertThat(parser().parse(" foo ")).isEqualTo(" foo ");
  }

@@ -44,8 +44,14 @@ public class TDSpiExpressionRequest implements SpiExpressionRequest {
  }

  @Override
-  public SpiExpressionRequest append(String sqlExpression) {
-    sql.append(sqlExpression);
+  public SpiExpressionRequest append(String expression) {
+    sql.append(expression);
+    return this;
+  }
+
+  @Override
+  public SpiExpressionRequest property(String expression) {
+    sql.append(expression);
    return this;
  }