You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by "palashc (via GitHub)" <gi...@apache.org> on 2026/01/23 23:34:52 UTC
[PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
palashc opened a new pull request, #2357:
URL: https://github.com/apache/phoenix/pull/2357
(no comment)
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "palashc (via GitHub)" <gi...@apache.org>.
palashc commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2770610800
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -3840,6 +3874,15 @@ Collections.<byte[]> emptySet(), HConstants.NO_NONCE, HConstants.NO_NONCE,
for (ImmutableBytesPtr invalidateKey : invalidateList) {
metaDataCache.invalidate(invalidateKey);
}
+ // Also invalidate indexes
Review Comment:
@tkhurana When we alter the table, indexes were not invalidated in the server cache. In the test, when we get the ptable to verify the ttl, I noticed that index ptable is not updated without this change. Let me know if I might be missing something.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2743295483
##########
phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -6413,7 +6417,11 @@ private boolean evaluateStmtProperties(MetaProperties metaProperties,
}
if (metaProperties.getTTL() != table.getTTLExpression()) {
TTLExpression newTTL = metaProperties.getTTL();
- newTTL.validateTTLOnAlter(connection, table);
+ boolean isStrictTTL =
+ metaProperties.isStrictTTL() != null ? metaProperties.isStrictTTL : table.isStrictTTL();
Review Comment:
It would be better if you pass the `isStrictTTL` to the validateTTLOnAlter function and then in the specific implementation of conditional ttl expression skip the checks for uncovered index if the ttl is not strict.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2770537848
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -3840,6 +3874,15 @@ Collections.<byte[]> emptySet(), HConstants.NO_NONCE, HConstants.NO_NONCE,
for (ImmutableBytesPtr invalidateKey : invalidateList) {
metaDataCache.invalidate(invalidateKey);
}
+ // Also invalidate indexes
Review Comment:
Why is this change needed ?
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2743297669
##########
phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -6413,7 +6417,11 @@ private boolean evaluateStmtProperties(MetaProperties metaProperties,
}
if (metaProperties.getTTL() != table.getTTLExpression()) {
TTLExpression newTTL = metaProperties.getTTL();
- newTTL.validateTTLOnAlter(connection, table);
+ boolean isStrictTTL =
+ metaProperties.isStrictTTL() != null ? metaProperties.isStrictTTL : table.isStrictTTL();
+ if (!(newTTL instanceof ConditionalTTLExpression) || isStrictTTL) {
+ newTTL.validateTTLOnAlter(connection, table);
Review Comment:
It would be better if you pass the isStrictTTL to the validateTTLOnAlter function and then in the specific implementation of conditional ttl expression skip the checks for uncovered index if the ttl is not strict.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2743232202
##########
phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -2529,7 +2529,11 @@ private PTable createTableInternal(CreateTableStatement statement, byte[][] spli
} else {
ttlFromHierarchy = checkAndGetTTLFromHierarchy(parent, tableName);
if (!ttlFromHierarchy.equals(TTL_EXPRESSION_NOT_DEFINED)) {
- ttlFromHierarchy.validateTTLOnCreate(connection, statement, parent, tableProps);
+ if (parent.hasConditionalTTL() && !parent.isStrictTTL()) {
+ ttlFromHierarchy = TTL_EXPRESSION_NOT_DEFINED;
Review Comment:
I thought this change was only for uncovered indexes. Isn't this change going to impact views and covered indexes also ? Any child view or covered index will not inherit from its parent having a relaxed conditional ttl .
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "palashc (via GitHub)" <gi...@apache.org>.
palashc commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2743130885
##########
phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -6413,7 +6417,11 @@ private boolean evaluateStmtProperties(MetaProperties metaProperties,
}
if (metaProperties.getTTL() != table.getTTLExpression()) {
TTLExpression newTTL = metaProperties.getTTL();
- newTTL.validateTTLOnAlter(connection, table);
+ boolean isStrictTTL =
+ metaProperties.isStrictTTL() != null ? metaProperties.isStrictTTL : table.isStrictTTL();
Review Comment:
Yeah, if we are not altering strict TTL in the Alter statement then we should fallback to table's strict ttl value, right?
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "palashc (via GitHub)" <gi...@apache.org>.
palashc merged PR #2357:
URL: https://github.com/apache/phoenix/pull/2357
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "palashc (via GitHub)" <gi...@apache.org>.
palashc commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2743725708
##########
phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -2529,7 +2529,11 @@ private PTable createTableInternal(CreateTableStatement statement, byte[][] spli
} else {
ttlFromHierarchy = checkAndGetTTLFromHierarchy(parent, tableName);
if (!ttlFromHierarchy.equals(TTL_EXPRESSION_NOT_DEFINED)) {
- ttlFromHierarchy.validateTTLOnCreate(connection, statement, parent, tableProps);
+ if (parent.hasConditionalTTL() && !parent.isStrictTTL()) {
+ ttlFromHierarchy = TTL_EXPRESSION_NOT_DEFINED;
Review Comment:
Yeah let me fix this
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "palashc (via GitHub)" <gi...@apache.org>.
palashc commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2761368275
##########
phoenix-core/src/test/java/org/apache/phoenix/schema/ConditionalTTLExpressionTest.java:
##########
@@ -602,6 +620,34 @@ public void testSettingCondTTLOnTableWithIndexWithMissingExprCols() throws Excep
} catch (SQLException e) {
assertTrue(e.getCause() instanceof ColumnNotFoundException);
}
+ // relaxed ttl
+ ddl = String.format("alter table %s set TTL = '%s', IS_STRICT_TTL = false", tableName,
+ retainSingleQuotes(ttl));
+ try {
+ conn.createStatement().execute(ddl);
+ fail("Should have thrown ColumnNotFoundException");
+ } catch (SQLException e) {
+ assertTrue(e.getCause() instanceof ColumnNotFoundException);
+ }
+ }
+ }
+
+ @Test
Review Comment:
@tkhurana should we update the uncovered index ttl from the client or should we handle it on the server side when building the ptable for the uncovered index?
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2761509076
##########
phoenix-core/src/test/java/org/apache/phoenix/schema/ConditionalTTLExpressionTest.java:
##########
@@ -602,6 +620,34 @@ public void testSettingCondTTLOnTableWithIndexWithMissingExprCols() throws Excep
} catch (SQLException e) {
assertTrue(e.getCause() instanceof ColumnNotFoundException);
}
+ // relaxed ttl
+ ddl = String.format("alter table %s set TTL = '%s', IS_STRICT_TTL = false", tableName,
+ retainSingleQuotes(ttl));
+ try {
+ conn.createStatement().execute(ddl);
+ fail("Should have thrown ColumnNotFoundException");
+ } catch (SQLException e) {
+ assertTrue(e.getCause() instanceof ColumnNotFoundException);
+ }
+ }
+ }
+
+ @Test
Review Comment:
I think for CDC index we handle it when building the PTable. You can do something similar.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "palashc (via GitHub)" <gi...@apache.org>.
palashc commented on PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#issuecomment-3820741873
> do we already have test where creation of uncovered index fails on strict conditional TTL
@virajjasani Yes we do. https://github.com/apache/phoenix/blob/master/phoenix-core/src/test/java/org/apache/phoenix/schema/ConditionalTTLExpressionTest.java#L538
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2747372543
##########
phoenix-core/src/test/java/org/apache/phoenix/schema/ConditionalTTLExpressionTest.java:
##########
@@ -602,6 +620,34 @@ public void testSettingCondTTLOnTableWithIndexWithMissingExprCols() throws Excep
} catch (SQLException e) {
assertTrue(e.getCause() instanceof ColumnNotFoundException);
}
+ // relaxed ttl
+ ddl = String.format("alter table %s set TTL = '%s', IS_STRICT_TTL = false", tableName,
+ retainSingleQuotes(ttl));
+ try {
+ conn.createStatement().execute(ddl);
+ fail("Should have thrown ColumnNotFoundException");
+ } catch (SQLException e) {
+ assertTrue(e.getCause() instanceof ColumnNotFoundException);
+ }
+ }
+ }
+
+ @Test
Review Comment:
Can you also add a test case where the table has uncovered index and a literal TTL. Now you alter the ttl to conditional with IS_STRICT_TTL set to false. Then verify that the table has conditional ttl and the uncovered index has ttl undefined. I am suspecting that while you skipped checking for uncovered indexes during the ttl validation on alter, I am not seeing that you are actually updating the ttl on uncovered index to undefined.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2743238861
##########
phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -2529,7 +2529,11 @@ private PTable createTableInternal(CreateTableStatement statement, byte[][] spli
} else {
ttlFromHierarchy = checkAndGetTTLFromHierarchy(parent, tableName);
if (!ttlFromHierarchy.equals(TTL_EXPRESSION_NOT_DEFINED)) {
- ttlFromHierarchy.validateTTLOnCreate(connection, statement, parent, tableProps);
+ if (parent.hasConditionalTTL() && !parent.isStrictTTL()) {
+ ttlFromHierarchy = TTL_EXPRESSION_NOT_DEFINED;
Review Comment:
With this change even for covered index any relaxed conditional ttl will become undefined. I don't think that is intended right ?
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "virajjasani (via GitHub)" <gi...@apache.org>.
virajjasani commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2739871898
##########
phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -6413,7 +6417,11 @@ private boolean evaluateStmtProperties(MetaProperties metaProperties,
}
if (metaProperties.getTTL() != table.getTTLExpression()) {
TTLExpression newTTL = metaProperties.getTTL();
- newTTL.validateTTLOnAlter(connection, table);
+ boolean isStrictTTL =
+ metaProperties.isStrictTTL() != null ? metaProperties.isStrictTTL : table.isStrictTTL();
Review Comment:
Question: do we need to fallback to `table.isStrictTTL()`?
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2770571065
##########
phoenix-core-server/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
Review Comment:
@palashc can we combine this entire logic in one method `getTTLForTable`. It will be make the code more readable.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "palashc (via GitHub)" <gi...@apache.org>.
palashc commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2766562943
##########
phoenix-core/src/test/java/org/apache/phoenix/schema/ConditionalTTLExpressionTest.java:
##########
@@ -602,6 +620,34 @@ public void testSettingCondTTLOnTableWithIndexWithMissingExprCols() throws Excep
} catch (SQLException e) {
assertTrue(e.getCause() instanceof ColumnNotFoundException);
}
+ // relaxed ttl
+ ddl = String.format("alter table %s set TTL = '%s', IS_STRICT_TTL = false", tableName,
+ retainSingleQuotes(ttl));
+ try {
+ conn.createStatement().execute(ddl);
+ fail("Should have thrown ColumnNotFoundException");
+ } catch (SQLException e) {
+ assertTrue(e.getCause() instanceof ColumnNotFoundException);
+ }
+ }
+ }
+
+ @Test
Review Comment:
@tkhurana I made this change, let me know what you think
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] PHOENIX-7753 : Allow uncovered index creation on tables with relaxed conditional TTL [phoenix]
Posted by "tkhurana (via GitHub)" <gi...@apache.org>.
tkhurana commented on code in PR #2357:
URL: https://github.com/apache/phoenix/pull/2357#discussion_r2743295483
##########
phoenix-core-client/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -6413,7 +6417,11 @@ private boolean evaluateStmtProperties(MetaProperties metaProperties,
}
if (metaProperties.getTTL() != table.getTTLExpression()) {
TTLExpression newTTL = metaProperties.getTTL();
- newTTL.validateTTLOnAlter(connection, table);
+ boolean isStrictTTL =
+ metaProperties.isStrictTTL() != null ? metaProperties.isStrictTTL : table.isStrictTTL();
Review Comment:
It would be better if you pass the `isStrictTTL` to the validateTTLOnAlter function and then in the specific implementation of conditional ttl expression skip the checks for uncovered index if the ttl is not strict.
--
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: issues-unsubscribe@phoenix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org