gjacoby126 commented on code in PR #1751:
URL: https://github.com/apache/phoenix/pull/1751#discussion_r1417785517
##########
phoenix-core/src/it/java/org/apache/phoenix/end2end/CreateTableIT.java:
##########
@@ -1699,6 +1701,27 @@ public void testCreateTableWithNoVerify() throws
SQLException, IOException, Inte
}
}
+ @Test
+ public void testCreateTableWithTableLevelMaxLookbackAge() throws Exception
{
+ String schemaName = generateUniqueName();
+ String dataTableName = generateUniqueName();
+ String fullTableName = SchemaUtil.getTableName(schemaName,
dataTableName);
+ 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=259200000"; // Set table level max
lookback age to 3 days in milli-seconds
Review Comment:
nit: 259200000 can be a variable or constant since it's used twice in this
method.
##########
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -3153,6 +3158,7 @@ public boolean isViewReferenced() {
} else {
tableUpsert.setNull(36, Types.VARCHAR);
}
+ tableUpsert.setLong(37, maxLookbackAge);
Review Comment:
When creating a table from now on maxlookbackage will default to 0 instead
of NULL? Are we sure that we aren't introducing assumptions that max lookback
age is non null elsewhere, since existing tables will have the value be null?
##########
phoenix-core/src/main/java/org/apache/phoenix/schema/PTable.java:
##########
@@ -997,6 +997,8 @@ IndexMaintainer getIndexMaintainer(PTable dataTable,
PhoenixConnection connectio
* @throws SQLException
*/
Set<ColumnReference> getIndexWhereColumns(PhoenixConnection connection)
throws SQLException;
+
+ long getMaxLookbackAge();
Review Comment:
We're absolutely sure that every path that can possibly create a PTable from
a table without a max lookback age will transparently use the default?
##########
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -5882,6 +5912,14 @@ public void setSchemaVersion(String schemaVersion) {
public void setStreamingTopicName(String streamingTopicName) {
this.streamingTopicName = streamingTopicName;
}
+
+ public Long getMaxLookbackAge() {
Review Comment:
So here max lookback age is a Long, and we have some logic dealing with it
being null at, e.g line 5578...but elsewhere like on PTable we're assuming it's
a long that can't be null.
--
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]
