[GitHub] [phoenix] virajjasani commented on a diff in pull request #1622: PHOENIX-6979 Use HBase TTL as TTL for Tables only at Phoenix Level when phoenix.table.ttl.enabled is true

["virajjasani (via GitHub)" <gi...@apache.org>]

virajjasani commented on code in PR #1622:
URL: https://github.com/apache/phoenix/pull/1622#discussion_r1270215067


##########
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -1938,6 +1946,44 @@ private void populateFamilyPropsList(Map<String, PName> familyNames, Map<String,
         }
     }
 
+    /**
+     * Get TTL defined for Index or View in its parent hierarchy as defining TTL directly on index or view is
+     * not allowed. View on SYSTEM table is not allowed and already handled during plan compilation.
+     * @param parent
+     * @return appropriate TTL for the entity calling the function.
+     * @throws TableNotFoundException
+     */
+    private Long getTTLFromParent(PTable parent) throws TableNotFoundException {
+        return (parent.getType() == TABLE) ? Long.valueOf(parent.getPhoenixTTL()) :
+                (parent.getType() == VIEW ? getTTLFromAncestor(parent) : null);
+    }
+
+    /**
+     * Get TTL defined for the given View according to its hierarchy.
+     * @param view
+     * @return appropriate TTL from Views defined above for the entity calling.
+     * @throws TableNotFoundException
+     */
+    private Long getTTLFromAncestor(PTable view) throws TableNotFoundException {
+        try {
+            return view.getPhoenixTTL() != PHOENIX_TTL_NOT_DEFINED ? Long.valueOf(view.getPhoenixTTL()) :
+                    (checkIfParentIsTable(view) ? connection.getTable(new PTableKey(null ,
+                            view.getPhysicalNames().get(0).getString())).getPhoenixTTL() :
+                            getTTLFromAncestor(connection.getTable(new PTableKey(connection.getTenantId(), view.getParentName().getString()))));
+        } catch (TableNotFoundException tne) {
+            return getTTLFromAncestor(connection.getTable(new PTableKey(null, view.getParentName().getString())));
+        }
+    }
+
+    private boolean checkIfParentIsTable(PTable view) {
+        PName parentName = view.getParentName();
+        if (parentName == null) {
+            //means this is a view on dataTable
+            return true;
+        }
+        return parentName.getString().equals(view.getPhysicalName().getString());

Review Comment:
   are we checking that parent name is same as view name?



##########
phoenix-core/src/main/java/org/apache/phoenix/mapreduce/index/IndexScrutinyMapper.java:
##########
@@ -166,7 +171,8 @@ protected void setup(final Context context) throws IOException, InterruptedExcep
                     PhoenixRuntime.generateColumnInfo(connection, qSourceTable, sourceColNames);
             LOGGER.info("Target table base query: " + targetTableQuery);
             md5 = MessageDigest.getInstance("MD5");
-            ttl = getTableTtl();
+            ttl = getTableTTL(configuration);
+            ttl *= 1000;

Review Comment:
   nit: let's comment why we are multiplying with 1000?



##########
phoenix-core/src/main/java/org/apache/phoenix/schema/MetaDataClient.java:
##########
@@ -1938,6 +1946,44 @@ private void populateFamilyPropsList(Map<String, PName> familyNames, Map<String,
         }
     }
 
+    /**
+     * Get TTL defined for Index or View in its parent hierarchy as defining TTL directly on index or view is
+     * not allowed. View on SYSTEM table is not allowed and already handled during plan compilation.
+     * @param parent
+     * @return appropriate TTL for the entity calling the function.
+     * @throws TableNotFoundException
+     */
+    private Long getTTLFromParent(PTable parent) throws TableNotFoundException {
+        return (parent.getType() == TABLE) ? Long.valueOf(parent.getPhoenixTTL()) :
+                (parent.getType() == VIEW ? getTTLFromAncestor(parent) : null);
+    }
+
+    /**
+     * Get TTL defined for the given View according to its hierarchy.
+     * @param view
+     * @return appropriate TTL from Views defined above for the entity calling.
+     * @throws TableNotFoundException
+     */
+    private Long getTTLFromAncestor(PTable view) throws TableNotFoundException {
+        try {
+            return view.getPhoenixTTL() != PHOENIX_TTL_NOT_DEFINED ? Long.valueOf(view.getPhoenixTTL()) :
+                    (checkIfParentIsTable(view) ? connection.getTable(new PTableKey(null ,
+                            view.getPhysicalNames().get(0).getString())).getPhoenixTTL() :
+                            getTTLFromAncestor(connection.getTable(new PTableKey(connection.getTenantId(), view.getParentName().getString()))));
+        } catch (TableNotFoundException tne) {
+            return getTTLFromAncestor(connection.getTable(new PTableKey(null, view.getParentName().getString())));

Review Comment:
   good to add some log here?



##########
phoenix-core/src/main/java/org/apache/phoenix/util/ViewUtil.java:
##########
@@ -954,7 +954,9 @@ public static void addTagsToPutsForViewAlteredProperties(List<Mutation> tableMet
             PTable parent, ExtendedCellBuilder extendedCellBuilder) {
         byte[] parentUpdateCacheFreqBytes = null;
         byte[] parentUseStatsForParallelizationBytes = null;
-        byte[] parentPhoenixTTLBytes = null;
+        //Commenting out phoenixTTL property to exclude.
+        //TODO:- re-enable after introducing TTL for views.
+        //byte[] parentPhoenixTTLBytes = null;

Review Comment:
   does this mean we need to re-enable all the commented out code in this class?



-- 
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