algo.steinerTree attaches edge weights by iteration position, so a relTypes filter and the analytical view each silently change the tree it returns

[lvca]

Found while working on #6289 (PR #6298) and verified with runnable reproductions before filing. Not a performance question like #6289 was - algo.steinerTree returns wrong trees and wrong weights, and it returns different ones depending on whether a Graph Analytical View happens to be present.

Root cause

AlgoSteinerTree.buildWeightedAdj pairs weights with neighbours by position:

final int[][] adj = graph.adjacency(Vertex.DIRECTION.BOTH, relTypes);   // built WITH relTypes
final double[][] adjW = buildWeightedAdj(graph, adj, weightProperty);
...
private double[][] buildWeightedAdj(...) {
  for (int i = 0; i < n; i++) {
    final Iterable<Edge> edges = graph.getVertex(i).getEdges(Vertex.DIRECTION.BOTH);  // NO relTypes
    final double[] tmpW = new double[adj[i].length];
    int pos = 0;
    for (final Edge e : edges) {
      ...
      if (pos < adj[i].length) {
        tmpW[pos] = ...;   // the pos-th EDGE's weight lands on the pos-th FILTERED NEIGHBOUR
        pos++;
      }
    }
    adjW[i] = tmpW;
  }
}

adj[i] is the neighbour list after the relTypes filter and in whatever order the backing store produced. tmpW is filled from an unfiltered getEdges(BOTH) walk in OLTP order. Nothing reconciles the two - nbIdx is computed and then only used as a skip test, never as the index to write at. So adjW[i][j] is the weight of "the j-th edge I happened to iterate", not "the weight of the edge to adj[i][j]".

That array is not just what gets reported - it is what Dijkstra runs on in step 1, so the tree itself changes, not only the weight column.

Reproduction 1: a second edge type corrupts the weights

Three vertices on a ROAD path of weight 1 per hop, plus one NOISE edge of weight 999 hanging off X. relTypes='ROAD' should exclude NOISE entirely.

MATCH (a:N {name:'X'}), (z:N {name:'Z'})
CALL algo.steinerTree([a,z],'ROAD','w') YIELD source, target, weight, totalWeight

{s: X, t: Y, weight: 999.0, totalWeight: 1000.0}      <-- expected 1.0 and 2.0
{s: Y, t: Z, weight: 1.0,   totalWeight: 1000.0}

The excluded edge's weight is attached to an included edge. totalWeight is 500x the true cost of the tree.

Reproduction 2: the analytical view changes the answer

Single edge type, no filter subtlety at all - X has two ROAD neighbours (Y at 1.0, Z at 50.0) and Y-Z costs 1.0. The cheapest tree connecting X and Z is X-Y-Z at 2.0.

OLTP {s: X, t: Y, weight: 1.0,  totalWeight: 2.0}     <-- correct
OLTP {s: Y, t: Z, weight: 1.0,  totalWeight: 2.0}

CSR  {s: X, t: Y, weight: 1.0,  totalWeight: 51.0}    <-- same query, view present
CSR  {s: Z, t: Y, weight: 50.0, totalWeight: 51.0}

The CSR adjacency order need not match the OLTP getEdges order, so the positional pairing lands differently and the procedure picks a 51.0 tree over the 2.0 one. A Graph Analytical View is meant to be a transparent accelerator; here its mere presence changes a query's result, which is the worst failure mode a view can have and is not something a user has any way to anticipate.

Suggested fix, and a better one

The mechanical fix is to build the weight array by neighbour index rather than by position - getEdges(BOTH, relTypes) with the filter applied, resolve nbIdx = graph.indexOf(nbRid), find that neighbour's slot in adj[i] and write there. That closes both reproductions.

The better fix is to stop deriving weights from a second, independent traversal at all. GraphData already exposes edgeWeights(dir, weightProperty, relTypes), which returns weights aligned with adjacency(...) by construction and reads them from columnar storage on the CSR path instead of deserialising edge records. algo.apsp already uses exactly that. Switching algo.steinerTree to it removes the alignment question rather than answering it, is faster on the view path, and deletes buildWeightedAdj outright. edgeWeights returns null when the view has no edge properties, so the positional walk survives only as the OLTP fallback - and that fallback should be index-based too.

Worth checking in the same pass: whether any other procedure derives weights from an independent getEdges walk instead of edgeWeights(...), since the bug is a pattern rather than a line.

Secondary, same method

buildWeightedAdj calls graph.getVertex(i).getEdges(...) without a null check. GraphData.getVertex returns null by design for a vertex deleted since the CSR was built (it catches RecordNotFoundException and returns null), so a concurrent delete turns this into an NPE rather than a skipped vertex. Every other caller in the package null-checks it.

Related

#6289, PR #6298, #6263, #6300.