You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@phoenix.apache.org by ch...@apache.org on 2020/03/28 00:47:41 UTC

[phoenix] branch master updated: PHOENIX-5801: Connection leak when creating a view with a where condition

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

chinmayskulkarni pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/phoenix.git


The following commit(s) were added to refs/heads/master by this push:
     new 8247029  PHOENIX-5801: Connection leak when creating a view with a where condition
8247029 is described below

commit 8247029ea8e415d82610c55b9ab3515c3f176ac2
Author: Chinmay Kulkarni <ch...@gmail.com>
AuthorDate: Thu Mar 26 21:36:21 2020 -0700

    PHOENIX-5801: Connection leak when creating a view with a where condition
---
 .../phoenix/monitoring/PhoenixMetricsIT.java       | 15 +++++
 .../phoenix/coprocessor/WhereConstantParser.java   | 75 +++++++++++-----------
 2 files changed, 54 insertions(+), 36 deletions(-)

diff --git a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java
index 48a02e6..ac21865 100644
--- a/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java
+++ b/phoenix-core/src/it/java/org/apache/phoenix/monitoring/PhoenixMetricsIT.java
@@ -644,6 +644,21 @@ public class PhoenixMetricsIT extends BasePhoenixMetricsIT {
     }
 
     @Test
+    public void createViewWithWhereConditionNoConnLeak() throws SQLException {
+        resetGlobalMetrics();
+        String tableName = generateUniqueName();
+        String viewName = generateUniqueName();
+        try (Connection conn = DriverManager.getConnection(getUrl())) {
+            conn.createStatement().execute("CREATE TABLE " + tableName +
+                    " (K INTEGER PRIMARY KEY, V VARCHAR(10))");
+            conn.createStatement().execute("CREATE VIEW " + viewName +
+                    " AS SELECT * FROM " + tableName + " WHERE K = 1");
+        }
+        assertTrue(PhoenixRuntime.areGlobalClientMetricsBeingCollected());
+        assertEquals(0, GLOBAL_OPEN_PHOENIX_CONNECTIONS.getMetric().getValue());
+    }
+
+    @Test
     public void testClosingConnectionClearsMetrics() throws Exception {
         Connection conn = null;
         try {
diff --git a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/WhereConstantParser.java b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/WhereConstantParser.java
index c12ad18..ae6b865 100644
--- a/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/WhereConstantParser.java
+++ b/phoenix-core/src/main/java/org/apache/phoenix/coprocessor/WhereConstantParser.java
@@ -50,7 +50,6 @@ import com.google.common.collect.Lists;
 public class WhereConstantParser {
 
     public static PTable addViewInfoToPColumnsIfNeeded(PTable view) throws SQLException {
-    	boolean[] viewColumnConstantsMatched = new boolean[view.getColumns().size()];
         byte[][] viewColumnConstantsToBe = new byte[view.getColumns().size()][];
         if (view.getViewStatement() == null) {
         	return view;
@@ -58,44 +57,48 @@ public class WhereConstantParser {
         SelectStatement select = new SQLParser(view.getViewStatement()).parseQuery();
         ParseNode whereNode = select.getWhere();
         ColumnResolver resolver = FromCompiler.getResolver(new TableRef(view));
-        StatementContext context = new StatementContext(new PhoenixStatement(getConnectionlessConnection()), resolver);
-        Expression expression = null;
-        try {
-        	expression = WhereCompiler.compile(context, whereNode);
-        }
-        catch (ColumnNotFoundException e) {
-        	// if we could not find a column used in the view statement (which means its was dropped)
-        	// this view is not valid any more
-        	return null;
-        }
-        CreateTableCompiler.ViewWhereExpressionVisitor visitor =
-            new CreateTableCompiler.ViewWhereExpressionVisitor(view, viewColumnConstantsToBe);
-        expression.accept(visitor);
-        
-        BitSet isViewColumnReferencedToBe = new BitSet(view.getColumns().size());
-        // Used to track column references in a view
-        ExpressionCompiler expressionCompiler = new CreateTableCompiler.ColumnTrackingExpressionCompiler(context, isViewColumnReferencedToBe);
-        whereNode.accept(expressionCompiler);
-        
-        List<PColumn> result = Lists.newArrayList();
-        for (PColumn column : PTableImpl.getColumnsToClone(view)) {
-        	boolean isViewReferenced = isViewColumnReferencedToBe.get(column.getPosition());
-        	if ( (visitor.isUpdatable() || view.getPKColumns().get(MetaDataUtil.getAutoPartitionColIndex(view)).equals(column)) 
-        			&& viewColumnConstantsToBe[column.getPosition()] != null) {
-				result.add(new PColumnImpl(column, viewColumnConstantsToBe[column.getPosition()], isViewReferenced));
-				viewColumnConstantsMatched[column.getPosition()]=true;
+
+        try (PhoenixConnection conn = getConnectionlessConnection()) {
+            StatementContext context = new StatementContext(new PhoenixStatement(conn), resolver);
+
+            Expression expression;
+            try {
+                expression = WhereCompiler.compile(context, whereNode);
+            } catch (ColumnNotFoundException e) {
+                // if we could not find a column used in the view statement
+                // (which means its was dropped) this view is not valid any more
+                return null;
             }
-        	// If view is not updatable, viewColumnConstants should be empty. We will still
-            // inherit our parent viewConstants, but we have no additional ones.
-        	else if(isViewReferenced ){
-        		result.add(new PColumnImpl(column, column.getViewConstant(), isViewReferenced));
-        	}
-        	else {
-                result.add(column);
+            CreateTableCompiler.ViewWhereExpressionVisitor visitor = new CreateTableCompiler
+                    .ViewWhereExpressionVisitor(view, viewColumnConstantsToBe);
+            expression.accept(visitor);
+
+            BitSet isViewColumnReferencedToBe = new BitSet(view.getColumns().size());
+            // Used to track column references in a view
+            ExpressionCompiler expressionCompiler = new CreateTableCompiler
+                    .ColumnTrackingExpressionCompiler(context, isViewColumnReferencedToBe);
+            whereNode.accept(expressionCompiler);
+
+            List<PColumn> result = Lists.newArrayList();
+            for (PColumn column : PTableImpl.getColumnsToClone(view)) {
+                boolean isViewReferenced = isViewColumnReferencedToBe.get(column.getPosition());
+                if ((visitor.isUpdatable() || view.getPKColumns()
+                        .get(MetaDataUtil.getAutoPartitionColIndex(view)).equals(column))
+                        && viewColumnConstantsToBe[column.getPosition()] != null) {
+                    result.add(new PColumnImpl(column,
+                            viewColumnConstantsToBe[column.getPosition()], isViewReferenced));
+                }
+                // If view is not updatable, viewColumnConstants should be empty. We will still
+                // inherit our parent viewConstants, but we have no additional ones.
+                else if (isViewReferenced ){
+                    result.add(new PColumnImpl(column, column.getViewConstant(), isViewReferenced));
+                } else {
+                    result.add(column);
+                }
             }
+            return PTableImpl.builderWithColumns(view, result)
+                    .build();
         }
-        return PTableImpl.builderWithColumns(view, result)
-                .build();
     }
 
     private static PhoenixConnection getConnectionlessConnection() throws SQLException {