bitflicker64 commented on code in PR #3184:
URL: https://github.com/apache/hugegraph/pull/3184#discussion_r3928440003
##########
hugegraph-server/hugegraph-hstore/src/test/java/org/apache/hugegraph/backend/store/hstore/HstoreTableTest.java:
##########
@@ -124,6 +134,101 @@ public void
testRangeScanBudgetIncludesOneLookaheadRecord() {
Assert.assertEquals(14L, HstoreTable.rangeScanBudget(query));
}
+ @Test
+ public void testRangeQueryWithoutUserpropsDoesNotPushConditions() {
+ // Sort-key prefix/range queries keep sysprop conditions only (owner
+ // vertex, direction, label, sort values); those are enforced by the
+ // key range already and must not be pushed to the store, whose row
+ // decoder cannot parse the server's raw property layout (issue #3090)
+ ConditionQuery origin = new ConditionQuery(HugeType.EDGE);
+ origin.eq(HugeKeys.OWNER_VERTEX, IdGenerator.of("v1"));
+ origin.eq(HugeKeys.DIRECTION, Directions.OUT);
+ origin.eq(HugeKeys.LABEL, IdGenerator.of(1L));
+ origin.gte(HugeKeys.SORT_VALUES, "ETC!");
+ origin.lt(HugeKeys.SORT_VALUES, "ETC~");
+ int before = origin.conditions().size();
+
+ ScanRecordingSession session = new ScanRecordingSession();
+ this.newTestTable().queryByRange(session, edgeRangeQuery(origin));
+
+ Assert.assertTrue(session.scanCalled);
+ Assert.assertNull(session.lastQueryBytes);
+ Assert.assertEquals(before, origin.conditions().size());
+ }
+
+ @Test
+ public void testRangeQueryWithUserpropsPushesCopyAndKeepsOrigin() {
+ ConditionQuery origin = new ConditionQuery(HugeType.EDGE);
+ origin.eq(HugeKeys.OWNER_VERTEX, IdGenerator.of("v1"));
+ origin.query(Condition.eq(IdGenerator.of(7L), 100));
+ int before = origin.conditions().size();
+
+ ScanRecordingSession session = new ScanRecordingSession();
+ this.newTestTable().queryByRange(session, edgeRangeQuery(origin));
+
+ Assert.assertTrue(session.scanCalled);
+ Assert.assertNotNull(session.lastQueryBytes);
+ // the pushed-down query is a copy: the origin query keeps all its
+ // conditions for core-side filtering after the scan returns
+ Assert.assertEquals(before, origin.conditions().size());
+ // pushed payload: user-prop condition survives, owner-vertex is
+ // dropped, and the back reference to the origin query is cleared
+ ConditionQuery pushed =
ConditionQuery.fromBytes(session.lastQueryBytes);
+ Assert.assertNull(pushed.condition(HugeKeys.OWNER_VERTEX));
+ Assert.assertFalse(pushed.userpropConditions().isEmpty());
+ Assert.assertNull(pushed.originQuery());
+ }
+
+ @Test
+ public void testPrefixListQueryPushesCopyAndKeepsOrigin() {
+ // prepareConditionQueryList() is reached from queryByPrefixList() and
Review Comment:
🧹 Worth stating what sits above these two call sites: nothing calls either
of them in a live server, so `prepareConditionQueryList()` does not run today.
As written, lines 184-186 read as a description of a live path.
`queryByPrefixList` (`HstoreTable.java:526`) has one production caller,
`HstoreTable.query(Session, List<IdPrefixQuery>, String)` (line 331), whose
only caller is `HstoreStore.query(List<HugeType>, List<IdPrefixQuery>)`
(`HstoreStore.java:506`), and that overload is never called:
`HstoreStore.java:492` passes a `QueryWrapper implements
Iterator<IdPrefixQuery>` and so binds to the `Iterator` overload at line 540.
The streaming `query(Session, Iterator<IdPrefixQuery>, String)`
(`HstoreTable.java:348`, called from `HstoreStore.java:557`) is reachable only
through `BackendStore.query(Iterator<Query>, Function, HugeGraph)`
(`BackendStore.java:75-78`), which carries the comment `// TODO: unused now`;
nothing in the repository supplies its `queryWriter` argument.
This withdraws the reachability half of my earlier comments (`3916985809`,
`3921393173`), which called these call sites live and are where the wording
here came from. The guard mismatch itself still stands, but it cannot reach the
store row decoder as things are, which is worth recording in the #3090
follow-up.
Requested change, optional and not worth a repush on its own: reword lines
184-186 to say the method is called from those two sites and that neither has a
live caller today.
--
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]
