Repository: squashql/squashql
Pull request #311 (2024-04-16 10:38:21)

Guess type of AliasedField when used as criterion


Patch:
diff --git a/core/src/main/java/io/squashql/query/compiled/CompiledCriteria.java b/core/src/main/java/io/squashql/query/compiled/CompiledCriteria.java
index 7a74dde..9867cf2 100644
--- a/core/src/main/java/io/squashql/query/compiled/CompiledCriteria.java
+++ b/core/src/main/java/io/squashql/query/compiled/CompiledCriteria.java
@@ -47,20 +47,22 @@ public record CompiledCriteria(ConditionDto condition, ConditionType conditionTy
 
   public static String toSql(TypedField field, ConditionDto dto, QueryRewriter queryRewriter) {
     String expression = field.sqlExpression(queryRewriter); // FIXME should be queryRewriter.where(field) because some db supports using the alias, others not.
-    if (dto instanceof SingleValueConditionDto || dto instanceof InConditionDto) {
-      Function<Object, String> sqlMapper = SQLTranslator.getQuoteFn(field, queryRewriter);
+
+    if (dto instanceof SingleValueConditionDto svc) {
+      Object value = svc.value;
+      Function<Object, String> sqlMapper = SQLTranslator.getQuoteFn(field.type(), value.getClass(), queryRewriter);
       return switch (dto.type()) {
-        case IN -> expression + " " + dto.type().sqlInfix + " (" +
-                ((InConditionDto) dto).values
-                        .stream()
-                        .map(sqlMapper)
-                        .collect(Collectors.joining(", ")) + ")";
-        case EQ, NEQ, LT, LE, GT, GE, LIKE ->
-                expression + " " + dto.type().sqlInfix + " " + sqlMapper.apply(((SingleValueConditionDto) dto).value);
-        case ARRAY_CONTAINS ->
-                queryRewriter.arrayContains(field, sqlMapper.apply(((SingleValueConditionDto) dto).value));
+        case EQ, NEQ, LT, LE, GT, GE, LIKE -> expression + " " + dto.type().sqlInfix + " " + sqlMapper.apply(value);
+        case ARRAY_CONTAINS -> queryRewriter.arrayContains(field, sqlMapper.apply(value));
         default -> throw new IllegalStateException("Unexpected value: " + dto.type());
       };
+    } else if (dto instanceof InConditionDto ic) {
+      Function<Object, String> sqlMapper = SQLTranslator.getQuoteFn(field.type(), ic.values.iterator().next().getClass(), queryRewriter);
+      return expression + " " + dto.type().sqlInfix + " (" +
+              ic.values
+                      .stream()
+                      .map(sqlMapper)
+                      .collect(Collectors.joining(", ")) + ")";
     } else if (dto instanceof LogicalConditionDto logical) {
       String first = toSql(field, logical.one, queryRewriter);
       String second = toSql(field, logical.two, queryRewriter);
diff --git a/core/src/main/java/io/squashql/query/database/SQLTranslator.java b/core/src/main/java/io/squashql/query/database/SQLTranslator.java
index 628d508..e0ef545 100644
--- a/core/src/main/java/io/squashql/query/database/SQLTranslator.java
+++ b/core/src/main/java/io/squashql/query/database/SQLTranslator.java
@@ -6,7 +6,6 @@ import io.squashql.query.compiled.CompiledCriteria;
 import io.squashql.query.compiled.CompiledOrderBy;
 import io.squashql.query.compiled.CteRecordTable;
 import io.squashql.store.UnknownType;
-import io.squashql.type.TypedField;
 
 import java.util.ArrayList;
 import java.util.Collections;
@@ -157,24 +156,28 @@ public class SQLTranslator {
     }
   }
 
-  public static Function<Object, String> getQuoteFn(TypedField field, QueryRewriter queryRewriter) {
-    Class<?> fType = field.type();
-    if (Number.class.isAssignableFrom(fType)
-            || fType.equals(double.class)
-            || fType.equals(int.class)
-            || fType.equals(long.class)
-            || fType.equals(float.class)
-            || fType.equals(boolean.class)
-            || fType.equals(Boolean.class)
-            || fType.equals(UnknownType.class)
-            || fType.equals(Lists.LongList.class)) {
+  /**
+   * Sometimes the type of the field is unknown (cf. {@link io.squashql.type.AliasedTypedField}). In that case, we use the
+   * type of the object to quote to determine its type. It should be correct in most cases.
+   */
+  public static Function<Object, String> getQuoteFn(Class<?> fieldType, Class<?> guessedClass, QueryRewriter queryRewriter) {
+    if (fieldType.equals(UnknownType.class)) {
+      return getQuoteFn(guessedClass, guessedClass, queryRewriter);
+    } else if (Number.class.isAssignableFrom(fieldType)
+            || fieldType.equals(double.class)
+            || fieldType.equals(int.class)
+            || fieldType.equals(long.class)
+            || fieldType.equals(float.class)
+            || fieldType.equals(boolean.class)
+            || fieldType.equals(Boolean.class)
+            || fieldType.equals(Lists.LongList.class)) {
       // no quote
       return String::valueOf;
-    } else if (fType.equals(String.class) || fType.equals(Lists.StringList.class)) {
+    } else if (fieldType.equals(String.class) || fieldType.equals(Lists.StringList.class)) {
       // quote
       return s -> "'" + queryRewriter.escapeSingleQuote(String.valueOf(s)) + "'";
     } else {
-      throw new RuntimeException("Not supported " + fType);
+      throw new RuntimeException("Not supported " + fieldType);
     }
   }
 
diff --git a/core/src/main/java/io/squashql/query/dto/InConditionDto.java b/core/src/main/java/io/squashql/query/dto/InConditionDto.java
index a04efb9..fe8de1a 100644
--- a/core/src/main/java/io/squashql/query/dto/InConditionDto.java
+++ b/core/src/main/java/io/squashql/query/dto/InConditionDto.java
@@ -15,8 +15,6 @@ import static io.squashql.query.dto.ConditionType.IN;
 @NoArgsConstructor // For Jackson
 public final class InConditionDto implements ConditionDto {
 
-  public final ConditionType type = IN;
-
   public Set<Object> values;
 
   public InConditionDto(Object value) {
@@ -36,6 +34,6 @@ public final class InConditionDto implements ConditionDto {
 
   @Override
   public ConditionType type() {
-    return this.type;
+    return IN;
   }
 }
diff --git a/js/typescript-library/src/condition.ts b/js/typescript-library/src/condition.ts
index 23007dc..7b31b53 100644
--- a/js/typescript-library/src/condition.ts
+++ b/js/typescript-library/src/condition.ts
@@ -75,7 +75,7 @@ export class InCondition implements Condition {
 
   toJSON() {
     return {
-      ...toJSON(this),
+      "@class": this.class,
       "values": this.values,
     }
   }


Test patch:
diff --git a/core/src/test/java/io/squashql/query/TestSQLTranslator.java b/core/src/test/java/io/squashql/query/TestSQLTranslator.java
index ad2c27e..70822fc 100644
--- a/core/src/test/java/io/squashql/query/TestSQLTranslator.java
+++ b/core/src/test/java/io/squashql/query/TestSQLTranslator.java
@@ -1,11 +1,13 @@
 package io.squashql.query;
 
 import io.squashql.query.builder.Query;
+import io.squashql.query.compiled.CompiledCriteria;
 import io.squashql.query.compiled.CompiledMeasure;
 import io.squashql.query.compiled.CompiledOrderBy;
 import io.squashql.query.database.*;
 import io.squashql.query.dto.*;
 import io.squashql.store.Store;
+import io.squashql.type.AliasedTypedField;
 import io.squashql.type.TableTypedField;
 import io.squashql.type.TypedField;
 import lombok.AllArgsConstructor;
@@ -131,6 +133,10 @@ public class TestSQLTranslator {
     return new TestResolver(new QueryDto().table("fake")).resolveField(field);
   }
 
+  private CompiledCriteria compileCriteria(CriteriaDto c) {
+    return new TestResolver(new QueryDto().table("fake")).compileCriteria(c);
+  }
+
   private DatabaseQuery compileQuery(QueryDto query, Map<String, Store> stores) {
     return new QueryResolver(query, stores) {
       DatabaseQuery toDatabaseQuery() {
@@ -300,13 +306,13 @@ public class TestSQLTranslator {
             criterion(c.name + ".c_f", a.name + ".a_f", ConditionType.EQ)
     ));
 
-    final Map<String, Store> stores = Map.of(
+    Map<String, Store> stores = Map.of(
             "A", new Store("A", List.of(new TableTypedField("A", "a_id", int.class), new TableTypedField("A", "a_f", int.class), new TableTypedField("A", "y", int.class))),
             "B", new Store("B", List.of(new TableTypedField("B", "b_id", int.class), new TableTypedField("B", "b_other_id", int.class))),
             "C", new Store("C", List.of(new TableTypedField("c", "a_id", int.class), new TableTypedField("C", "c_f", int.class), new TableTypedField("C", "c_other_id", int.class)))
     );
 
-    final QueryDto query = new QueryDto().table(a).withColumn(tableField("A.y"));
+    QueryDto query = new QueryDto().table(a).withColumn(tableField("A.y"));
 
     assertThat(translate(compileQuery(query, stores)))
             .isEqualTo("select `dataset.A`.`y` from `dataset.A` " +
@@ -568,15 +574,27 @@ public class TestSQLTranslator {
     SQLTranslatorQueryRewriter qr = new SQLTranslatorQueryRewriter();
     TableTypedField stringField = new TableTypedField("store", "field", String.class);
     TableTypedField intField = new TableTypedField("store", "field", int.class);
+
     String sql = new CompiledOrderBy(stringField, new SimpleOrderDto(OrderKeywordDto.ASC)).sqlExpression(qr);
     assertThat(sql).isEqualTo("`dataset.store`.`field` asc nulls first");
+
     sql = new CompiledOrderBy(stringField, new SimpleOrderDto(OrderKeywordDto.DESC)).sqlExpression(qr);
     assertThat(sql).isEqualTo("`dataset.store`.`field` desc nulls first");
+
     sql = new CompiledOrderBy(intField, new ExplicitOrderDto(List.of(2023, 2027))).sqlExpression(qr);
     assertThat(sql).isEqualTo("case when `dataset.store`.`field` is null then 1 when `dataset.store`.`field` = 2023 then 2 when `dataset.store`.`field` = 2027 then 3 else 4 end");
+
     sql = new CompiledOrderBy(stringField, new ExplicitOrderDto(List.of(2023, 2027))).sqlExpression(qr);
     // When field is a string, values should be escaped
     assertThat(sql).isEqualTo("case when `dataset.store`.`field` is null then 1 when `dataset.store`.`field` = '2023' then 2 when `dataset.store`.`field` = '2027' then 3 else 4 end");
+
+    AliasedTypedField aliasedTypedField = new AliasedTypedField("alias_field");
+    // Type is unknown. In that case, the type of the field is guessed by what's in the list
+    sql = new CompiledOrderBy(aliasedTypedField, new ExplicitOrderDto(List.of(2023, 2027))).sqlExpression(qr);
+    assertThat(sql).isEqualTo("case when `alias_field` is null then 1 when `alias_field` = 2023 then 2 when `alias_field` = 2027 then 3 else 4 end");
+
+    sql = new CompiledOrderBy(aliasedTypedField, new ExplicitOrderDto(List.of("A", "B"))).sqlExpression(qr);
+    assertThat(sql).isEqualTo("case when `alias_field` is null then 1 when `alias_field` = 'A' then 2 when `alias_field` = 'B' then 3 else 4 end");
   }
 
   @Test
@@ -651,4 +669,19 @@ public class TestSQLTranslator {
               .isEqualTo("select `pnl` from `dataset.baseStore` where `pnl` > 5 group by `pnl`");
     }
   }
+
+  @Test
+  void testCriteriaWithAliasedField() {
+    SQLTranslatorQueryRewriter qr = new SQLTranslatorQueryRewriter();
+    Field a = new AliasedField("a");
+
+    String sql = compileCriteria(criterion(a, eq("aaa"))).sqlExpression(qr);
+    assertThat(sql).isEqualTo("`a` = 'aaa'");
+
+    sql = compileCriteria(criterion(a, eq(123))).sqlExpression(qr);
+    assertThat(sql).isEqualTo("`a` = 123");
+
+    sql = compileCriteria(criterion(a, eq(true))).sqlExpression(qr);
+    assertThat(sql).isEqualTo("`a` = true");
+  }
 }