curiosity-ai / rocksdb-sharp

.net bindings for the rocksdb by facebook
BSD 2-Clause "Simplified" License
163 stars 41 forks source link

Options broken #32

Closed queequac closed 2 years ago

queequac commented 2 years ago

Hi @theolivenbaum,

while my PR on #27 was still working, your "optimization" afterwards broke the option system. You introduced generics, which does compile but does not execute at runtime.

With my code it worked, with yours I get System.ArgumentException: 'The specified Type must not be a generic type. Arg_ParamName_Name' when calling new ColumnFamilyOptions().SetComparator(someComparator);, etc.

Could you revert your commit https://github.com/curiosity-ai/rocksdb-sharp/commit/228f433613c5f3dea48eab825d3839001cab1e45 or fix the code? Not sure why your commit is called "fix chaining for option types", since it was chainable in my test scenarios due to the shared abstract class Options.

theolivenbaum commented 2 years ago

Hi @queequac - the issue is that while it was chainable, it was also a breaking change for methods receiving a ColumnFamilyOptions or a DbOptions Do you have some more code around your usage so I can see what's breaking?

theolivenbaum commented 2 years ago

@queequac can you try https://www.nuget.org/packages/RocksDB/7.5.3.32539 ?

queequac commented 2 years ago

No, it's still broken. You can see my broken sample above. I have just a custom comparator and I am setting it on a columnfamily option. Worked before, does not work with 7.5 anymore due to generics. :(

queequac commented 2 years ago

It fails at Marshal.StructureToPtr(state, statePtr, false); because state is {RocksDbSharp.Options<RocksDbSharp.ColumnFamilyOptions>.ComparatorState} and that does no longer work if ColumnFamilyOptions are actually using a generic.

queequac commented 2 years ago

@theolivenbaum Give it a try. I simply moved the two structs that have been defined inline in the (meanwhile generic) class. This solved the issue on my side.

theolivenbaum commented 2 years ago

Ok moved it, should be published here: https://www.nuget.org/packages/RocksDB/7.5.3.32564

queequac commented 2 years ago

Is working now, thanks!