Merge pull request #73 from ni-ze/main

[ISSUE #74]make count(*) right
alibaba · Jan 28, 2023 · 67aad1b · 67aad1b
2 parents 7a22f0d + 2852867
commit 67aad1b
Show file tree

Hide file tree

Showing 9 changed files with 102 additions and 32 deletions.
diff --git a/rsqldb-common/src/main/java/com/alibaba/rsqldb/common/function/AVGFunction.java b/rsqldb-common/src/main/java/com/alibaba/rsqldb/common/function/AVGFunction.java
@@ -21,6 +21,11 @@
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.JsonNode;
+import com.fasterxml.jackson.databind.deser.std.NumberDeserializers;
+import com.fasterxml.jackson.databind.node.NumericNode;
+import com.sun.org.slf4j.internal.Logger;
+import com.sun.org.slf4j.internal.LoggerFactory;
+import javafx.util.converter.BigDecimalStringConverter;
 
 import java.math.BigDecimal;
 import java.math.RoundingMode;
@@ -29,6 +34,7 @@
 
 @JsonIgnoreProperties(ignoreUnknown = true)
 public class AVGFunction implements SQLFunction {
+    private static final Logger logger = LoggerFactory.getLogger(AVGFunction.class);
     private String fieldName;
     private String asName;
 
@@ -42,42 +48,46 @@ public AVGFunction(@JsonProperty("fieldName")String fieldName, @JsonProperty("as
     public void apply(JsonNode jsonNode, ConcurrentHashMap<String, Object> container) {
         JsonNode valueNode = jsonNode.get(fieldName);
 
-        if (valueNode == null) {
-            return;
+        Number sum = (Number) container.get(sumField());
+        Number count = (Number) container.get(countField());
+        if (count == null) {
+            count = new BigDecimal(1);
+        } else {
+            BigDecimal bigDecimal = new BigDecimal(String.valueOf(count));
+            count = bigDecimal.add(new BigDecimal(1));
         }
+        container.put(countField(), count);
 
-        BigDecimal sum = (BigDecimal) container.get(sumField());
-        BigDecimal count = (BigDecimal) container.get(countField());
-
-
-        if (sum == null && count == null) {
-            container.put(countField(), new BigDecimal(1));
-            String newValue = valueNode.asText();
-            BigDecimal value = new BigDecimal(newValue);
-
-            container.put(sumField(), value);
-        } else if (sum != null && count != null) {
-            BigDecimal add = count.add(new BigDecimal(1));
-            container.put(countField(), add);
+        if (!(valueNode instanceof NumericNode)) {
+            return;
+        }
 
-            String node = valueNode.asText();
-            BigDecimal value = new BigDecimal(node);
+        String newValue = valueNode.asText();
+        BigDecimal value = new BigDecimal(newValue);
 
-            BigDecimal newValue = sum.add(value);
-            container.put(sumField(), newValue);
+        if (sum == null) {
+            sum = value;
         } else {
-            throw new RSQLServerException();
+            BigDecimal bigDecimal = new BigDecimal(String.valueOf(sum));
+            sum = bigDecimal.add(value);
         }
+        container.put(sumField(), sum);
     }
 
     @Override
     public void secondCalcu(ConcurrentHashMap<String, Object> container, Properties context) {
-        BigDecimal sum = (BigDecimal) container.get(sumField());
-        BigDecimal count = (BigDecimal) container.get(countField());
+        Number sum = (Number) container.get(sumField());
+        Number count = (Number) container.get(countField());
+
+        if (count == null || "0".equals(String.valueOf(count))) {
+            logger.error("the divided is zero or empty.");
+            return;
+        }
+
+        BigDecimal sumBigDecimal = new BigDecimal(String.valueOf(sum));
+        BigDecimal countBigDecimal = new BigDecimal(String.valueOf(count));
 
-        container.put(asName, sum.divide(count, 2, RoundingMode.HALF_UP));
-        container.remove(sumField());
-        container.remove(countField());
+        container.put(asName, sumBigDecimal.divide(countBigDecimal, 2, RoundingMode.HALF_UP));
     }
 
     private String sumField() {

diff --git a/rsqldb-common/src/main/java/com/alibaba/rsqldb/common/function/CountFunction.java b/rsqldb-common/src/main/java/com/alibaba/rsqldb/common/function/CountFunction.java
@@ -16,6 +16,7 @@
 package com.alibaba.rsqldb.common.function;
 
 import com.alibaba.rsqldb.common.RSQLConstant;
+import com.alibaba.rsqldb.common.exception.SyntaxErrorException;
 import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.JsonNode;
@@ -39,11 +40,13 @@ public void apply(JsonNode jsonNode, final ConcurrentHashMap<String, Object> con
 
         if (valueNode != null || RSQLConstant.STAR.equals(fieldName)) {
 
-            BigDecimal old = (BigDecimal)container.get(asName);
+            Object target = container.get(asName);
 
-            if (old == null) {
+            if (target == null) {
                 container.put(asName, new BigDecimal(1));
             } else {
+                Number oldNumber = (Number) target;
+                BigDecimal old = new BigDecimal(String.valueOf(oldNumber));
                 BigDecimal count = old.add(new BigDecimal(1));
 
                 container.put(asName, count);

diff --git a/rsqldb-common/src/main/java/com/alibaba/rsqldb/common/function/MaxFunction.java b/rsqldb-common/src/main/java/com/alibaba/rsqldb/common/function/MaxFunction.java
@@ -44,7 +44,9 @@ public void apply(JsonNode jsonNode, ConcurrentHashMap<String, Object> container
             if (!container.containsKey(asName)) {
                 container.put(asName, newValue);
             }else {
-                BigDecimal old = (BigDecimal)container.get(asName);
+                Number oldNumber = (Number)container.get(asName);
+                BigDecimal old = new BigDecimal(String.valueOf(oldNumber));
+
                 BigDecimal max = old.max(newValue);
 
                 container.put(asName, max);

diff --git a/rsqldb-common/src/main/java/com/alibaba/rsqldb/common/function/MinFunction.java b/rsqldb-common/src/main/java/com/alibaba/rsqldb/common/function/MinFunction.java
@@ -44,7 +44,9 @@ public void apply(JsonNode jsonNode, ConcurrentHashMap<String, Object> container
             if (!container.containsKey(asName)) {
                 container.put(asName, newValue);
             }else {
-                BigDecimal old = (BigDecimal)container.get(asName);
+                Number oldNumber = (Number)container.get(asName);
+                BigDecimal old = new BigDecimal(String.valueOf(oldNumber));
+
                 BigDecimal min = old.min(newValue);
 
                 container.put(asName, min);

diff --git a/rsqldb-parser/src/main/java/com/alibaba/rsqldb/parser/impl/BuildContext.java b/rsqldb-parser/src/main/java/com/alibaba/rsqldb/parser/impl/BuildContext.java
@@ -16,8 +16,11 @@
 package com.alibaba.rsqldb.parser.impl;
 
 import com.alibaba.rsqldb.parser.model.statement.CreateTableStatement;
+import com.fasterxml.jackson.core.JsonGenerator;
+import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 import org.apache.commons.lang3.StringUtils;
 import org.apache.rocketmq.client.producer.DefaultMQProducer;
 import org.apache.rocketmq.streams.core.rstream.GroupedStream;
@@ -56,6 +59,12 @@ public class BuildContext {
     public BuildContext(DefaultMQProducer producer, String jobId) {
         this.producer = producer;
         this.streamBuilder = new StreamBuilder(jobId);
+        this.objectMapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
+                .disable(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES)
+                .enable(DeserializationFeature.ACCEPT_EMPTY_STRING_AS_NULL_OBJECT)
+                .enable(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS)
+                .enable(JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN)
+                .setNodeFactory(JsonNodeFactory.withExactBigDecimals(true));
     }
 
     public StreamBuilder getStreamBuilder() {

diff --git a/...-parser/src/main/java/com/alibaba/rsqldb/parser/model/statement/query/QueryStatement.java b/...-parser/src/main/java/com/alibaba/rsqldb/parser/model/statement/query/QueryStatement.java
@@ -101,7 +101,7 @@ public QueryStatement(@JsonProperty("content") String content, @JsonProperty("ta
 
     private List<SQLFunction> buildFunction() {
         ArrayList<SQLFunction> result = new ArrayList<>();
-        if (isSelectAll()) {
+        if (isSelectField()) {
             return result;
         }
 
@@ -122,18 +122,30 @@ private List<SQLFunction> buildFunction() {
                         break;
                     }
                     case MAX: {
+                        if (RSQLConstant.STAR.equals(fieldName)) {
+                            throw new SyntaxErrorException("syntax error: MAX(*)");
+                        }
                         function = new MaxFunction(fieldName, newName);
                         break;
                     }
                     case MIN: {
+                        if (RSQLConstant.STAR.equals(fieldName)) {
+                            throw new SyntaxErrorException("syntax error: MIN(*)");
+                        }
                         function = new MinFunction(fieldName, newName);
                         break;
                     }
                     case SUM: {
+                        if (RSQLConstant.STAR.equals(fieldName)) {
+                            throw new SyntaxErrorException("syntax error: SUM(*)");
+                        }
                         function = new SumFunction(fieldName, newName);
                         break;
                     }
                     case AVG: {
+                        if (RSQLConstant.STAR.equals(fieldName)) {
+                            throw new SyntaxErrorException("syntax error: AVG(*)");
+                        }
                         function = new AVGFunction(fieldName, newName);
                         break;
                     }

diff --git a/rsqldb-parser/src/main/java/com/alibaba/rsqldb/parser/serialization/json/JsonDe.java b/rsqldb-parser/src/main/java/com/alibaba/rsqldb/parser/serialization/json/JsonDe.java
@@ -19,10 +19,12 @@
 import com.alibaba.rsqldb.parser.model.Field;
 import com.alibaba.rsqldb.parser.serialization.Deserializer;
 import com.alibaba.rsqldb.parser.serialization.FieldKeyDeserializer;
+import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
 import com.fasterxml.jackson.databind.module.SimpleModule;
+import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 
 import java.io.IOException;
 
@@ -34,8 +36,12 @@ public JsonDe() {
         SimpleModule simpleModule = new SimpleModule();
         simpleModule.addKeyDeserializer(Field.class, new FieldKeyDeserializer());
         objectMapper.registerModule(simpleModule);
-        objectMapper.configure(DeserializationFeature.ACCEPT_EMPTY_STRING_AS_NULL_OBJECT, true);
-        objectMapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false);
+        objectMapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
+                .disable(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES)
+                .enable(DeserializationFeature.ACCEPT_EMPTY_STRING_AS_NULL_OBJECT)
+                .enable(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS)
+                .enable(JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN)
+                .setNodeFactory(JsonNodeFactory.withExactBigDecimals(true));
     }
 
     public JsonNode deserialize(byte[] source) throws DeserializeException {

diff --git a/rsqldb-parser/src/main/java/com/alibaba/rsqldb/parser/serialization/json/JsonSer.java b/rsqldb-parser/src/main/java/com/alibaba/rsqldb/parser/serialization/json/JsonSer.java
@@ -17,13 +17,24 @@
 
 import com.alibaba.rsqldb.common.exception.SerializeException;
 import com.alibaba.rsqldb.parser.serialization.Serializer;
+import com.fasterxml.jackson.core.JsonGenerator;
 import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.databind.DeserializationFeature;
 import com.fasterxml.jackson.databind.JsonNode;
 import com.fasterxml.jackson.databind.ObjectMapper;
+import com.fasterxml.jackson.databind.node.JsonNodeFactory;
 import com.fasterxml.jackson.databind.node.ObjectNode;
 
 public class JsonSer implements Serializer {
     private static final ObjectMapper objectMapper = new ObjectMapper();
+    static {
+        objectMapper.disable(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES)
+                .disable(DeserializationFeature.FAIL_ON_NULL_FOR_PRIMITIVES)
+                .enable(DeserializationFeature.ACCEPT_EMPTY_STRING_AS_NULL_OBJECT)
+                .enable(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS)
+                .enable(JsonGenerator.Feature.WRITE_BIGDECIMAL_AS_PLAIN)
+                .setNodeFactory(JsonNodeFactory.withExactBigDecimals(true));
+    }
 
     public byte[] serialize(Object obj) throws SerializeException {
         if (obj == null) {

diff --git a/rsqldb-parser/src/test/java/com/alibaba/rsqldb/parser/query/QueryStatementParser.java b/rsqldb-parser/src/test/java/com/alibaba/rsqldb/parser/query/QueryStatementParser.java
@@ -15,6 +15,7 @@
   */
  package com.alibaba.rsqldb.parser.query;
 
+ import com.alibaba.rsqldb.common.exception.SyntaxErrorException;
  import com.alibaba.rsqldb.parser.SerDer;
  import com.alibaba.rsqldb.parser.model.Calculator;
  import com.alibaba.rsqldb.parser.model.Field;
@@ -410,6 +411,20 @@ public void query7() throws Throwable {
              assertSame(Calculator.COUNT, calculator);
          }
      }
+     @Test
+     public void query70() throws Throwable {
+         String sql = "select sum(*) as newName from rocketmq_source;";
+
+         Throwable error = null;
+         try {
+             QueryStatement queryStatement = parser(sql, QueryStatement.class);
+         }catch (Throwable t) {
+             error = t;
+         }
+
+         assertNotNull(error);
+         assertTrue(error instanceof SyntaxErrorException);
+     }
 
      @Test
      public void query8() throws Throwable {