rowIndex is not recalculated when concatenating tables (javascript implementation)

[loredanacirstea]

After concatenating two tables, their row indexes are not recalculated. This is counter intuitive, so I suspect it is a bug. In the example below I would have expected that "name1" row has index 0 and "name2" row has index 1. Now, they both have rowIndex 0, hence the index given by scan has no utility.

  const AType = new Struct([Field.new({ name: "name", type: new Utf8() })])
  const table1 = Table.from({
    type: AType,
    values: [{name:"name1"}]
  })
  const table2 = Table.from({
    type: AType,
    values: [{name:"name2"}]
  })
    
  const table3 = table1.concat(table2);
  
  // 0 0
  console.log(table3.get(0)[Symbol.for('rowIndex')], table3.get(1)[Symbol.for('rowIndex')])

  table3.filter(predicate.col('name').eq('name2')).scan((idx, batch) => {
    // 0 Row {name: "name1", Symbol(rowIndex): 0, 0: "name1"}
    console.log('idx', idx, table3.get(idx));
  })

[emkornfield]

CC @trxcllnt @TheNeuralBit does this sound like a bug?

[trxcllnt]

No, this is working as designed. The row index is relative to the the record batch, as it's meant to be used to access values from the batch.

If you want a global index, you can sum this index and the lengths of the batches as they're processed.

[trxcllnt]

The internal rowIndex symbol is an optimization that enables the Row flyweight pattern class. It's also relative to the batch, but is not meant to be used directly.

[TheNeuralBit]

Yes the idx is supposed to be relative to the batch, that's why there's a second argument with the current batch. Alternatively you can pass a second callback that will be called when we transition to reading from a new batch.

Agree this is a confusing API, we should make sure it's more clearly documented, right now it has no docstring.

[loredanacirstea]

Thank you for clarifying.