use fixed agg name and undo changes on op registeration

Author: chelsea-lin

bigframes/core/compile/sqlglot/aggregations/op_registration.py

@@ -41,9 +41,12 @@ def arg_checker(*args, **kwargs):
                     )
                 return item(*args, **kwargs)
 
-            key = op if isinstance(op, type) else type(op)
-            if key in self._registered_ops:
-                raise ValueError(f"{key} is already registered")
+            if hasattr(op, "name"):
+                key = typing.cast(str, op.name)
+                if key in self._registered_ops:
+                    raise ValueError(f"{key} is already registered")
+            else:
+                raise ValueError(f"The operator must have a 'name' attribute. Got {op}")
             self._registered_ops[key] = item
             return arg_checker
 
@@ -55,7 +58,5 @@ def __getitem__(self, op: str | agg_ops.WindowOp) -> CompilationFunc:
                 raise ValueError(f"The operator must have a 'name' attribute. Got {op}")
             else:
                 key = typing.cast(str, op.name)
-                if key in self._registered_ops:
-                    return self._registered_ops[key]
-                return self._registered_ops[type(op)]
+                return self._registered_ops[key]
         return self._registered_ops[op]

bigframes/core/compile/sqlglot/aggregations/unary_compiler.py

@@ -116,7 +116,7 @@ def _(
 ) -> sge.Expression:
     # TODO: Support interpolation argument
     # TODO: Support percentile_disc
-    result = sge.func("PERCENTILE_CONT", column.expr, sge.convert(op.q))
+    result: sge.Expression = sge.func("PERCENTILE_CONT", column.expr, sge.convert(op.q))
     if window is None:
         # PERCENTILE_CONT is a navigation function, not an aggregate function, so it always needs an OVER clause.
         result = sge.Window(this=result)

bigframes/operations/aggregations.py

@@ -239,12 +239,9 @@ def output_type(self, *input_types: dtypes.ExpressionType) -> dtypes.ExpressionT
 
 @dataclasses.dataclass(frozen=True)
 class ApproxQuartilesOp(UnaryAggregateOp):
+    name: typing.ClassVar[str] = "approx_quantile"
     quartile: int
 
-    @property
-    def name(self):
-        return f"{self.quartile * 25}%"
-
     def output_type(self, *input_types: dtypes.ExpressionType) -> dtypes.ExpressionType:
         if not dtypes.is_orderable(input_types[0]):
             raise TypeError(f"Type {input_types[0]} is not orderable")