[fix][headless]Fix logic bug in s2sql parsing. (#1996)

tencentmusic · Jan 4, 2025 · be59b05 · be59b05
1 parent 83cb696
commit be59b05
Show file tree

Hide file tree

Showing 7 changed files with 60 additions and 46 deletions.
diff --git a/common/src/main/java/com/tencent/supersonic/common/jsqlparser/SqlSelectHelper.java b/common/src/main/java/com/tencent/supersonic/common/jsqlparser/SqlSelectHelper.java
@@ -1,5 +1,7 @@
 package com.tencent.supersonic.common.jsqlparser;
 
+import com.google.common.collect.Lists;
+import com.google.common.collect.Sets;
 import com.tencent.supersonic.common.util.StringUtil;
 import lombok.extern.slf4j.Slf4j;
 import net.sf.jsqlparser.JSQLParserException;
@@ -292,19 +294,37 @@ private static List<String> getFieldsByPlainSelect(PlainSelect plainSelect) {
         }
         List<PlainSelect> plainSelectList = new ArrayList<>();
         plainSelectList.add(plainSelect);
-        Set<String> result = getSelectFields(plainSelectList);
-
-        getGroupByFields(plainSelect, result);
-
-        getOrderByFields(plainSelect, result);
-
-        getWhereFields(plainSelectList, result);
-
-        getHavingFields(plainSelect, result);
-
-        getLateralViewsFields(plainSelect, result);
-
-        return new ArrayList<>(result);
+        Set<String> selectFields = getSelectFields(plainSelectList);
+        Set<String> aliases = getAliasFields(plainSelect);
+
+        Set<String> groupByFields = Sets.newHashSet();
+        getGroupByFields(plainSelect, groupByFields);
+        groupByFields.removeAll(aliases);
+
+        Set<String> orderByFields = Sets.newHashSet();
+        getOrderByFields(plainSelect, orderByFields);
+        orderByFields.removeAll(aliases);
+
+        Set<String> whereFields = Sets.newHashSet();
+        getWhereFields(plainSelectList, whereFields);
+        whereFields.removeAll(aliases);
+
+        Set<String> havingFields = Sets.newHashSet();
+        getHavingFields(plainSelect, havingFields);
+        havingFields.removeAll(aliases);
+
+        Set<String> lateralFields = Sets.newHashSet();
+        getLateralViewsFields(plainSelect, lateralFields);
+        lateralFields.removeAll(aliases);
+
+        List<String> results = Lists.newArrayList();
+        results.addAll(selectFields);
+        results.addAll(groupByFields);
+        results.addAll(orderByFields);
+        results.addAll(whereFields);
+        results.addAll(havingFields);
+        results.addAll(lateralFields);
+        return new ArrayList<>(results);
     }
 
     private static void getHavingFields(PlainSelect plainSelect, Set<String> result) {

diff --git a/headless/chat/src/main/java/com/tencent/supersonic/headless/chat/mapper/AllFieldMapper.java b/headless/chat/src/main/java/com/tencent/supersonic/headless/chat/mapper/AllFieldMapper.java
@@ -31,8 +31,8 @@ public void doMap(ChatQueryContext chatQueryContext) {
             List<SchemaElementMatch> allMatches = Lists.newArrayList();
             for (SchemaElement schemaElement : schemaElements) {
                 allMatches.add(SchemaElementMatch.builder().word(schemaElement.getName())
-                        .element(schemaElement).detectWord(schemaElement.getName())
-                        .similarity(0.1).build());
+                        .element(schemaElement).detectWord(schemaElement.getName()).similarity(0.1)
+                        .build());
             }
             chatQueryContext.getMapInfo().setMatchedElements(entry.getKey(), allMatches);
         }

diff --git a/.../src/main/java/com/tencent/supersonic/headless/core/translator/parser/SqlQueryParser.java b/.../src/main/java/com/tencent/supersonic/headless/core/translator/parser/SqlQueryParser.java
@@ -1,5 +1,6 @@
 package com.tencent.supersonic.headless.core.translator.parser;
 
+import com.google.common.collect.Lists;
 import com.tencent.supersonic.common.jsqlparser.SqlAsHelper;
 import com.tencent.supersonic.common.jsqlparser.SqlReplaceHelper;
 import com.tencent.supersonic.common.jsqlparser.SqlSelectFunctionHelper;
@@ -57,15 +58,22 @@ public void parse(QueryStatement queryStatement) throws Exception {
         }
 
         // build ontologyQuery
-        List<String> allFields = SqlSelectHelper.getAllSelectFields(sqlQuery.getSql());
-        List<MetricSchemaResp> metricSchemas = getMetrics(semanticSchemaResp, allFields);
+        List<String> queryFields = SqlSelectHelper.getAllSelectFields(sqlQuery.getSql());
+        List<MetricSchemaResp> metricSchemas = getMetrics(semanticSchemaResp, queryFields);
         List<String> metrics =
                 metricSchemas.stream().map(SchemaItem::getBizName).collect(Collectors.toList());
-        Set<String> dimensions = getDimensions(semanticSchemaResp, allFields);
+        List<DimSchemaResp> dimensionSchemas = getDimensions(semanticSchemaResp, queryFields);
+        List<String> dimensions =
+                dimensionSchemas.stream().map(SchemaItem::getBizName).collect(Collectors.toList());
         // check if there are fields not matched with any metric or dimension
-        if (allFields.size() > metricSchemas.size() + dimensions.size()) {
-            queryStatement
-                    .setErrMsg("There are querying columns in the SQL not matched with any semantic field.");
+        if (queryFields.size() > metricSchemas.size() + dimensions.size()) {
+            List<String> semanticFields = Lists.newArrayList();
+            metricSchemas.forEach(m -> semanticFields.add(m.getBizName()));
+            dimensionSchemas.forEach(d -> semanticFields.add(d.getBizName()));
+            String errMsg =
+                    String.format("Querying columns[%s] not matched with semantic fields[%s].",
+                            queryFields, semanticFields);
+            queryStatement.setErrMsg(errMsg);
             queryStatement.setStatus(1);
             return;
         }
@@ -125,15 +133,15 @@ private AggOption getAggOption(String sql, List<MetricSchemaResp> metricSchemas)
         return AggOption.DEFAULT;
     }
 
-    private Set<String> getDimensions(SemanticSchemaResp semanticSchemaResp,
+    private List<DimSchemaResp> getDimensions(SemanticSchemaResp semanticSchemaResp,
             List<String> allFields) {
-        Map<String, String> dimensionLowerToNameMap = semanticSchemaResp.getDimensions().stream()
-                .collect(Collectors.toMap(entry -> entry.getBizName().toLowerCase(),
-                        SchemaItem::getBizName, (k1, k2) -> k1));
+        Map<String, DimSchemaResp> dimensionLowerToNameMap =
+                semanticSchemaResp.getDimensions().stream().collect(Collectors
+                        .toMap(entry -> entry.getBizName().toLowerCase(), entry -> entry));
         return allFields.stream()
                 .filter(entry -> dimensionLowerToNameMap.containsKey(entry.toLowerCase()))
                 .map(entry -> dimensionLowerToNameMap.get(entry.toLowerCase()))
-                .collect(Collectors.toSet());
+                .collect(Collectors.toList());
     }
 
     private List<MetricSchemaResp> getMetrics(SemanticSchemaResp semanticSchemaResp,

diff --git a/...va/com/tencent/supersonic/headless/server/facade/service/impl/S2SemanticLayerService.java b/...va/com/tencent/supersonic/headless/server/facade/service/impl/S2SemanticLayerService.java
@@ -30,7 +30,6 @@
 import com.tencent.supersonic.headless.server.service.DimensionService;
 import com.tencent.supersonic.headless.server.service.MetricService;
 import com.tencent.supersonic.headless.server.service.SchemaService;
-import com.tencent.supersonic.headless.server.utils.MetricDrillDownChecker;
 import com.tencent.supersonic.headless.server.utils.QueryUtils;
 import com.tencent.supersonic.headless.server.utils.StatUtils;
 import lombok.SneakyThrows;
@@ -53,7 +52,6 @@ public class S2SemanticLayerService implements SemanticLayerService {
     private final DataSetService dataSetService;
     private final SchemaService schemaService;
     private final SemanticTranslator semanticTranslator;
-    private final MetricDrillDownChecker metricDrillDownChecker;
     private final KnowledgeBaseService knowledgeBaseService;
     private final MetricService metricService;
     private final DimensionService dimensionService;
@@ -63,7 +61,6 @@ public class S2SemanticLayerService implements SemanticLayerService {
     public S2SemanticLayerService(StatUtils statUtils, QueryUtils queryUtils,
             SemanticSchemaManager semanticSchemaManager, DataSetService dataSetService,
             SchemaService schemaService, SemanticTranslator semanticTranslator,
-            MetricDrillDownChecker metricDrillDownChecker,
             KnowledgeBaseService knowledgeBaseService, MetricService metricService,
             DimensionService dimensionService) {
         this.statUtils = statUtils;
@@ -72,7 +69,6 @@ public S2SemanticLayerService(StatUtils statUtils, QueryUtils queryUtils,
         this.dataSetService = dataSetService;
         this.schemaService = schemaService;
         this.semanticTranslator = semanticTranslator;
-        this.metricDrillDownChecker = metricDrillDownChecker;
         this.knowledgeBaseService = knowledgeBaseService;
         this.metricService = metricService;
         this.dimensionService = dimensionService;
@@ -119,10 +115,6 @@ public SemanticQueryResp queryByReq(SemanticQueryReq queryReq, User user) {
             QueryStatement queryStatement = buildQueryStatement(queryReq, user);
             semanticTranslator.translate(queryStatement);
 
-            // Check whether the dimensions of the metric drill-down are correct temporarily,
-            // add the abstraction of a validator later.
-            metricDrillDownChecker.checkQuery(queryStatement);
-
             // 4.execute query
             SemanticQueryResp queryResp = null;
             for (QueryExecutor queryExecutor : queryExecutors) {

diff --git a/...r/src/main/java/com/tencent/supersonic/headless/server/service/impl/ModelServiceImpl.java b/...r/src/main/java/com/tencent/supersonic/headless/server/service/impl/ModelServiceImpl.java
@@ -116,7 +116,7 @@ public ModelServiceImpl(ModelRepository modelRepository, DatabaseService databas
     @Override
     @Transactional
     public ModelResp createModel(ModelReq modelReq, User user) throws Exception {
-        checkParams(modelReq);
+        // checkParams(modelReq);
         ModelDO modelDO = ModelConverter.convert(modelReq, user);
         modelRepository.createModel(modelDO);
         batchCreateDimension(modelDO, user);
@@ -140,7 +140,7 @@ public List<ModelResp> createModel(ModelBuildReq modelBuildReq, User user) throw
     @Override
     @Transactional
     public ModelResp updateModel(ModelReq modelReq, User user) throws Exception {
-        checkParams(modelReq);
+        // checkParams(modelReq);
         checkRelations(modelReq);
         ModelDO modelDO = modelRepository.getModelById(modelReq.getId());
         ModelConverter.convert(modelDO, modelReq, user);

diff --git a/...rver/src/main/java/com/tencent/supersonic/headless/server/utils/CoreComponentFactory.java b/...rver/src/main/java/com/tencent/supersonic/headless/server/utils/CoreComponentFactory.java
@@ -15,15 +15,12 @@ public class CoreComponentFactory extends ComponentFactory {
 
     private static List<SemanticModeller> semanticModellers = new ArrayList<>();
 
-    public static List<SemanticModeller> getSemanticModellers() {
-        if (semanticModellers.isEmpty()) {
-            initSemanticModellers();
-        }
-        return semanticModellers;
+    static {
+        init(SemanticModeller.class, semanticModellers);
     }
 
-    private static void initSemanticModellers() {
-        init(SemanticModeller.class, semanticModellers);
+    public static List<SemanticModeller> getSemanticModellers() {
+        return semanticModellers;
     }
 
 }
diff --git a/...s/server/src/main/java/com/tencent/supersonic/headless/server/utils/MetricCheckUtils.java b/...s/server/src/main/java/com/tencent/supersonic/headless/server/utils/MetricCheckUtils.java
@@ -33,9 +33,6 @@ public static void checkParam(MetricReq metricReq) {
                 throw new InvalidArgumentException("指标定义参数不可为空");
             }
             expr = typeParams.getExpr();
-            if (CollectionUtils.isEmpty(typeParams.getMeasures())) {
-                throw new InvalidArgumentException("定义指标的度量列表参数不可为空");
-            }
             if (hasAggregateFunction(expr)) {
                 throw new InvalidArgumentException("基于度量来创建指标,表达式中不可再包含聚合函数");
             }