RFC: Compatible evolution of T** params to unique_ptr<T>*

[pdillinger]

Summary: Adds a compatibility layer in public APIs such that out params
transferring ownership such as DB** dbptr can now accept either DB**
or unique_ptr<DB>*.

Test Plan: small test case added, CI etc.

[ltamasi]

I'm all for making the ownership semantics clear in the public API. However, I'm a bit concerned about introducing a custom smart pointer-like type, since there are a lot of ways it could be accidentally misused by application code. (I realize UniquePtrOut is only meant to be implicitly constructed when Open is called with a pointer to either a raw or a smart pointer but there's no way we can guarantee that.)

An alternative solution to consider: introduce new overloads that take unique_ptrs and turn the current methods into thin wrappers around the unique_ptr versions. (We could allocate a unique_ptr on the stack in the wrapper, call the smart pointer version, and then call release on the unique_ptr and copy the raw pointer value to the out parameter.) In addition, we could mark the raw pointer versions deprecated and remove them in 7.0.

[pdillinger]

I was hoping to avoid "overload bloat." Perhaps it is the lesser of evils.

[siying]

I agree with Levi on providing an overload that follows good practice for now. Deleting old one might be hard but at least people who want to follow the best practice can start to use the new one.

Another question is that, if we are changing DB::Open(), do we want to split DB options and Open options? There are options create_if_missing, create_missing_column_families, error_if_exists, etc that are weird to be thinking about as an DB option and it doesn't make sense to persistent them to OPTIONS files. There might be more of these in the future. If we are going to change to a better API, we can think about whether there are other ways we should improve this function.

[mrambacher]

I agree with Levi on providing an overload that follows good practice for now. Deleting old one might be hard but at least people who want to follow the best practice can start to use the new one.

Another question is that, if we are changing DB::Open(), do we want to split DB options and Open options? There are options create_if_missing, create_missing_column_families, error_if_exists, etc that are weird to be thinking about as an DB option and it doesn't make sense to persistent them to OPTIONS files. There might be more of these in the future. If we are going to change to a better API, we can think about whether there are other ways we should improve this function.

I suggested something similar (OpenOptions) in #8660. Previously I also suggested allowing "DBPlugin" options in #6661 (which might be OpenOptions or DBOptions) that could help unify the Open API for the various Stackable DBs.