SebastianGruza commented on issue #3090:
URL: https://github.com/apache/hugegraph/issues/3090#issuecomment-5492911185

   @imbajin @kjswaruph — reproducer and two additional findings for item 1 
(operator-sinking property codec), since this blocks every sort-key 
prefix/range query on 1.7.0 with HStore.
   
   ### Minimal reproducer
   
   Fresh graph, 3 property keys, 1 vertex label, 1 edge label with `sort_keys: 
[asset, epoch]`, 2 vertices, 3 edges. Full script (REST + Gremlin, ~100 lines): 
https://gist.github.com/SebastianGruza/616f81e915f00f08c4be3dc447ca77c7
   
   Property `amount` (DOUBLE) is created first on purpose, so it has the lowest 
property-key id and is the first value the store parser reads.
   
   | # | `g.V('a').outE('flow')` + | Server path | 1.7.0 result |
   |---|---|---|---|
   | 1 | no condition | prefix scan | OK, 3 edges |
   | 2 | `has('asset','ETC').has('epoch',100)` — equality on **all** sort keys 
| `IdPrefixQuery` → `HstoreTable.queryByPrefix` | OK, 1 edge |
   | 3 | `has('epoch',100)` — not a sort-key prefix | filtered in server memory 
| OK, 2 edges |
   | 4 | `has('asset','ETC')` — equality on sort-key **prefix only** | 
`IdRangeQuery` → `HstoreTable.queryByRange` | **`Can't construct Cardinality 
from code 0`** |
   | 5 | `has('asset','ETC').has('epoch', gte(150))` | `IdRangeQuery` → 
`queryByRange` | **`Unsupported data type UNKNOWN`** |
   | 6 | `has('asset','ETC').has('epoch', gte(201))` — range matches no row | 
`IdRangeQuery` → `queryByRange` | OK, 0 edges (no row reaches `parseEdge`) |
   
   Same results via `GET /graph/edges?vertex_id=…&properties=…` and via `POST 
/gremlin`.
   
   The two messages (and the `DataType code 43` mentioned above) are just the 
first byte of the first property value read as `(cardinality << 6) | dataType`:
   
   | first row in key range | `amount` | IEEE-754 | byte | `>> 6` | `& 0x3F` | 
error |
   |---|---|---|---|---|---|---|
   | #4 → `(ETC,100)` | 1.5 | `3FF8…` | `0x3F` | 0 (invalid) | — | `Cardinality 
from code 0` |
   | #5 → `(ETC,200)` | 2.5 | `4004…` | `0x40` | 1 (SINGLE) | 0 → `UNKNOWN` | 
`Unsupported data type UNKNOWN` |
   
   ### Finding 1 — only `queryByRange` pushes the query; a one-line interim 
mitigation exists
   
   Core already strips all user-prop conditions before the query reaches the 
backend (`HugeVertexStep.constructEdgesQuery` / 
`GraphTransaction.optimizeQuery` → `resetUserpropConditions()`), and 
`constructShardConditions` produces an `eq` on `SORT_VALUES` only for full 
equality on all sort keys; a prefix-only equality or any range becomes a 
`gte`/`lt` pair → `IdRangeQuery`.
   
   In `hugegraph-hstore/.../HstoreTable.java` (1.7.0):
   
   - `queryByPrefix` (~L497) calls `prepareConditionQuery(originQuery)`, which 
returns `null` when there are no user-prop conditions → nothing is pushed → 
`FilterIterator.of(it, null)` → works.
   - `queryByRange` (~L636–648) sends `cq.bytes()` **unconditionally** whenever 
`originQuery` is a `ConditionQuery`. The remaining conditions are sysprops only 
(owner vertex, direction, label, sort values), all already enforced by 
`keyFrom`/`keyTo`, so the store has nothing to filter and just crashes decoding 
the row. Query 6 above confirms the key range does the pruning: empty range → 
no row parsed → no crash.
   
   Making `queryByRange` use the same `prepareConditionQuery` check as 
`queryByPrefix` unblocks sort-key prefix/range queries on 1.7.x immediately, 
with no on-disk format change and no store-side change — the proper versioned 
codec can land independently. I'm happy to open a PR for that if you consider 
it an acceptable interim mitigation (or I can fold it into whatever kjswaruph 
is preparing).
   
   ### Finding 2 — the property codec is not the only mismatch on the sinking 
path
   
   To see what happens *after* the decoding is fixed, we patched property 
decoding locally in a test store (read cardinality/type from the `PropertyKey` 
instead of the header byte — note this only "works" because the pushed 
conditions never touch property values; with `graph == null` the store's 
`PropertyKey` stub defaults to `TEXT/SINGLE`, so it is not a real fix). 
`FilterIterator` then stopped crashing but **returned empty results**, because 
of two further core-vs-struct mismatches:
   
   1. `Condition$RelationType.EQ` compares a `hugegraph-struct` `Id` (from the 
parsed edge) with a `hugegraph-core` `LongId` (from the deserialized 
`ConditionQuery`) for `LABEL` / `SUB_LABEL` — `Objects.equals` is `false` 
across the two classes, so every row is filtered out.
   2. `Condition$RelationType.compare()` has no `Comparable` path, so string 
sort values (`SORT_VALUES gte/lt`) cannot be compared.
   
   So the acceptance criterion "sort-key prefix/range/paging tests pass" will 
need the `Id` equality and comparison contract between `hugegraph-core` and 
`hugegraph-struct` addressed as well, not only the property codec. The codec 
alone turns a crash into silently empty results, which is worse.
   


-- 
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]

Reply via email to