jpisaac commented on code in PR #1660:
URL: https://github.com/apache/phoenix/pull/1660#discussion_r1327839697
##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataEndpointImpl.java:
##########
@@ -1364,11 +1374,11 @@ private PTable getTableFromCells(List<Cell>
tableCellList, List<List<Cell>> allC
useStatsForParallelization : oldTable != null ?
oldTable.useStatsForParallelization() : null);
Cell phoenixTTLKv = tableKeyValues[PHOENIX_TTL_INDEX];
- long phoenixTTL = phoenixTTLKv == null ? PHOENIX_TTL_NOT_DEFINED :
-
PLong.INSTANCE.getCodec().decodeLong(phoenixTTLKv.getValueArray(),
+ long phoenixTTL = phoenixTTLKv == null ? PHOENIX_OLD_TTL_NOT_DEFINED :
PLong.INSTANCE
+ .getCodec().decodeLong(phoenixTTLKv.getValueArray(),
phoenixTTLKv.getValueOffset(), SortOrder.getDefault());
- builder.setPhoenixTTL(phoenixTTLKv != null ? phoenixTTL :
- oldTable != null ? oldTable.getPhoenixTTL() :
PHOENIX_TTL_NOT_DEFINED);
+ builder.setPhoenixTTL(phoenixTTLKv != null ? phoenixTTL : oldTable !=
null
Review Comment:
I think we should mark the setPhoenixTTL, setPhoenixTTLHighWaterMark, and
setViewModifiedPhoenixTTL for PTable.Builder also deprecated. WDTY
##########
phoenix-core/src/main/java/org/apache/phoenix/coprocessor/MetaDataProtocol.java:
##########
@@ -99,7 +99,7 @@ public abstract class MetaDataProtocol extends
MetaDataService {
public static final long MIN_SYSTEM_TABLE_TIMESTAMP_4_15_0 =
MIN_TABLE_TIMESTAMP + 29;
public static final long MIN_SYSTEM_TABLE_TIMESTAMP_4_16_0 =
MIN_TABLE_TIMESTAMP + 33;
public static final long MIN_SYSTEM_TABLE_TIMESTAMP_5_1_0 =
MIN_SYSTEM_TABLE_TIMESTAMP_4_16_0;
- public static final long MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0 =
MIN_TABLE_TIMESTAMP + 37;
+ public static final long MIN_SYSTEM_TABLE_TIMESTAMP_5_2_0 =
MIN_TABLE_TIMESTAMP + 39;
Review Comment:
Why the increase of 2 (from 37 to 39)?
##########
phoenix-core/src/main/java/org/apache/phoenix/util/ScanUtil.java:
##########
@@ -995,26 +995,26 @@ public static boolean
isServerSideMaskingEnabled(PhoenixConnection phoenixConnec
((isServerSideMaskingSet != null) &&
(Boolean.parseBoolean(isServerSideMaskingSet))));
}
- public static long getPhoenixTTL(Scan scan) {
- byte[] phoenixTTL =
scan.getAttribute(BaseScannerRegionObserver.PHOENIX_TTL);
+ public static int getPhoenixTTL(Scan scan) {
Review Comment:
Should this be getTTL(..)?
##########
phoenix-core/src/main/java/org/apache/phoenix/schema/DelegateTable.java:
##########
@@ -364,6 +364,10 @@ public Boolean useStatsForParallelization() {
return delegate.hasViewModifiedUseStatsForParallelization();
}
+ @Override public int getTTL() {
+ return delegate.getTTL();
+ }
+
@Override public long getPhoenixTTL() { return delegate.getPhoenixTTL(); }
Review Comment:
Mark as deprecated too?
##########
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -5480,21 +5522,21 @@ private boolean evaluateStmtProperties(MetaProperties
metaProperties, MetaProper
}
}
- if (metaProperties.getPhoenixTTL() != null) {
+ if (metaProperties.getTTL() != null) {
if (table.getType() == PTableType.INDEX) {
throw new SQLExceptionInfo.Builder(
SQLExceptionCode.CANNOT_SET_OR_ALTER_PROPERTY_FOR_INDEX)
.build()
.buildException();
}
- if (table.getType() != PTableType.TABLE && table.getType() !=
SYSTEM) {
+ if (table.getType() != PTableType.TABLE) {
Review Comment:
TTL is not supported for SYSTEM tables as well.
##########
phoenix-core/src/main/java/org/apache/phoenix/jdbc/PhoenixDatabaseMetaData.java:
##########
@@ -392,10 +392,15 @@ public class PhoenixDatabaseMetaData implements
DatabaseMetaData {
public static final byte[] USE_STATS_FOR_PARALLELIZATION_BYTES =
Bytes.toBytes(USE_STATS_FOR_PARALLELIZATION);
// The TTL property will hold the duration after which rows will be marked
as expired and
- // is stored in column PHOENIX_TTL in SYSCAT. TODO:- Rename PHOENIX_TTL to
TTL in SYSCAT!?
+ // is stored in column TTL in SYSCAT
public static final String TTL = "TTL";
- public static final long PHOENIX_TTL_NOT_DEFINED = 0L;
- public static final long DEFAULT_PHOENIX_TTL = HConstants.FOREVER;
+ public static final byte[] TTL_BYTES = Bytes.toBytes(TTL);
+ public static final int TTL_NOT_DEFINED = 0;
+ @Deprecated
+ public static final long PHOENIX_OLD_TTL_NOT_DEFINED = 0L;
Review Comment:
It will be clearer if we used PHOENIX_TTL_NOT_DEFINED_DEPRECATED instead,
WDYT
--
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]
