Minor: Remove clone in transform_to_states

[jayzhan211]

Which issue does this PR close?

While working on #12697, I discovered the change. Since this change is not negligible, I’ve separated it to ensure more accurate benchmarking results for #12697.

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Benchmark

--------------------
Benchmark clickbench_1.json
--------------------
┏━━━━━━━━━━━━━━┳━━━━━━━━━━━━┳━━━━━━━━━━━━━┳━━━━━━━━━━━━━━━┓
┃ Query ┃ main ┃ rm-clone-v4 ┃ Change ┃
┡━━━━━━━━━━━━━━╇━━━━━━━━━━━━╇━━━━━━━━━━━━━╇━━━━━━━━━━━━━━━┩
│ QQuery 0 │ 0.53ms │ 0.42ms │ +1.25x faster │
│ QQuery 1 │ 45.57ms │ 44.49ms │ no change │
│ QQuery 2 │ 88.79ms │ 80.65ms │ +1.10x faster │
│ QQuery 3 │ 74.46ms │ 67.41ms │ +1.10x faster │
│ QQuery 4 │ 423.98ms │ 391.22ms │ +1.08x faster │
│ QQuery 5 │ 678.29ms │ 673.73ms │ no change │
│ QQuery 6 │ 40.60ms │ 37.46ms │ +1.08x faster │
│ QQuery 7 │ 45.41ms │ 40.97ms │ +1.11x faster │
│ QQuery 8 │ 614.83ms │ 605.85ms │ no change │
│ QQuery 9 │ 691.24ms │ 665.57ms │ no change │
│ QQuery 10 │ 207.39ms │ 195.89ms │ +1.06x faster │
│ QQuery 11 │ 226.88ms │ 212.60ms │ +1.07x faster │
│ QQuery 12 │ 738.67ms │ 713.79ms │ no change │
│ QQuery 13 │ 933.90ms │ 879.69ms │ +1.06x faster │
│ QQuery 14 │ 852.38ms │ 821.42ms │ no change │
│ QQuery 15 │ 507.49ms │ 503.20ms │ no change │
│ QQuery 16 │ 1722.33ms │ 1310.97ms │ +1.31x faster │
│ QQuery 17 │ 1422.20ms │ 1175.26ms │ +1.21x faster │
│ QQuery 18 │ 4047.53ms │ 3119.16ms │ +1.30x faster │
│ QQuery 19 │ 55.00ms │ 56.74ms │ no change │
│ QQuery 20 │ 1015.81ms │ 922.51ms │ +1.10x faster │
│ QQuery 21 │ 1217.63ms │ 1213.17ms │ no change │
│ QQuery 22 │ 3347.57ms │ 3168.49ms │ +1.06x faster │
│ QQuery 23 │ 8226.45ms │ 7974.39ms │ no change │
│ QQuery 24 │ 488.81ms │ 510.23ms │ no change │
│ QQuery 25 │ 490.35ms │ 493.62ms │ no change │
│ QQuery 26 │ 566.92ms │ 553.18ms │ no change │
│ QQuery 27 │ 1475.90ms │ 1396.85ms │ +1.06x faster │
│ QQuery 28 │ 10447.53ms │ 10713.14ms │ no change │
│ QQuery 29 │ 395.86ms │ 391.29ms │ no change │
│ QQuery 30 │ 757.40ms │ 707.42ms │ +1.07x faster │
│ QQuery 31 │ 684.84ms │ 653.02ms │ no change │
│ QQuery 32 │ 3911.74ms │ 3190.83ms │ +1.23x faster │
│ QQuery 33 │ 4077.03ms │ 3500.06ms │ +1.16x faster │
│ QQuery 34 │ 4718.41ms │ 3702.13ms │ +1.27x faster │
│ QQuery 35 │ 1088.16ms │ 1001.00ms │ +1.09x faster │
│ QQuery 36 │ 149.16ms │ 146.17ms │ no change │
│ QQuery 37 │ 103.00ms │ 102.51ms │ no change │
│ QQuery 38 │ 108.70ms │ 107.61ms │ no change │
│ QQuery 39 │ 324.72ms │ 311.87ms │ no change │
│ QQuery 40 │ 32.81ms │ 32.83ms │ no change │
│ QQuery 41 │ 32.10ms │ 31.78ms │ no change │
│ QQuery 42 │ 40.00ms │ 39.60ms │ no change │
└──────────────┴────────────┴─────────────┴───────────────┘
┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━┳━━━━━━━━━━━━┓
┃ Benchmark Summary ┃ ┃
┡━━━━━━━━━━━━━━━━━━━━━━━━━━━━╇━━━━━━━━━━━━┩
│ Total Time (main) │ 57118.33ms │
│ Total Time (rm-clone-v4) │ 52460.22ms │
│ Average Time (main) │ 1328.33ms │
│ Average Time (rm-clone-v4) │ 1220.01ms │
│ Queries Faster │ 20 │
│ Queries Slower │ 0 │
│ Queries with No Change │ 23 │
└────────────────────────────┴────────────┘

[alamb]

Thanks @jayzhan211 -- makes sense to me

I reminded myself about RecordBatch:
https://docs.rs/arrow-array/53.0.0/src/arrow_array/record_batch.rs.html#72

It does indeed appear that cloneing a record batch requires cloneing a Vec (not just some referenc counts).

Now that I see that I wonder if we should contemplate changing RecordBatch to not use Vec 🤔 (it could e.g. use Arc<[]>)

[jayzhan211]

Now that I see that I wonder if we should contemplate changing RecordBatch to not use Vec 🤔 (it could e.g. use Arc<[]>)

Maybe, we need benchmark to confirm it.

• I think we need to check whether we have a lot of unavoidable clone
• And, verify the Arc<[]> did save the cost

[alamb]

Maybe, we need benchmark to confirm it.

100%