gjacoby126 commented on code in PR #1751:
URL: https://github.com/apache/phoenix/pull/1751#discussion_r1431860148
##########
phoenix-core-client/src/main/java/org/apache/phoenix/schema/TableProperty.java:
##########
@@ -336,6 +336,29 @@ public Object getValue(Object value) {
@Override public Object getPTableValue(PTable table) {
return table.getStreamingTopicName();
}
+ },
+
+ MAX_LOOKBACK_AGE(PhoenixDatabaseMetaData.MAX_LOOKBACK_AGE, true, false,
false) {
+ @Override
+ public Object getValue(Object value) {
+ if (value == null) {
+ return null;
+ }
+ else if (value instanceof Integer) {
+ return Long.valueOf((Integer) value);
+ }
+ else if (value instanceof Long) {
+ return value;
+ }
+ else {
+ throw new IllegalArgumentException("Table level
MAX_LOOKBACK_AGE should be a numeric value in milli-seconds");
Review Comment:
nit: instead of "numeric" specify BIGINT, since "numeric" might refer to a
floating point value that isn't supported.
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java:
##########
@@ -1795,4 +1796,65 @@ public void
testChangePropertiesUpdatesLASTDDLTimestamp() throws Exception {
newLastDDLTimestamp > oldLastDDLTimestamp);
}
}
-}
\ No newline at end of file
+
Review Comment:
What about if someone sets MAX_LOOKBACK_AGE to NULL?
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java:
##########
@@ -1795,4 +1795,45 @@ public void
testChangePropertiesUpdatesLASTDDLTimestamp() throws Exception {
newLastDDLTimestamp > oldLastDDLTimestamp);
}
}
-}
\ No newline at end of file
+
+ @Test
+ public void testChangeTableLevelMaxLookbackAge() throws Exception {
+ String schemaName = generateUniqueName();
+ String dataTableName = generateUniqueName();
+ String fullTableName = SchemaUtil.getTableName(schemaName,
dataTableName);
+ long baseMaxLookbackAge = 86400000; // 1 day
+ long maxLookbackAge = baseMaxLookbackAge;
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ String ddl = "CREATE TABLE " + fullTableName +
+ " (a_string varchar not null, a_binary VARCHAR not null,
col1 integer" +
+ " CONSTRAINT pk PRIMARY KEY (a_string, a_binary)) " +
"MAX_LOOKBACK_AGE=" + maxLookbackAge;
+ conn.createStatement().execute(ddl);
+ assertMaxLookbackAge(schemaName, dataTableName, maxLookbackAge);
+ maxLookbackAge = 3L * baseMaxLookbackAge;
+ ddl = "ALTER TABLE " + fullTableName + " SET MAX_LOOKBACK_AGE = "
+ maxLookbackAge;
+ conn.createStatement().execute(ddl);
+ assertMaxLookbackAge(schemaName, dataTableName, maxLookbackAge);
+ maxLookbackAge = 2L * baseMaxLookbackAge;
+ ddl = "ALTER TABLE " + fullTableName + " SET MAX_LOOKBACK_AGE = "
+ maxLookbackAge;
+ conn.createStatement().execute(ddl);
+ assertMaxLookbackAge(schemaName, dataTableName, maxLookbackAge);
+ maxLookbackAge = 0;
+ ddl = "ALTER TABLE " + fullTableName + " SET MAX_LOOKBACK_AGE = "
+ maxLookbackAge;
+ conn.createStatement().execute(ddl);
+ assertMaxLookbackAge(schemaName, dataTableName, maxLookbackAge);
+ }
+ }
+
+ private void assertMaxLookbackAge(String schemaName, String dataTableName,
long expectedMaxLookbackAge) throws Exception {
+ String query = "SELECT MAX_LOOKBACK_AGE FROM \"SYSTEM\".\"CATALOG\"\n"
+ + "WHERE TENANT_ID IS NULL AND\n"
+ + "(TABLE_SCHEM, TABLE_NAME) = ('" + schemaName + "','"+
dataTableName + "') AND\n"
+ + "COLUMN_FAMILY IS NULL AND COLUMN_NAME IS NULL";
+ try (Connection conn = DriverManager.getConnection(getUrl())) {
+ ResultSet rs = conn.createStatement().executeQuery(query);
+ assertTrue(rs.next());
+ assertEquals(expectedMaxLookbackAge, rs.getLong(1));
Review Comment:
This would be simpler to do as
PhoenixRuntime.getTableNoCache().getMaxLookbackAge()
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java:
##########
@@ -1727,4 +1767,26 @@ private int checkGuidePostWidth(String tableName) throws
Exception {
}
}
+ private void createTableWithTableLevelMaxLookbackAge(String fullTableName,
String maxLookbackAge) throws Exception {
+ try(Connection conn = DriverManager.getConnection(getUrl())) {
+ String createDdl = "CREATE TABLE " + fullTableName +
+ " (id char(1) NOT NULL," + " col1 integer NOT NULL," + "
col2 bigint NOT NULL," +
+ " CONSTRAINT NAME_PK PRIMARY KEY (id, col1, col2)) " +
+ "MAX_LOOKBACK_AGE="+maxLookbackAge;
+ conn.createStatement().execute(createDdl);
+ }
+ }
+
+ private Long queryTableLevelMaxLookbackAge(String schemaName, String
dataTableName) throws Exception {
+ try(Connection conn = DriverManager.getConnection(getUrl())) {
Review Comment:
Likewise this would also be simpler using PhoenixRuntime.getTableNoCache()
and then looking up from the PTable that's returned.
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/AlterTableIT.java:
##########
@@ -1795,4 +1796,65 @@ public void
testChangePropertiesUpdatesLASTDDLTimestamp() throws Exception {
newLastDDLTimestamp > oldLastDDLTimestamp);
}
}
-}
\ No newline at end of file
+
+ @Test
+ public void testChangeTableLevelMaxLookbackAge() throws Exception {
+ String schemaName = generateUniqueName();
+ String dataTableName = generateUniqueName();
+ String fullTableName = SchemaUtil.getTableName(schemaName,
dataTableName);
+ long baseMaxLookbackAge = 86400000; // 1 day
Review Comment:
this can be a constant because it's used at 1830. Also it should be
86400000L to avoid a cast. Alternately you could use
Duration.ofDays(1).getSeconds() * 1000 if you want to make it more explicit.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
