mistercrunch commented on code in PR #35018:
URL: https://github.com/apache/superset/pull/35018#discussion_r2326354212


##########
superset/daos/base.py:
##########
@@ -251,3 +411,247 @@ def filter_by(cls, **filter_by: Any) -> list[T]:
                 cls.id_column_name, data_model
             ).apply(query, None)
         return query.filter_by(**filter_by).all()
+
+    @classmethod
+    def apply_column_operators(
+        cls, query: Any, column_operators: Optional[List[ColumnOperator]] = 
None
+    ) -> Any:
+        """
+        Apply column operators (list of ColumnOperator) to the query using
+        ColumnOperatorEnum logic. Raises ValueError if a filter references a
+        non-existent column.
+        """
+        if not column_operators:
+            return query
+        for c in column_operators:
+            if not isinstance(c, ColumnOperator):
+                continue
+            col = c.col
+            opr = c.opr
+            value = c.value
+            if not col or not hasattr(cls.model_cls, col):
+                model_name = cls.model_cls.__name__ if cls.model_cls else 
"Unknown"
+                logging.error(
+                    f"Invalid filter: column '{col}' does not exist on 
{model_name}"
+                )
+                raise ValueError(
+                    f"Invalid filter: column '{col}' does not exist on 
{model_name}"
+                )
+            column = getattr(cls.model_cls, col)
+            try:
+                # Always use ColumnOperatorEnum's apply method
+                operator_enum = ColumnOperatorEnum(opr)
+                query = query.filter(operator_enum.apply(column, value))
+            except Exception as e:
+                logging.error(f"Error applying filter on column '{col}': {e}")
+                raise
+        return query
+
+    @classmethod
+    def get_filterable_columns_and_operators(cls) -> Dict[str, List[str]]:
+        """
+        Returns a dict mapping filterable columns (including hybrid/computed 
fields if
+        present) to their supported operators. Used by MCP tools to 
dynamically expose
+        filter options. Custom fields supported by the DAO but not present on 
the model
+        should be documented here.
+        """
+        from sqlalchemy.ext.hybrid import hybrid_property

Review Comment:
   NIT: i think top-of-module imports are generally better, except for some 
exceptions, I keep catching claude doing this as I think it means fewer edits 
for it. [I think] exception are around potential circular deps or importing 
heavy modules



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