-
Notifications
You must be signed in to change notification settings - Fork 154
feat: add ISO-GQL PROPERTY_EXISTS predicate (#359) #702
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: add ISO-GQL PROPERTY_EXISTS predicate (#359) #702
Conversation
Implement PROPERTY_EXISTS function according to ISO-GQL Section 19.13. Features: - ISO-GQL compliant PROPERTY_EXISTS predicate - Three-valued logic support (True/False/NULL) - Support for vertices, edges, and rows - Comprehensive unit tests and SQL test cases Implementation: - Add PropertyExists UDF class - Register function in BuildInSqlFunctionTable - Add PropertyExistsTest with 5 test cases (100% pass) - Add 3 SQL integration test cases Testing: - Unit tests: 5/5 passed - Checkstyle: 0 violations - Build: SUCCESS Closes apache#359
|
Hi @SeasonPilot. About this PR. I saw you just implement the simpify PROPERTY_EXISTS predicate. Will you add more change for this issue? |
…pache#359) Refactor PROPERTY_EXISTS to follow GeaFlow's established ISO-GQL predicate pattern by introducing PropertyExistsFunctions utility class. This aligns PropertyExists with IsSourceOf/IsDestinationOf implementation and addresses technical debt from the initial implementation. **Architecture Improvements:** - Add PropertyExistsFunctions utility class (three-layer pattern) * UDF → Utility → Business Logic - Delegate all eval() methods to utility class - Implement type validation with IllegalArgumentException - Add property name validation (null/empty checks) - Maintain ISO-GQL three-valued logic (NULL → null) **Error Handling:** - Invalid element type → clear error messages with type info - NULL/empty property name → descriptive error messages - Consistent with SourceDestinationFunctions error handling **Testing:** - Expand from 4 to 13 tests (+225% coverage) - Add 9 error handling tests: * Invalid element types (String, Integer) * NULL/empty/whitespace property names * Error message validation * Type-specific overload testing - All 13/13 tests pass **Code Quality:** - Checkstyle: 0 violations - Comprehensive Javadoc with ISO-GQL Section 19.13 reference - Design decision documentation - Implementation notes for Row interface limitations **Implementation Strategy:** - Runtime validation follows compile-time checking approach - Row interface indexed access documented - Future runtime schema validation options identified **Files:** - NEW: PropertyExistsFunctions.java (137 lines) - MOD: PropertyExists.java (refactored to delegation) - MOD: PropertyExistsTest.java (comprehensive tests) **Build Status:** - Tests: 13 run, 0 failures, 0 errors - Checkstyle: PASS - Build: SUCCESS
Hi @Manakaa ! Thanks for your attention to this PR. I've just pushed the refined/improved code for this issue, which includes the complete implementation of the PROPERTY_EXISTS predicate (beyond the initial simplified version). Feel free to review the latest commits when you have time. |
Leomrlin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
yaozhongq
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
What changes were proposed in this pull request?
Implement ISO-GQL PROPERTY_EXISTS predicate function according to Section 19.13 specification.
Core Changes:
org.apache.geaflow.dsl.udf.table.otherFeatures:
Test Coverage:
How was this PR tested?
Closes #359"