SebastianGruza commented on code in PR #3184:
URL: https://github.com/apache/hugegraph/pull/3184#discussion_r4023880205
##########
hugegraph-server/hugegraph-hstore/src/main/java/org/apache/hugegraph/backend/store/hstore/HstoreTable.java:
##########
@@ -640,21 +647,20 @@ protected BackendColumnIterator queryByRange(Session
session,
if (query.paging() && !query.page().isEmpty()) {
position = PageState.fromString(query.page()).position();
}
+ byte[] queryBytes = null;
if (origin instanceof ConditionQuery &&
(query.resultType().isEdge() || query.resultType().isVertex())) {
- cq = (ConditionQuery) query.originQuery();
-
- // LOG.debug("query {} with ownerKeyFrom: {}, ownerKeyTo: {}, " +
- // "keyFrom: {}, keyTo: {}, " +
- // "scanType: {}, conditionQuery: {}",
- // this.table(), bytes2String(ownerStart),
- // bytes2String(ownerEnd), bytes2String(start),
- // bytes2String(end), type, cq.bytes());
- return session.scan(this.table(), ownerStart,
- ownerEnd, start, end, type, cq.bytes(),
position);
- }
- return session.scan(this.table(), ownerStart,
- ownerEnd, start, end, type, null, position);
+ // Same guard as queryByPrefix(): only push the query down to the
+ // store when user-prop conditions remain. A sort-key prefix/range
+ // query keeps sysprop conditions only (owner vertex, direction,
+ // label, sort values), which are already enforced by the key
+ // range, and the store-side row decoder cannot parse the raw
+ // property layout written by the server (see issue #3090).
+ ConditionQuery cq = prepareConditionQuery((ConditionQuery) origin);
Review Comment:
Done in 89bb0ec: four lines above the guard now say it applies to the range
path only, because the key range already covers its sysprops, while the
full-scan path (`queryAll()` and the shard overload of `queryByRange`) still
needs the pushdown to filter. `queryAll()` and the shard overload are recorded
in the #3090 follow-up:
https://github.com/apache/hugegraph/issues/3090#issuecomment-5694243559.
Two things from the lab on the reachability itself, master `36811483`, the
same cluster as in the PR description:
- The `g.E().hasLabel('flow')` example does not reach `queryAll()` on
hstore. `HstoreFeatures.supportsQueryByLabel()` returns `false`, so for a
label-only query `GraphTransaction.optimizeQuery` (line 1655) does not hand the
query to the backend; `GraphIndexTransaction.queryByLabel` goes to the label
index table and fetches the edges by id. Measured: a label with the label index
enabled and 3 edges → 3 rows, also with `valueMap(true)`; a label with the
label index disabled → 400 "label index is disabled", raised in core. The same
3 rows through `g.V(x).outE(l).has('asset','ETC')` → `Can't construct
Cardinality from code 0`, so the rows do trip the decoder. `g.E()` without
`has()` sends a plain `Query` (`HugeGraphStep.makeQuery`), so `queryAll()`
pushes `null`.
- Two paths really do push an unguarded `ConditionQuery`: the paged full
scan `g.E().has('~page','').limit(2).valueMap(true)` (paging branch of
`queryAll()`) and REST `/traversers/edges/scan?page=&page_limit=2` (shard
overload). Both returned rows on master, on the same owner's edges on which the
range path fails. I did not trace why the decoder is not tripped there, so I
leave both open in #3090 rather than call them safe.
--
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]
