rocksdb Add error pointer to CompactRange, WriteBatch, WriteBatchWI C APIs

This PR adds error pointer to CompactRange, WriteBatch, WriteBatchWithIndex C APIs for receiving any error.

Aug 14 '22 08:08 muthukrishnan24

With user defined timestamps, I was using CompactRange but it was not working and figured out it was returning error status. Same with write batch put, returned an error. There is no way to check these statuses via c api.

I considered new function names, but wondering is new functions needed for returning status alone

Aug 18 '22 02:08 muthukrishnan24

@ajkr any consensus on this? Should i add new function names for WriteBatch APIs to get the Status?

Aug 30 '22 03:08 muthukrishnan24

I still lean towards new function names for compatibility. Perhaps they can have a suffix like "_err". Or maybe "2" or "v2" is a better suffix to indicate they're an improved form of the API since other functions commonly report error without any special suffix.

Sep 01 '22 00:09 ajkr

Yeah I'd go with new functions with a "v2" suffix for fixing up APIs like this (adding char** errptr to APIs that can fail). cc @pdillinger, @siying, @ltamasi in case there are differing opinions.

Sep 01 '22 00:09 ajkr

@ajkr Added v2 functions for WriteBatch and WriteBatchWithIndex APIs

Sep 03 '22 16:09 muthukrishnan24

@ajkr can you please review this PR?

Sep 17 '22 02:09 muthukrishnan24

Yeah I'd go with new functions with a "v2" suffix for fixing up APIs like this (adding char** errptr to APIs that can fail). cc @pdillinger, @siying, @ltamasi in case there are differing opinions.

A potential problem is what if we introduce a V2 of the same API in C++, and someone forgets to check the C API? Then we have a real mess. I'd prefer adding something like _check_error or _return_error to the C API name.

Sep 20 '22 16:09 pdillinger