haridsv commented on code in PR #1660:
URL: https://github.com/apache/phoenix/pull/1660#discussion_r1307285500
##########
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -3661,7 +3681,7 @@ private MutationCode processMutationResult(String
schemaName, String tableName,
.setSchemaName(schemaName).setTableName(tableName).setFamilyName(familyName).setColumnName(columnName).setMessage(msg).build().buildException();
case UNALLOWED_SCHEMA_MUTATION:
throw new SQLExceptionInfo.Builder(
- SQLExceptionCode.CANNOT_SET_OR_ALTER_PHOENIX_TTL)
+ SQLExceptionCode.CANNOT_SET_OR_ALTER_TTL)
Review Comment:
Just curious, why is `UNALLOWED_SCHEMA_MUTATION` unconditionally mapped to
`CANNOT_SET_OR_ALTER_TTL`?
##########
phoenix-core/src/main/java/org/apache/phoenix/query/ConnectionQueryServicesImpl.java:
##########
@@ -4186,19 +4185,30 @@ protected PhoenixConnection
upgradeSystemCatalogIfRequired(PhoenixConnection met
}
if (currentServerSideTableTimeStamp <
MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0) {
metaConnection = addColumnsIfNotExists(metaConnection,
- PhoenixDatabaseMetaData.SYSTEM_CATALOG,
MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0 -3,
+ PhoenixDatabaseMetaData.SYSTEM_CATALOG,
MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0 - 5,
PhoenixDatabaseMetaData.PHYSICAL_TABLE_NAME
+ " " + PVarchar.INSTANCE.getSqlTypeName());
metaConnection = addColumnsIfNotExists(metaConnection,
PhoenixDatabaseMetaData.SYSTEM_CATALOG,
- MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0 -2,
+ MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0 - 4,
PhoenixDatabaseMetaData.SCHEMA_VERSION + " " +
PVarchar.INSTANCE.getSqlTypeName());
metaConnection = addColumnsIfNotExists(metaConnection,
PhoenixDatabaseMetaData.SYSTEM_CATALOG,
- MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0 -1,
+ MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0 - 3,
PhoenixDatabaseMetaData.EXTERNAL_SCHEMA_ID + " " +
PVarchar.INSTANCE.getSqlTypeName());
metaConnection = addColumnsIfNotExists(metaConnection,
PhoenixDatabaseMetaData.SYSTEM_CATALOG,
- MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0,
+ MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0 - 2,
PhoenixDatabaseMetaData.STREAMING_TOPIC_NAME + " " +
PVarchar.INSTANCE.getSqlTypeName());
+ /**
+ * TODO: Provide a path to copy existing data from PHOENIX_TTL to
TTL column and then
+ * to DROP PHOENIX_TTL Column. See PHOENIX-7023
Review Comment:
Doesn't this mean that only new tables created or existing tables altered
will recognize this new property? But we are dropping support for `PHOENIX_TTL`
property in several places, so with no manual intervention the existing
PHOENIX_TTL value would stop working between the period till PHOENIX-7023 is
done and rolled out. Is this the expected behavior?
From the description of PHOENIX-7023, I got the sense that PHOENIX_TTL will
be deprecated with this feature, but the support will still be there, but that
is not going to be the reality.
##########
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/util/PhoenixMultiInputUtil.java:
##########
@@ -31,19 +31,19 @@
import java.util.Properties;
+import static
org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.PHOENIX_LEVEL_TTL_NOT_DEFINED;
import static
org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.SYSTEM_CATALOG_NAME;
import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.TABLE_TYPE;
-import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.PHOENIX_TTL;
-import static
org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.PHOENIX_TTL_NOT_DEFINED;
+import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.TTL;
import static org.apache.phoenix.jdbc.PhoenixDatabaseMetaData.VIEW_TYPE;
public class PhoenixMultiInputUtil {
public static final String SELECT_ALL_VIEW_METADATA_FROM_SYSCAT_QUERY =
"SELECT TENANT_ID, TABLE_SCHEM, TABLE_NAME, PHOENIX_TTL FROM " +
Review Comment:
?
```suggestion
"SELECT TENANT_ID, TABLE_SCHEM, TABLE_NAME, TTL FROM " +
```
--
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]
