You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@calcite.apache.org by jh...@apache.org on 2019/10/24 23:16:29 UTC
[calcite] 02/02: [CALCITE-3436] In CalciteConnectionConfigImpl,
add isSet and unset methods (Ryan Fu)
This is an automated email from the ASF dual-hosted git repository.
jhyde pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/calcite.git
commit 2c6ccd50ec90e14c1320c8f049a08c4b5de515fa
Author: Ryan Fu <ry...@looker.com>
AuthorDate: Thu Oct 17 10:34:46 2019 -0700
[CALCITE-3436] In CalciteConnectionConfigImpl, add isSet and unset methods (Ryan Fu)
The 'isSet' method allows Planner to set "conformance" and "caseSensitive"
connection properties based on the parser configuration only if they
have not been explicitly set in the connection properties.
In PlannerImpl, store 'costFactory' and no longer store
'frameworkConfig'; it might prevent memory leaks.
Close apache/calcite#1528
---
.../config/CalciteConnectionConfigImpl.java | 28 ++++++--
.../org/apache/calcite/prepare/PlannerImpl.java | 30 ++++----
.../org/apache/calcite/tools/FrameworksTest.java | 81 ++++++++++++++++++++++
3 files changed, 123 insertions(+), 16 deletions(-)
diff --git a/core/src/main/java/org/apache/calcite/config/CalciteConnectionConfigImpl.java b/core/src/main/java/org/apache/calcite/config/CalciteConnectionConfigImpl.java
index 8353dc5..1d03e38 100644
--- a/core/src/main/java/org/apache/calcite/config/CalciteConnectionConfigImpl.java
+++ b/core/src/main/java/org/apache/calcite/config/CalciteConnectionConfigImpl.java
@@ -37,12 +37,32 @@ public class CalciteConnectionConfigImpl extends ConnectionConfigImpl
super(properties);
}
- /** Returns a copy of this configuration with one property changed. */
+ /** Returns a copy of this configuration with one property changed.
+ *
+ * <p>Does not modify this configuration. */
public CalciteConnectionConfigImpl set(CalciteConnectionProperty property,
String value) {
- final Properties properties1 = new Properties(properties);
- properties1.setProperty(property.camelName(), value);
- return new CalciteConnectionConfigImpl(properties1);
+ final Properties newProperties = (Properties) properties.clone();
+ newProperties.setProperty(property.camelName(), value);
+ return new CalciteConnectionConfigImpl(newProperties);
+ }
+
+ /** Returns a copy of this configuration with the value of a property
+ * removed.
+ *
+ * <p>Does not modify this configuration. */
+ public CalciteConnectionConfigImpl unset(CalciteConnectionProperty property) {
+ final Properties newProperties = (Properties) properties.clone();
+ newProperties.remove(property.camelName());
+ return new CalciteConnectionConfigImpl(newProperties);
+ }
+
+ /** Returns whether a given property has been assigned a value.
+ *
+ * <p>If not, the value returned for the property will be its default value.
+ */
+ public boolean isSet(CalciteConnectionProperty property) {
+ return properties.containsKey(property.camelName());
}
public boolean approximateDistinctCount() {
diff --git a/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java b/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java
index ec0f8d9..9acbc42 100644
--- a/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java
+++ b/core/src/main/java/org/apache/calcite/prepare/PlannerImpl.java
@@ -26,6 +26,7 @@ import org.apache.calcite.jdbc.JavaTypeFactoryImpl;
import org.apache.calcite.plan.Context;
import org.apache.calcite.plan.ConventionTraitDef;
import org.apache.calcite.plan.RelOptCluster;
+import org.apache.calcite.plan.RelOptCostFactory;
import org.apache.calcite.plan.RelOptPlanner;
import org.apache.calcite.plan.RelOptTable.ViewExpander;
import org.apache.calcite.plan.RelOptUtil;
@@ -69,7 +70,7 @@ import java.util.Properties;
public class PlannerImpl implements Planner, ViewExpander {
private final SqlOperatorTable operatorTable;
private final ImmutableList<Program> programs;
- private final FrameworkConfig frameworkConfig;
+ private final RelOptCostFactory costFactory;
private final Context context;
private final CalciteConnectionConfig connectionConfig;
@@ -101,7 +102,7 @@ public class PlannerImpl implements Planner, ViewExpander {
/** Creates a planner. Not a public API; call
* {@link org.apache.calcite.tools.Frameworks#getPlanner} instead. */
public PlannerImpl(FrameworkConfig config) {
- this.frameworkConfig = config;
+ this.costFactory = config.getCostFactory();
this.defaultSchema = config.getDefaultSchema();
this.operatorTable = config.getOperatorTable();
this.programs = config.getPrograms();
@@ -116,17 +117,22 @@ public class PlannerImpl implements Planner, ViewExpander {
reset();
}
+ /** Gets a user defined config and appends default connection values */
private CalciteConnectionConfig connConfig() {
- CalciteConnectionConfig unwrapped = context.unwrap(CalciteConnectionConfig.class);
- if (unwrapped != null) {
- return unwrapped;
+ CalciteConnectionConfigImpl config =
+ context.unwrap(CalciteConnectionConfigImpl.class);
+ if (config == null) {
+ config = new CalciteConnectionConfigImpl(new Properties());
}
- Properties properties = new Properties();
- properties.setProperty(CalciteConnectionProperty.CASE_SENSITIVE.camelName(),
- String.valueOf(parserConfig.caseSensitive()));
- properties.setProperty(CalciteConnectionProperty.CONFORMANCE.camelName(),
- String.valueOf(frameworkConfig.getParserConfig().conformance()));
- return new CalciteConnectionConfigImpl(properties);
+ if (!config.isSet(CalciteConnectionProperty.CASE_SENSITIVE)) {
+ config = config.set(CalciteConnectionProperty.CASE_SENSITIVE,
+ String.valueOf(parserConfig.caseSensitive()));
+ }
+ if (!config.isSet(CalciteConnectionProperty.CONFORMANCE)) {
+ config = config.set(CalciteConnectionProperty.CONFORMANCE,
+ String.valueOf(parserConfig.conformance()));
+ }
+ return config;
}
/** Makes sure that the state is at least the given state. */
@@ -168,7 +174,7 @@ public class PlannerImpl implements Planner, ViewExpander {
connectionConfig.typeSystem(RelDataTypeSystem.class,
RelDataTypeSystem.DEFAULT);
typeFactory = new JavaTypeFactoryImpl(typeSystem);
- planner = new VolcanoPlanner(frameworkConfig.getCostFactory(), context);
+ planner = new VolcanoPlanner(costFactory, context);
RelOptUtil.registerDefaultRules(planner,
connectionConfig.materializationsEnabled(),
Hook.ENABLE_BINDABLE.get(false));
diff --git a/core/src/test/java/org/apache/calcite/tools/FrameworksTest.java b/core/src/test/java/org/apache/calcite/tools/FrameworksTest.java
index a1830ac..8b188ea 100644
--- a/core/src/test/java/org/apache/calcite/tools/FrameworksTest.java
+++ b/core/src/test/java/org/apache/calcite/tools/FrameworksTest.java
@@ -19,6 +19,8 @@ package org.apache.calcite.tools;
import org.apache.calcite.DataContext;
import org.apache.calcite.adapter.enumerable.EnumerableConvention;
import org.apache.calcite.adapter.enumerable.EnumerableTableScan;
+import org.apache.calcite.config.CalciteConnectionConfigImpl;
+import org.apache.calcite.config.CalciteConnectionProperty;
import org.apache.calcite.config.CalciteSystemProperty;
import org.apache.calcite.linq4j.Enumerable;
import org.apache.calcite.linq4j.QueryProvider;
@@ -79,9 +81,11 @@ import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
+import java.util.Properties;
import static org.hamcrest.CoreMatchers.equalTo;
import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.nullValue;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertThat;
import static org.junit.Assert.fail;
@@ -244,6 +248,83 @@ public class FrameworksTest {
}
}
+ /** Unit test for {@link CalciteConnectionConfigImpl#set}
+ * and {@link CalciteConnectionConfigImpl#isSet}. */
+ @Test public void testConnectionConfig() {
+ final CalciteConnectionProperty forceDecorrelate =
+ CalciteConnectionProperty.FORCE_DECORRELATE;
+ final CalciteConnectionProperty lenientOperatorLookup =
+ CalciteConnectionProperty.LENIENT_OPERATOR_LOOKUP;
+ final CalciteConnectionProperty caseSensitive =
+ CalciteConnectionProperty.CASE_SENSITIVE;
+ final CalciteConnectionProperty model = CalciteConnectionProperty.MODEL;
+
+ final Properties p = new Properties();
+ p.setProperty(forceDecorrelate.camelName(),
+ Boolean.toString(false));
+ p.setProperty(lenientOperatorLookup.camelName(),
+ Boolean.toString(false));
+
+ final CalciteConnectionConfigImpl c = new CalciteConnectionConfigImpl(p);
+
+ assertThat(c.lenientOperatorLookup(), is(false));
+ assertThat(c.isSet(lenientOperatorLookup), is(true));
+ assertThat(c.caseSensitive(), is(true));
+ assertThat(c.isSet(caseSensitive), is(false));
+ assertThat(c.forceDecorrelate(), is(false));
+ assertThat(c.isSet(forceDecorrelate), is(true));
+ assertThat(c.model(), nullValue());
+ assertThat(c.isSet(model), is(false));
+
+ final CalciteConnectionConfigImpl c2 = c
+ .set(lenientOperatorLookup, Boolean.toString(true))
+ .set(caseSensitive, Boolean.toString(true));
+
+ assertThat(c2.lenientOperatorLookup(), is(true));
+ assertThat(c2.isSet(lenientOperatorLookup), is(true));
+ assertThat("same value as for c", c2.caseSensitive(), is(true));
+ assertThat("set to the default value", c2.isSet(caseSensitive), is(true));
+ assertThat(c2.forceDecorrelate(), is(false));
+ assertThat(c2.isSet(forceDecorrelate), is(true));
+ assertThat(c2.model(), nullValue());
+ assertThat(c2.isSet(model), is(false));
+ assertThat("retrieves default because not set", c2.schema(), nullValue());
+
+ // Create a config similar to c2 but starting from an empty Properties.
+ final CalciteConnectionConfigImpl c3 =
+ new CalciteConnectionConfigImpl(new Properties());
+ final CalciteConnectionConfigImpl c4 = c3
+ .set(lenientOperatorLookup, Boolean.toString(true))
+ .set(caseSensitive, Boolean.toString(true));
+ assertThat(c4.lenientOperatorLookup(), is(true));
+ assertThat(c4.isSet(lenientOperatorLookup), is(true));
+ assertThat(c4.caseSensitive(), is(true));
+ assertThat("set to the default value", c4.isSet(caseSensitive), is(true));
+ assertThat("different from c2", c4.forceDecorrelate(), is(true));
+ assertThat("different from c2", c4.isSet(forceDecorrelate), is(false));
+ assertThat(c4.model(), nullValue());
+ assertThat(c4.isSet(model), is(false));
+ assertThat("retrieves default because not set", c4.schema(), nullValue());
+
+ // Call 'unset' on a few properties.
+ final CalciteConnectionConfigImpl c5 = c2.unset(lenientOperatorLookup);
+ assertThat(c5.isSet(lenientOperatorLookup), is(false));
+ assertThat(c5.lenientOperatorLookup(), is(false));
+ assertThat(c5.isSet(caseSensitive), is(true));
+ assertThat(c5.caseSensitive(), is(true));
+
+ // Call 'set' on properties that have already been set.
+ final CalciteConnectionConfigImpl c6 = c5
+ .set(lenientOperatorLookup, Boolean.toString(false))
+ .set(forceDecorrelate, Boolean.toString(true));
+ assertThat(c6.isSet(lenientOperatorLookup), is(true));
+ assertThat(c6.lenientOperatorLookup(), is(false));
+ assertThat(c6.isSet(caseSensitive), is(true));
+ assertThat(c6.caseSensitive(), is(true));
+ assertThat(c6.isSet(forceDecorrelate), is(true));
+ assertThat(c6.forceDecorrelate(), is(true));
+ }
+
/** Test case for
* <a href="https://issues.apache.org/jira/browse/CALCITE-1996">[CALCITE-1996]
* VALUES syntax</a>.