amrishlal commented on a change in pull request #8394:
URL: https://github.com/apache/pinot/pull/8394#discussion_r833796285



##########
File path: 
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/recordtransformer/ExpressionTransformer.java
##########
@@ -60,29 +67,41 @@ public ExpressionTransformer(TableConfig tableConfig, 
Schema schema) {
       }
     }
 
-    // Sort the transform functions based on dependencies
-    Set<String> visited = new HashSet<>();
+    // Carry out DFS traversal to topologically sort column names based on 
transform function dependencies. Throw
+    // exception if a cycle is discovered. When a name is first seen it is 
added to discoveredNames set. When a name
+    // is completely processed (i.e the name and all of its children have been 
full explored and no cycles have been
+    // seen), it gets added to the _expressionEvaluators list in topologically 
sorted order. Fully explored names are
+    // removed from discoveredNames set.
+    Set<String> discoveredNames = new HashSet<>();
     for (Map.Entry<String, FunctionEvaluator> entry : 
expressionEvaluators.entrySet()) {
-      topologicalSort(entry.getKey(), expressionEvaluators, visited);
+      String columnName = entry.getKey();
+      if (!_expressionEvaluators.containsKey(columnName)) {
+        topologicalSort(columnName, expressionEvaluators, discoveredNames);
+      }
     }
   }
 
   private void topologicalSort(String column, Map<String, FunctionEvaluator> 
expressionEvaluators,
-      Set<String> visited) {
-    if (visited.contains(column)) {
-      return;
+      Set<String> discoveredNames) {
+    if (discoveredNames.contains(column)) {
+      throw new RuntimeException("Expression cycle found for column '" + 
column + "' in Ingestion Transform Function"

Review comment:
       Fixed.




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



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to