HivePartition: Add also lock on partition_state

[carlopi]

I am not actually sure if the first lock lock_guard<mutex> global_lock(g.lock); is actually needed or could be potentially removed, but should not be a problem to keep both.

Ideally I think the inner and outer lock should be the same lock, but that would require a deeper review of the code that the one I am comfortable doing right now.

[carlopi]

Note that CI on this is not really useful given it's skipping the only CI job that was failing, the Thread sanitizer run ( https://github.com/duckdb/duckdb/actions/runs/6764754096/job/18383729513), but I checked on my local fork that this change fixes it (https://github.com/carlopi/duckdb/actions/runs/6768785301/job/18394053039) and the change makes sense in any case.

[Mytherin]

Thanks! LGTM. I wonder if the two separate locks make sense here? Could they be unified?

[carlopi]

Thanks! LGTM. I wonder if the two separate locks make sense here? Could they be unified?

In general I don't get what's the role of having two different locks around this code, one as part of CopyToFunctionGlobalState class, and one as part of the inner GlobalHivePartitionState class.

I think in this case the one strictly needed one is the inner one (GlobalHivePartitionState), while the other can be potentially removed, but the the whole code in src/execution/operator/persistent/physical_copy_to_file.cpp uses the outer one, any pointers @samansmink?

[samansmink]

They can be merged by making the one in GlobalHivePartitionState a reference to the one in CopyToFunctionGlobalState, but not sure if it would really improve the code

[carlopi]

Then we good to go I think, thanks!

[Mytherin]

Thanks!