contrueCT commented on issue #3090: URL: https://github.com/apache/hugegraph/issues/3090#issuecomment-5520968744
@SebastianGruza @imbajin @kjswaruph A coordination note on #2994, #3182, #3184, and the already merged #3140: These changes are not duplicates, and I don't see an immediate textual merge conflict at their current heads. However, they modify related stages of the HStore query pipeline and should be tested together. - #2994 changes the upstream query semantics. Its updates to `ConditionQuery`, `GraphTransaction`, `BinarySerializer`, and `HstoreStore` determine how `LABEL` conditions are resolved, whether sort-key optimization is applicable, and whether an edge query becomes an `IdPrefixQuery`, an `IdRangeQuery`, or falls back to local filtering. - #3184 changes the next boundary: once a range query reaches `HstoreTable.queryByRange()`, it decides whether the remaining `ConditionQuery` should be serialized and pushed to the Store. It also copies the query instead of mutating the original because Core may still need the original conditions for result filtering. - #3182 changes Core-side result filtering and cache eligibility for flattened and post-filtered queries. - #3140, which is already contained in the base used by #3182 and #3184, handles global ordering and page-cursor correctness for range-index scans across HStore partitions. The semantic dependency between #2994 and #3184 seems particularly important. #3184 assumes that Core and the serializers have already transformed the query into the expected range form and that the conditions reaching `HstoreTable` can be classified correctly as user properties or system properties. #2994 changes exactly how those upstream decisions are made. There is also some potentially confusing history here: #2994 contains earlier commits related to HStore range ordering, but its current diff no longer changes `HstoreTable`; the active ordered range-scan implementation is represented by the already merged #3140. The remaining relationship between #2994 and #3184 is therefore primarily semantic rather than duplicate implementation. Before merging either open PR, it would be useful to test a combined branch covering(apparently already covered by CI): 1. #3184's six sort-key prefix/range reproducer cases; 2. #2994's single-label, multi-label, conflicting-label, `IN`, and non-equality label cases, especially those combined with edge sort keys; 3. #3182's flattened `within()` and post-filtering cases with both cache hits and misses; 4. #3140's cross-partition limit, offset, and page-cursor ordering cases. It may be worth cross-linking #2994, #3182, and #3184 in their descriptions and agreeing on a merge/rebase order so that the final combined behavior is validated rather than validating each branch only against the current master. -- 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]
