You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by GitBox <gi...@apache.org> on 2019/12/31 22:19:29 UTC
[GitHub] [phoenix] ntshmah opened a new pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
ntshmah opened a new pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668
…ion level config phoenix.default.update.cache.frequency.
- Introduced a new enum ConnectionProperty (similar to the existing TableProperty)
for connection level properties.
- Added validation code for connection properties at connection creation time.
- Changed the type of DEFAULT_UPDATE_CACHE_FREQUENCY from int to long.
Other changes made as part of this JIRA:
- Updated TableProperty.UPDATE_CACHE_FREQUENCY to raise relevant error message
for invalid table level property
- More tests for valid and invalid table/connection level property.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ntshmah commented on issue #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ntshmah commented on issue #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668#issuecomment-571355933
Thanks @ChinmaySKulkarni. Sure, on it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ntshmah commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ntshmah commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668#discussion_r363020901
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/ConnectionProperty.java
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.schema;
+
+import org.apache.phoenix.query.QueryServicesOptions;
+
+public enum ConnectionProperty {
+ /**
+ * Connection level property phoenix.default.update.cache.frequency
+ */
+ UPDATE_CACHE_FREQUENCY() {
+ @Override
+ public Object getValue(String value) {
+ if (value == null) {
+ return QueryServicesOptions.DEFAULT_UPDATE_CACHE_FREQUENCY;
+ } else {
+ if ("ALWAYS".equalsIgnoreCase(value)) {
+ return 0L;
+ } else if ("NEVER".equalsIgnoreCase(value)) {
+ return Long.MAX_VALUE;
+ } else {
+ try {
+ return Long.parseLong(value);
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException("Connection's phoenix.default.update.cache.frequency can only be set to 'ALWAYS', 'NEVER' or a millisecond numeric value.");
+ }
Review comment:
Done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ntshmah commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ntshmah commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668#discussion_r363020788
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java
##########
@@ -621,55 +623,96 @@ public void testMultiTenantImmutableTableMetadata() throws Exception {
}
}
+ private void verifyUCFValueInSysCat(String tableName, String createTableString, Properties props, long expectedUCFInSysCat) throws SQLException {
+ String readSysCatQuery = "SELECT TABLE_NAME, UPDATE_CACHE_FREQUENCY FROM SYSTEM.CATALOG WHERE "
+ + "TABLE_NAME = '" + tableName + "' AND TABLE_TYPE='u'";
+
+ Connection connection = DriverManager.getConnection(getUrl(), props);
Review comment:
Done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668#discussion_r363002868
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java
##########
@@ -621,55 +623,96 @@ public void testMultiTenantImmutableTableMetadata() throws Exception {
}
}
+ private void verifyUCFValueInSysCat(String tableName, String createTableString, Properties props, long expectedUCFInSysCat) throws SQLException {
+ String readSysCatQuery = "SELECT TABLE_NAME, UPDATE_CACHE_FREQUENCY FROM SYSTEM.CATALOG WHERE "
+ + "TABLE_NAME = '" + tableName + "' AND TABLE_TYPE='u'";
+
+ Connection connection = DriverManager.getConnection(getUrl(), props);
+ connection.createStatement().execute(createTableString);
+ ResultSet rs = connection.createStatement().executeQuery(readSysCatQuery);
+ assertTrue(rs.next());
+ assertEquals(expectedUCFInSysCat, rs.getLong(2));
+ connection.createStatement().execute("drop table " + tableName);
+ connection.close();
+ }
+
@Test
- public void testCreateTableWithUpdateCacheFrequencyAttrib() throws Exception {
- Connection connection = null;
+ public void testCreateTableNoUpdateCacheFreq() throws Exception {
String tableName = generateUniqueName();
- try {
- Properties props = PropertiesUtil.deepCopy(TestUtil.TEST_PROPERTIES);
- connection = DriverManager.getConnection(getUrl(), props);
-
- // Assert update cache frequency to default value zero
- connection.createStatement().execute(
- "create table " + tableName + " (k VARCHAR PRIMARY KEY, v1 VARCHAR, v2 VARCHAR)");
- String readSysCatQuery =
- "select TABLE_NAME,UPDATE_CACHE_FREQUENCY from SYSTEM.CATALOG where "
- + "TABLE_NAME = '" + tableName + "' AND TABLE_TYPE='u'";
- ResultSet rs = connection.createStatement().executeQuery(readSysCatQuery);
- Assert.assertTrue(rs.next());
- Assert.assertEquals(0, rs.getLong(2));
- connection.createStatement().execute("drop table " + tableName);
- connection.close();
-
- // Assert update cache frequency to configured default value 10sec
- int defaultUpdateCacheFrequency = 10000;
- props.put(QueryServices.DEFAULT_UPDATE_CACHE_FREQUENCY_ATRRIB,
- "" + defaultUpdateCacheFrequency);
- connection = DriverManager.getConnection(getUrl(), props);
- connection.createStatement().execute(
- "create table " + tableName + " (k VARCHAR PRIMARY KEY, v1 VARCHAR, v2 VARCHAR)");
- rs = connection.createStatement().executeQuery(readSysCatQuery);
- Assert.assertTrue(rs.next());
- Assert.assertEquals(defaultUpdateCacheFrequency, rs.getLong(2));
- connection.createStatement().execute("drop table " + tableName);
-
- // Assert update cache frequency to table specific value 30sec
- int tableSpecificUpdateCacheFrequency = 30000;
- connection.createStatement()
- .execute("create table " + tableName
- + " (k VARCHAR PRIMARY KEY, v1 VARCHAR, v2 VARCHAR) "
- + "UPDATE_CACHE_FREQUENCY=" + tableSpecificUpdateCacheFrequency);
- rs = connection.createStatement().executeQuery(readSysCatQuery);
- Assert.assertTrue(rs.next());
- Assert.assertEquals(tableSpecificUpdateCacheFrequency, rs.getLong(2));
- } finally {
- if (connection != null) {
- connection.createStatement().execute("drop table if exists " + tableName);
- connection.close();
+ Properties props = PropertiesUtil.deepCopy(TestUtil.TEST_PROPERTIES);
+ String createTableString = "CREATE TABLE " + tableName + " (k VARCHAR PRIMARY KEY,"
+ + "v1 VARCHAR, v2 VARCHAR)";
+ verifyUCFValueInSysCat(tableName, createTableString, props, 0L);
+ }
+
+ @Test
+ public void testCreateTableWithTableLevelUpdateCacheFreq() throws Exception {
+ String tableName = generateUniqueName();
+ Properties props = PropertiesUtil.deepCopy(TestUtil.TEST_PROPERTIES);
+
+ HashMap<String, Long> expectedUCF = new HashMap<>();
+ expectedUCF.put("10", new Long(10L));
+ expectedUCF.put("0", new Long(0L));
+ expectedUCF.put("10000", new Long(10000L));
+ expectedUCF.put("ALWAYS", new Long(0L));
+ expectedUCF.put("NEVER", new Long(Long.MAX_VALUE));
+
+ for (HashMap.Entry<String, Long> entry : expectedUCF.entrySet()) {
+ String tableLevelUCF = entry.getKey();
+ long expectedUCFInSysCat = entry.getValue();
+
+ String createTableString = "CREATE TABLE " + tableName + " (k VARCHAR PRIMARY KEY,"
+ + "v1 VARCHAR, v2 VARCHAR) UPDATE_CACHE_FREQUENCY = " + tableLevelUCF;
+ verifyUCFValueInSysCat(tableName, createTableString, props, expectedUCFInSysCat);
+ }
+ }
+
+ @Test
+ public void testCreateTableWithInvalidTableUpdateCacheFreqShouldThrow() throws Exception {
+ String tableName = generateUniqueName();
+ Properties props = PropertiesUtil.deepCopy(TestUtil.TEST_PROPERTIES);
+
+ ArrayList<String> invalidUCF = new ArrayList<>();
+ invalidUCF.add("GIBBERISH");
+ invalidUCF.add("10000.6");
+
+ for (String tableLevelUCF : invalidUCF) {
+ String createTableString = "CREATE TABLE " + tableName + " (k VARCHAR PRIMARY KEY,"
+ + "v1 VARCHAR, v2 VARCHAR) UPDATE_CACHE_FREQUENCY = " + tableLevelUCF;
+ try {
+ verifyUCFValueInSysCat(tableName, createTableString, props, -1L);
+ fail();
+ } catch (IllegalArgumentException e) {
+ // expected
+ assertEquals("Table's UPDATE_CACHE_FREQUENCY can only be set to 'ALWAYS', 'NEVER' or a millisecond numeric value.",
Review comment:
I dislike comparing actual string messages in the exception, since a subtle change can cause test failures. Is there any other way you can confirm this via a sqlCode comparison for example?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668#discussion_r363003175
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/ConnectionProperty.java
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.schema;
+
+import org.apache.phoenix.query.QueryServicesOptions;
+
+public enum ConnectionProperty {
+ /**
+ * Connection level property phoenix.default.update.cache.frequency
+ */
+ UPDATE_CACHE_FREQUENCY() {
+ @Override
+ public Object getValue(String value) {
+ if (value == null) {
+ return QueryServicesOptions.DEFAULT_UPDATE_CACHE_FREQUENCY;
+ } else {
Review comment:
nit: Don't need an `else` since we return immediately inside the `if` block.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ntshmah closed pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ntshmah closed pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ntshmah commented on issue #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ntshmah commented on issue #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668#issuecomment-570763734
Thanks for the review, @ChinmaySKulkarni. I have addressed your comments, can you please take a look again?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668#discussion_r363003218
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/ConnectionProperty.java
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.schema;
+
+import org.apache.phoenix.query.QueryServicesOptions;
+
+public enum ConnectionProperty {
+ /**
+ * Connection level property phoenix.default.update.cache.frequency
+ */
+ UPDATE_CACHE_FREQUENCY() {
+ @Override
+ public Object getValue(String value) {
+ if (value == null) {
+ return QueryServicesOptions.DEFAULT_UPDATE_CACHE_FREQUENCY;
+ } else {
+ if ("ALWAYS".equalsIgnoreCase(value)) {
+ return 0L;
Review comment:
nit: ditto comment about `else` conditions
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ntshmah commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ntshmah commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668#discussion_r363020889
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/ConnectionProperty.java
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.schema;
+
+import org.apache.phoenix.query.QueryServicesOptions;
+
+public enum ConnectionProperty {
+ /**
+ * Connection level property phoenix.default.update.cache.frequency
+ */
+ UPDATE_CACHE_FREQUENCY() {
+ @Override
+ public Object getValue(String value) {
+ if (value == null) {
+ return QueryServicesOptions.DEFAULT_UPDATE_CACHE_FREQUENCY;
+ } else {
+ if ("ALWAYS".equalsIgnoreCase(value)) {
+ return 0L;
Review comment:
Done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ntshmah commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ntshmah commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668#discussion_r363020877
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/rpc/UpdateCacheIT.java
##########
@@ -244,4 +248,25 @@ private static void validateSelectRowKeyCols(Connection conn, String selectSql,
}
assertFalse(rs.next());
}
+
+ @Test
+ public void testInvalidConnUpdateCacheFrequencyShouldThrow() throws Exception {
+ Properties props = PropertiesUtil.deepCopy(TestUtil.TEST_PROPERTIES);
+
+ ArrayList<String> invalidUCF = new ArrayList<>();
+ invalidUCF.add("GIBBERISH");
+ invalidUCF.add("10000.6");
+
+ for (String connLevelUCF : invalidUCF) {
+ props.put(QueryServices.DEFAULT_UPDATE_CACHE_FREQUENCY_ATRRIB, connLevelUCF);
+ try {
+ DriverManager.getConnection(getUrl(), props);
+ fail();
+ } catch (IllegalArgumentException e) {
+ // expected
+ assertEquals("Connection's phoenix.default.update.cache.frequency can only be set to 'ALWAYS', 'NEVER' or a millisecond numeric value.",
Review comment:
Okay, same here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ntshmah commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ntshmah commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668#discussion_r363020862
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java
##########
@@ -621,55 +623,96 @@ public void testMultiTenantImmutableTableMetadata() throws Exception {
}
}
+ private void verifyUCFValueInSysCat(String tableName, String createTableString, Properties props, long expectedUCFInSysCat) throws SQLException {
+ String readSysCatQuery = "SELECT TABLE_NAME, UPDATE_CACHE_FREQUENCY FROM SYSTEM.CATALOG WHERE "
+ + "TABLE_NAME = '" + tableName + "' AND TABLE_TYPE='u'";
+
+ Connection connection = DriverManager.getConnection(getUrl(), props);
+ connection.createStatement().execute(createTableString);
+ ResultSet rs = connection.createStatement().executeQuery(readSysCatQuery);
+ assertTrue(rs.next());
+ assertEquals(expectedUCFInSysCat, rs.getLong(2));
+ connection.createStatement().execute("drop table " + tableName);
+ connection.close();
+ }
+
@Test
- public void testCreateTableWithUpdateCacheFrequencyAttrib() throws Exception {
- Connection connection = null;
+ public void testCreateTableNoUpdateCacheFreq() throws Exception {
String tableName = generateUniqueName();
- try {
- Properties props = PropertiesUtil.deepCopy(TestUtil.TEST_PROPERTIES);
- connection = DriverManager.getConnection(getUrl(), props);
-
- // Assert update cache frequency to default value zero
- connection.createStatement().execute(
- "create table " + tableName + " (k VARCHAR PRIMARY KEY, v1 VARCHAR, v2 VARCHAR)");
- String readSysCatQuery =
- "select TABLE_NAME,UPDATE_CACHE_FREQUENCY from SYSTEM.CATALOG where "
- + "TABLE_NAME = '" + tableName + "' AND TABLE_TYPE='u'";
- ResultSet rs = connection.createStatement().executeQuery(readSysCatQuery);
- Assert.assertTrue(rs.next());
- Assert.assertEquals(0, rs.getLong(2));
- connection.createStatement().execute("drop table " + tableName);
- connection.close();
-
- // Assert update cache frequency to configured default value 10sec
- int defaultUpdateCacheFrequency = 10000;
- props.put(QueryServices.DEFAULT_UPDATE_CACHE_FREQUENCY_ATRRIB,
- "" + defaultUpdateCacheFrequency);
- connection = DriverManager.getConnection(getUrl(), props);
- connection.createStatement().execute(
- "create table " + tableName + " (k VARCHAR PRIMARY KEY, v1 VARCHAR, v2 VARCHAR)");
- rs = connection.createStatement().executeQuery(readSysCatQuery);
- Assert.assertTrue(rs.next());
- Assert.assertEquals(defaultUpdateCacheFrequency, rs.getLong(2));
- connection.createStatement().execute("drop table " + tableName);
-
- // Assert update cache frequency to table specific value 30sec
- int tableSpecificUpdateCacheFrequency = 30000;
- connection.createStatement()
- .execute("create table " + tableName
- + " (k VARCHAR PRIMARY KEY, v1 VARCHAR, v2 VARCHAR) "
- + "UPDATE_CACHE_FREQUENCY=" + tableSpecificUpdateCacheFrequency);
- rs = connection.createStatement().executeQuery(readSysCatQuery);
- Assert.assertTrue(rs.next());
- Assert.assertEquals(tableSpecificUpdateCacheFrequency, rs.getLong(2));
- } finally {
- if (connection != null) {
- connection.createStatement().execute("drop table if exists " + tableName);
- connection.close();
+ Properties props = PropertiesUtil.deepCopy(TestUtil.TEST_PROPERTIES);
+ String createTableString = "CREATE TABLE " + tableName + " (k VARCHAR PRIMARY KEY,"
+ + "v1 VARCHAR, v2 VARCHAR)";
+ verifyUCFValueInSysCat(tableName, createTableString, props, 0L);
+ }
+
+ @Test
+ public void testCreateTableWithTableLevelUpdateCacheFreq() throws Exception {
+ String tableName = generateUniqueName();
+ Properties props = PropertiesUtil.deepCopy(TestUtil.TEST_PROPERTIES);
+
+ HashMap<String, Long> expectedUCF = new HashMap<>();
+ expectedUCF.put("10", new Long(10L));
+ expectedUCF.put("0", new Long(0L));
+ expectedUCF.put("10000", new Long(10000L));
+ expectedUCF.put("ALWAYS", new Long(0L));
+ expectedUCF.put("NEVER", new Long(Long.MAX_VALUE));
+
+ for (HashMap.Entry<String, Long> entry : expectedUCF.entrySet()) {
+ String tableLevelUCF = entry.getKey();
+ long expectedUCFInSysCat = entry.getValue();
+
+ String createTableString = "CREATE TABLE " + tableName + " (k VARCHAR PRIMARY KEY,"
+ + "v1 VARCHAR, v2 VARCHAR) UPDATE_CACHE_FREQUENCY = " + tableLevelUCF;
+ verifyUCFValueInSysCat(tableName, createTableString, props, expectedUCFInSysCat);
+ }
+ }
+
+ @Test
+ public void testCreateTableWithInvalidTableUpdateCacheFreqShouldThrow() throws Exception {
+ String tableName = generateUniqueName();
+ Properties props = PropertiesUtil.deepCopy(TestUtil.TEST_PROPERTIES);
+
+ ArrayList<String> invalidUCF = new ArrayList<>();
+ invalidUCF.add("GIBBERISH");
+ invalidUCF.add("10000.6");
+
+ for (String tableLevelUCF : invalidUCF) {
+ String createTableString = "CREATE TABLE " + tableName + " (k VARCHAR PRIMARY KEY,"
+ + "v1 VARCHAR, v2 VARCHAR) UPDATE_CACHE_FREQUENCY = " + tableLevelUCF;
+ try {
+ verifyUCFValueInSysCat(tableName, createTableString, props, -1L);
+ fail();
+ } catch (IllegalArgumentException e) {
+ // expected
+ assertEquals("Table's UPDATE_CACHE_FREQUENCY can only be set to 'ALWAYS', 'NEVER' or a millisecond numeric value.",
Review comment:
The message comparison was probably an overhead in the first place, I only need to verify that an IllegalArgumentException was thrown. Will remove the message comparison.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ntshmah commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ntshmah commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668#discussion_r363020885
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/ConnectionProperty.java
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.schema;
+
+import org.apache.phoenix.query.QueryServicesOptions;
+
+public enum ConnectionProperty {
+ /**
+ * Connection level property phoenix.default.update.cache.frequency
+ */
+ UPDATE_CACHE_FREQUENCY() {
+ @Override
+ public Object getValue(String value) {
+ if (value == null) {
+ return QueryServicesOptions.DEFAULT_UPDATE_CACHE_FREQUENCY;
+ } else {
Review comment:
Done.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668#discussion_r363002717
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java
##########
@@ -621,55 +623,96 @@ public void testMultiTenantImmutableTableMetadata() throws Exception {
}
}
+ private void verifyUCFValueInSysCat(String tableName, String createTableString, Properties props, long expectedUCFInSysCat) throws SQLException {
+ String readSysCatQuery = "SELECT TABLE_NAME, UPDATE_CACHE_FREQUENCY FROM SYSTEM.CATALOG WHERE "
+ + "TABLE_NAME = '" + tableName + "' AND TABLE_TYPE='u'";
+
+ Connection connection = DriverManager.getConnection(getUrl(), props);
Review comment:
nit: Put connection object in a try-with-resources
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668#discussion_r363002947
##########
File path: phoenix-core/src/it/java/org/apache/phoenix/rpc/UpdateCacheIT.java
##########
@@ -244,4 +248,25 @@ private static void validateSelectRowKeyCols(Connection conn, String selectSql,
}
assertFalse(rs.next());
}
+
+ @Test
+ public void testInvalidConnUpdateCacheFrequencyShouldThrow() throws Exception {
+ Properties props = PropertiesUtil.deepCopy(TestUtil.TEST_PROPERTIES);
+
+ ArrayList<String> invalidUCF = new ArrayList<>();
+ invalidUCF.add("GIBBERISH");
+ invalidUCF.add("10000.6");
+
+ for (String connLevelUCF : invalidUCF) {
+ props.put(QueryServices.DEFAULT_UPDATE_CACHE_FREQUENCY_ATRRIB, connLevelUCF);
+ try {
+ DriverManager.getConnection(getUrl(), props);
+ fail();
+ } catch (IllegalArgumentException e) {
+ // expected
+ assertEquals("Connection's phoenix.default.update.cache.frequency can only be set to 'ALWAYS', 'NEVER' or a millisecond numeric value.",
Review comment:
Same with comparing the message string here.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [phoenix] ChinmaySKulkarni commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
Posted by GitBox <gi...@apache.org>.
ChinmaySKulkarni commented on a change in pull request #668: PHOENIX-5654: String values (ALWAYS and NEVER) don't work for connect…
URL: https://github.com/apache/phoenix/pull/668#discussion_r363003264
##########
File path: phoenix-core/src/main/java/org/apache/phoenix/schema/ConnectionProperty.java
##########
@@ -0,0 +1,50 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.phoenix.schema;
+
+import org.apache.phoenix.query.QueryServicesOptions;
+
+public enum ConnectionProperty {
+ /**
+ * Connection level property phoenix.default.update.cache.frequency
+ */
+ UPDATE_CACHE_FREQUENCY() {
+ @Override
+ public Object getValue(String value) {
+ if (value == null) {
+ return QueryServicesOptions.DEFAULT_UPDATE_CACHE_FREQUENCY;
+ } else {
+ if ("ALWAYS".equalsIgnoreCase(value)) {
+ return 0L;
+ } else if ("NEVER".equalsIgnoreCase(value)) {
+ return Long.MAX_VALUE;
+ } else {
+ try {
+ return Long.parseLong(value);
+ } catch (NumberFormatException e) {
+ throw new IllegalArgumentException("Connection's phoenix.default.update.cache.frequency can only be set to 'ALWAYS', 'NEVER' or a millisecond numeric value.");
+ }
Review comment:
nit: Replace the string `phoenix.default.update.cache.frequency` with the static variable name
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services