tecbot / gorocksdb

gorocksdb is a Go wrapper for RocksDB
http://rocksdb.org
MIT License
940 stars 269 forks source link

Allow users to implement PartialMergeMulti #172

Closed jamesbibby closed 5 years ago

jamesbibby commented 5 years ago

Problem: The current PartialMerge function accepts a left and right operand and is called many times by this function: https://github.com/DataDog/gorocksdb/blob/master/merge_operator.go#L100

This means implementors may have to deserialize / unmarshal those byte arrays into comparable objects for merging many times.

Proposal: Split the interface.

MergeOperator only requires that you implement: FullMerge(key, existingValue []byte, operands [][]byte) ([]byte, bool)

In addition, you can implement either: PartialMerger - PartialMerge(key, leftOperand, rightOperand []byte) ([]byte, bool) or MultiMerger - PartialMergeMulti(key []byte, operands [][]byte) ([]byte, bool)

In gorocksdb_mergeoperator_partial_merge_multi I do a type switch to see if either PartialMerger or MultiMerger are implemented. This should result in the same functionality for all existing users, but give users the option to implement PartialMergeMulti if it is more efficient for their use case.

Tests: I added tests that result in actual merges taking place. By setting a value, compacting, then doing multiple merge operations, the test ensures that both a PartialMerge and a FullMerge occur.