[ 
https://issues.apache.org/jira/browse/PHOENIX-7022?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17765872#comment-17765872
 ] 

ASF GitHub Bot commented on PHOENIX-7022:
-----------------------------------------

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





> Add new columns TTL and ROW_KEY_PREFIX
> --------------------------------------
>
>                 Key: PHOENIX-7022
>                 URL: https://issues.apache.org/jira/browse/PHOENIX-7022
>             Project: Phoenix
>          Issue Type: Sub-task
>            Reporter: Jacob Isaac
>            Assignee: Lokesh Khurana
>            Priority: Major
>
> When a view statement is defined by the constraints articulated in 
> PHOENIX-4555, all rows created by the view will be prefixed by a KeyRange. 
> The view thus can simply be represented by the prefixed KeyRange generated by 
> the expression representing the view statement. In other words, there exists 
> a one-to-one mapping between the view (defined by tenant, schema, tablename) 
> and PREFIXED KeyRange.
> For lookup on the PREFIXED KeyRange we will create a new column 
> ROW_KEY_PREFIX in SYSTEM.CATALOG. This new column will be populated during 
> view creation when TTL is specified.
>  
> The TTL column (INTEGER) will store the TTL when specified in line with the 
> HBase spec (which uses an int). The PHOENIX_TTL-related columns and code will 
> be deprecated in a separate jira.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to