You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kylin.apache.org by ni...@apache.org on 2020/02/07 14:26:10 UTC

[kylin] 15/44: KYLIN-4260 When using server side PreparedStatement cache, the query result are not match on TopN scenario

This is an automated email from the ASF dual-hosted git repository.

nic pushed a commit to branch 3.0.x
in repository https://gitbox.apache.org/repos/asf/kylin.git

commit e8e1a20c902d756c66a22097c63828b93fec6334
Author: mawu2 <ma...@cisco.com>
AuthorDate: Sat Nov 30 16:23:18 2019 +0800

    KYLIN-4260 When using server side PreparedStatement cache, the query result are not match on TopN scenario
---
 .../java/org/apache/kylin/measure/topn/TopNMeasureType.java   |  5 +++++
 .../java/org/apache/kylin/metadata/realization/SQLDigest.java |  4 +++-
 .../org/apache/kylin/storage/hybrid/HybridInstanceTest.java   |  1 +
 .../java/org/apache/kylin/storage/hbase/ITStorageTest.java    |  2 +-
 .../org/apache/kylin/query/enumerator/OLAPEnumerator.java     |  5 ++++-
 .../main/java/org/apache/kylin/query/relnode/OLAPContext.java |  3 ++-
 .../main/java/org/apache/kylin/rest/service/QueryService.java | 11 +++++++++++
 7 files changed, 27 insertions(+), 4 deletions(-)

diff --git a/core-metadata/src/main/java/org/apache/kylin/measure/topn/TopNMeasureType.java b/core-metadata/src/main/java/org/apache/kylin/measure/topn/TopNMeasureType.java
index 472de3c..0586370 100644
--- a/core-metadata/src/main/java/org/apache/kylin/measure/topn/TopNMeasureType.java
+++ b/core-metadata/src/main/java/org/apache/kylin/measure/topn/TopNMeasureType.java
@@ -395,6 +395,11 @@ public class TopNMeasureType extends MeasureType<TopNCounter<ByteArray>> {
 
     @Override
     public void adjustSqlDigest(List<MeasureDesc> measureDescs, SQLDigest sqlDigest) {
+        // If sqlDiegest is already adjusted, then not to adjust it again.
+        if (sqlDigest.isBorrowedContext) {
+            return;
+        }
+
         if (sqlDigest.aggregations.size() > 1) {
             return;
         }
diff --git a/core-metadata/src/main/java/org/apache/kylin/metadata/realization/SQLDigest.java b/core-metadata/src/main/java/org/apache/kylin/metadata/realization/SQLDigest.java
index 78f0adc..875068b 100644
--- a/core-metadata/src/main/java/org/apache/kylin/metadata/realization/SQLDigest.java
+++ b/core-metadata/src/main/java/org/apache/kylin/metadata/realization/SQLDigest.java
@@ -80,6 +80,7 @@ public class SQLDigest {
     public List<TblColRef> sortColumns;
     public List<OrderEnum> sortOrders;
     public boolean isRawQuery;
+    public boolean isBorrowedContext;
     public boolean limitPrecedesAggr;
     public boolean hasLimit;
 
@@ -92,7 +93,7 @@ public class SQLDigest {
             List<DynamicFunctionDesc> dynAggregations, //
             Set<TblColRef> rtDimensionColumns, Set<TblColRef> rtMetricColumns, // dynamic col related columns
             Set<TblColRef> filterColumns, TupleFilter filter, TupleFilter havingFilter, // filter
-            List<TblColRef> sortColumns, List<OrderEnum> sortOrders, boolean limitPrecedesAggr, boolean hasLimit, // sort & limit
+            List<TblColRef> sortColumns, List<OrderEnum> sortOrders, boolean limitPrecedesAggr, boolean hasLimit, boolean isBorrowedContext, // sort & limit
             Set<MeasureDesc> involvedMeasure
     ) {
         this.factTable = factTable;
@@ -121,6 +122,7 @@ public class SQLDigest {
         this.sortColumns = sortColumns;
         this.sortOrders = sortOrders;
         this.isRawQuery = isRawQuery();
+        this.isBorrowedContext = isBorrowedContext;
         this.limitPrecedesAggr = limitPrecedesAggr;
         this.hasLimit = hasLimit;
 
diff --git a/core-storage/src/test/java/org/apache/kylin/storage/hybrid/HybridInstanceTest.java b/core-storage/src/test/java/org/apache/kylin/storage/hybrid/HybridInstanceTest.java
index 99aede4..04e938a 100644
--- a/core-storage/src/test/java/org/apache/kylin/storage/hybrid/HybridInstanceTest.java
+++ b/core-storage/src/test/java/org/apache/kylin/storage/hybrid/HybridInstanceTest.java
@@ -74,6 +74,7 @@ public class HybridInstanceTest {
                 new LinkedList<>(),
                 true,
                 true,
+                false,
                 new LinkedHashSet<>());
         CapabilityResult capabilityResult = hybridInstance.isCapable(sQLDigest);
 
diff --git a/kylin-it/src/test/java/org/apache/kylin/storage/hbase/ITStorageTest.java b/kylin-it/src/test/java/org/apache/kylin/storage/hbase/ITStorageTest.java
index 9022016..3e174bb 100644
--- a/kylin-it/src/test/java/org/apache/kylin/storage/hbase/ITStorageTest.java
+++ b/kylin-it/src/test/java/org/apache/kylin/storage/hbase/ITStorageTest.java
@@ -148,7 +148,7 @@ public class ITStorageTest extends HBaseMetadataTestCase {
                     /*runtimeDimensionColumns*/ Collections.<TblColRef> emptySet(), //
                     /*runtimeMetricColumns*/ Collections.<TblColRef> emptySet(), //
                     /*filter col*/ Collections.<TblColRef> emptySet(), filter, null, //
-                    /*sortCol*/ new ArrayList<TblColRef>(), new ArrayList<SQLDigest.OrderEnum>(), false, false, new HashSet<MeasureDesc>());
+                    /*sortCol*/ new ArrayList<TblColRef>(), new ArrayList<SQLDigest.OrderEnum>(), false, false, false, new HashSet<MeasureDesc>());
             iterator = storageEngine.search(context, sqlDigest, mockup.newTupleInfo(groups, aggregations));
             while (iterator.hasNext()) {
                 ITuple tuple = iterator.next();
diff --git a/query/src/main/java/org/apache/kylin/query/enumerator/OLAPEnumerator.java b/query/src/main/java/org/apache/kylin/query/enumerator/OLAPEnumerator.java
index 3f7beff..129be02 100644
--- a/query/src/main/java/org/apache/kylin/query/enumerator/OLAPEnumerator.java
+++ b/query/src/main/java/org/apache/kylin/query/enumerator/OLAPEnumerator.java
@@ -106,7 +106,10 @@ public class OLAPEnumerator implements Enumerator<Object[]> {
         // bind dynamic variables
         olapContext.bindVariable(optiqContext);
 
-        olapContext.resetSQLDigest();
+        // If olapContext is cached, then inherit it.
+        if (!olapContext.isBorrowedContext) {
+            olapContext.resetSQLDigest();
+        }
         SQLDigest sqlDigest = olapContext.getSQLDigest();
 
         // query storage engine
diff --git a/query/src/main/java/org/apache/kylin/query/relnode/OLAPContext.java b/query/src/main/java/org/apache/kylin/query/relnode/OLAPContext.java
index 59f84df..39c3472 100755
--- a/query/src/main/java/org/apache/kylin/query/relnode/OLAPContext.java
+++ b/query/src/main/java/org/apache/kylin/query/relnode/OLAPContext.java
@@ -157,6 +157,7 @@ public class OLAPContext {
     public TupleFilter havingFilter;
     public List<JoinDesc> joins = new LinkedList<>();
     public JoinsTree joinsTree;
+    public boolean isBorrowedContext = false; // Whether preparedContext is borrowed from cache
     List<TblColRef> sortColumns;
     List<SQLDigest.OrderEnum> sortOrders;
 
@@ -200,7 +201,7 @@ public class OLAPContext {
                     metricsColumns, aggregations, aggrSqlCalls, dynFuncs, // aggregation
                     rtDimColumns, rtMetricColumns, // runtime related columns
                     filterColumns, filter, havingFilter, // filter
-                    sortColumns, sortOrders, limitPrecedesAggr, hasLimit, // sort & limit
+                    sortColumns, sortOrders, limitPrecedesAggr, hasLimit, isBorrowedContext, // sort & limit
                     involvedMeasure);
         }
         return sqlDigest;
diff --git a/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java b/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java
index 293b421..0ce8379 100644
--- a/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java
+++ b/server-base/src/main/java/org/apache/kylin/rest/service/QueryService.java
@@ -696,6 +696,13 @@ public class QueryService extends BasicService {
             DBUtils.closeQuietly(conn);
             if (preparedContext != null) {
                 if (borrowPrepareContext) {
+                    // Set tag isBorrowedContext true, when return preparedContext back
+                    for (OLAPContext olapContext : preparedContext.olapContexts) {
+                        if (borrowPrepareContext) {
+                            olapContext.isBorrowedContext = true;
+                        }
+                    }
+
                     preparedContextPool.returnObject(preparedContextKey, preparedContext);
                 } else {
                     preparedContext.close();
@@ -1253,6 +1260,10 @@ public class QueryService extends BasicService {
         Connection conn = QueryConnection.getConnection(project);
         PreparedStatement preparedStatement = conn.prepareStatement(sql);
         Collection<OLAPContext> olapContexts = OLAPContext.getThreadLocalContexts();
+        // If the preparedContext is first initialized, then set the borrowed tag to false
+        for (OLAPContext olapContext : olapContexts) {
+            olapContext.isBorrowedContext = false;
+        }
         return new PreparedContext(conn, preparedStatement, olapContexts);
     }