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%