Myracle commented on code in PR #27728:
URL: https://github.com/apache/flink/pull/27728#discussion_r2909974729
##########
flink-python/pyflink/table/catalog.py:
##########
@@ -1251,6 +1251,21 @@ def get_object_name(self) -> str:
def get_full_name(self) -> str:
return self._j_object_path.getFullName()
+ def compare_to(self, other: 'ObjectPath') -> int:
Review Comment:
Thanks for the sharp catch! You're absolutely right — the isinstance()
short-circuit in the comparison dunder methods (__lt__, __le__, __gt__, __ge__)
silently returns False for non-ObjectPath operands, which violates the expected
total ordering contract (e.g. both a < 42 and a >= 42 would return False).
I've fixed this by returning NotImplemented instead, which is the Pythonic
way to signal unsupported operand types — Python's comparison protocol will
then raise a proper TypeError.
I've also added a comprehensive ObjectPathTest class to test_catalog.py
covering:
- Basic property access (get_database_name, get_object_name, get_full_name,
__str__, from_string)
- Equality and hashing (including use as dict keys and in sets)
- All comparison operators (happy paths with same/different databases and
object names, sorting)
- Edge cases: comparisons against int, str, float, None, list, dict — all
now correctly raise TypeError
--
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]
