PHOENIX-5124 PropertyPolicyProvider should not evaluate default hbase config properties
authorThomas D'Silva <tdsilva@apache.org>
Tue, 5 Feb 2019 07:17:37 +0000 (23:17 -0800)
committerThomas D'Silva <tdsilva@apache.org>
Fri, 8 Feb 2019 00:28:31 +0000 (16:28 -0800)
phoenix-core/src/it/java/org/apache/phoenix/end2end/AppendOnlySchemaIT.java
phoenix-core/src/it/java/org/apache/phoenix/end2end/PropertyPolicyProviderIT.java [new file with mode: 0644]
phoenix-core/src/it/java/org/apache/phoenix/rpc/UpdateCacheIT.java
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixConnection.java
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixEmbeddedDriver.java
phoenix-core/src/main/java/org/apache/phoenix/util/PropertiesUtil.java

index b39c4f0..e1c56ea 100644 (file)
@@ -70,7 +70,7 @@ public class AppendOnlySchemaIT extends ParallelStatsDisabledIT {
                 Mockito.spy(driver.getConnectionQueryServices(getUrl(),
                     PropertiesUtil.deepCopy(TEST_PROPERTIES)));
         Properties props = new Properties();
-        props.putAll(PhoenixEmbeddedDriver.DEFFAULT_PROPS.asMap());
+        props.putAll(PhoenixEmbeddedDriver.DEFAULT_PROPS.asMap());
 
         try (Connection conn1 = connectionQueryServices.connect(getUrl(), props);
                 Connection conn2 = sameClient ? conn1 : connectionQueryServices.connect(getUrl(), props)) {
diff --git a/phoenix-core/src/it/java/org/apache/phoenix/end2end/PropertyPolicyProviderIT.java b/phoenix-core/src/it/java/org/apache/phoenix/end2end/PropertyPolicyProviderIT.java
new file mode 100644 (file)
index 0000000..48508a9
--- /dev/null
@@ -0,0 +1,26 @@
+package org.apache.phoenix.end2end;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseConfiguration;
+import org.apache.hadoop.hbase.HConstants;
+import org.apache.phoenix.mapreduce.util.ConnectionUtil;
+import org.junit.Test;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.Properties;
+
+public class PropertyPolicyProviderIT  extends ParallelStatsDisabledIT {
+
+    @Test
+    public void testUsingDefaultHBaseConfigs() throws SQLException {
+        Configuration config = HBaseConfiguration.create();
+        config.set(HConstants.ZOOKEEPER_QUORUM, getUrl());
+        Properties properties=new Properties();
+        properties.put("allowedProperty","value");
+        try(
+                Connection conn = ConnectionUtil.getInputConnection(config, properties)
+        ){}
+    }
+
+}
index 2959b99..a1bdad7 100644 (file)
@@ -192,7 +192,7 @@ public class UpdateCacheIT extends ParallelStatsDisabledIT {
                // use a spyed ConnectionQueryServices so we can verify calls to getTable
                ConnectionQueryServices connectionQueryServices = Mockito.spy(driver.getConnectionQueryServices(getUrl(), PropertiesUtil.deepCopy(TEST_PROPERTIES)));
                Properties props = new Properties();
-               props.putAll(PhoenixEmbeddedDriver.DEFFAULT_PROPS.asMap());
+               props.putAll(PhoenixEmbeddedDriver.DEFAULT_PROPS.asMap());
                Connection conn = connectionQueryServices.connect(getUrl(), props);
                try {
                        conn.setAutoCommit(false);
index 596e27c..d74ffff 100644 (file)
@@ -245,7 +245,7 @@ public class PhoenixConnection implements Connection, MetaDataMutated, SQLClosea
 
         // Filter user provided properties based on property policy, if
         // provided.
-        PropertyPolicyProvider.getPropertyPolicy().evaluate(info);
+        PropertyPolicyProvider.getPropertyPolicy().evaluate(PropertiesUtil.removeStandardHBasePhoenixConfig(info));
 
         // Copy so client cannot change
         this.info = info == null ? new Properties() : PropertiesUtil
index 00dfb5a..2669360 100644 (file)
@@ -19,7 +19,6 @@ package org.apache.phoenix.jdbc;
 
 import static org.apache.phoenix.util.PhoenixRuntime.PHOENIX_TEST_DRIVER_URL_PARAM;
 
-import java.io.File;
 import java.io.IOException;
 import java.sql.Connection;
 import java.sql.Driver;
@@ -37,7 +36,6 @@ import javax.annotation.concurrent.Immutable;
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.fs.Path;
 import org.apache.hadoop.hbase.HConstants;
 import org.apache.hadoop.hbase.security.User;
 import org.apache.hadoop.security.UserGroupInformation;
@@ -50,7 +48,6 @@ import org.apache.phoenix.query.HBaseFactoryProvider;
 import org.apache.phoenix.query.QueryServices;
 import org.apache.phoenix.util.PhoenixRuntime;
 import org.apache.phoenix.util.PropertiesUtil;
-import org.apache.phoenix.util.QueryUtil;
 import org.apache.phoenix.util.ReadOnlyProps;
 import org.apache.phoenix.util.SQLCloseable;
 import org.slf4j.LoggerFactory;
@@ -85,7 +82,7 @@ public abstract class PhoenixEmbeddedDriver implements Driver, SQLCloseable {
     public final static String MINOR_VERSION_PROP = "DriverMinorVersion";
     public final static String DRIVER_NAME_PROP = "DriverName";
     
-    public static final ReadOnlyProps DEFFAULT_PROPS = new ReadOnlyProps(
+    public static final ReadOnlyProps DEFAULT_PROPS = new ReadOnlyProps(
             ImmutableMap.of(
                     MAJOR_VERSION_PROP, Integer.toString(MetaDataProtocol.PHOENIX_MAJOR_VERSION),
                     MINOR_VERSION_PROP, Integer.toString(MetaDataProtocol.PHOENIX_MINOR_VERSION),
@@ -95,7 +92,7 @@ public abstract class PhoenixEmbeddedDriver implements Driver, SQLCloseable {
     }
     
     protected ReadOnlyProps getDefaultProps() {
-        return DEFFAULT_PROPS;
+        return DEFAULT_PROPS;
     }
     
     abstract public QueryServices getQueryServices() throws SQLException;
index 685b8cb..b029a26 100644 (file)
@@ -18,6 +18,7 @@
 package org.apache.phoenix.util;
 
 import java.util.Collections;
+import java.util.Enumeration;
 import java.util.Iterator;
 import java.util.Map;
 import java.util.Map.Entry;
@@ -25,6 +26,8 @@ import java.util.Properties;
 import java.util.Set;
 
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.HBaseConfiguration;
+import org.apache.phoenix.jdbc.PhoenixEmbeddedDriver;
 
 public class PropertiesUtil {
 
@@ -55,7 +58,7 @@ public class PropertiesUtil {
     public static Properties combineProperties(Properties props, final Configuration conf) {
         return combineProperties(props, conf, Collections.<String>emptySet());
     }
-    
+
     public static Properties combineProperties(Properties props, final Configuration conf, Set<String> withoutTheseProps) {
         Iterator<Map.Entry<String, String>> iterator = conf.iterator();
         Properties copy = deepCopy(props);
@@ -71,6 +74,29 @@ public class PropertiesUtil {
         return copy;
     }
 
+    /**
+     * Removes properties present that are present in standard HBase configuration and standard Phoenix properties
+     */
+    public static Properties removeStandardHBasePhoenixConfig(Properties props) {
+        Configuration config = HBaseConfiguration.create();
+        Properties normalizedProps  = new Properties();
+        for(Entry entry: props.entrySet()) {
+            if ( entry.getKey() instanceof String) {
+                String propName = (String) entry.getKey();
+                if (config.get(propName) == null
+                        && PhoenixEmbeddedDriver.DEFAULT_PROPS.get(propName) == null
+                        && !propName.equals(PhoenixRuntime.CURRENT_SCN_ATTRIB)
+                        && !propName.equals(PhoenixRuntime.TENANT_ID_ATTRIB)) {
+                    normalizedProps.put(propName, props.getProperty(propName));
+                }
+            }
+            else {
+                normalizedProps.put(entry.getKey(), entry.getValue());
+            }
+        }
+        return normalizedProps;
+    }
+
    /**
      * Utility to work around the limitation of the copy constructor
      * {@link Configuration#Configuration(Configuration)} provided by the {@link Configuration}