Missing dispose of RowGroupMetaData in RowGroupReader?

[Sense545]

• Added missing _metaData?.Dispose() in RowGroupReader
• Implemented IDisposable for RowGroupMetaData (to dispose _handle)

[Sense545]

Sorry about the spam. Haven't gotten the build system set up locally yet, so fixing the issues on the fly.
Also not sure if freeing these is actually necessary, just thought looked a bit weird that only some native handles was freed.

[Sense545]

Built successfully locally now

[adamreeve]

Hi @Sense545, I'm guessing you've closed this as you realised it doesn't really make sense for RowGroupMetaData and SchemaDescriptor to be IDisposable? They provide access to data owned by other classes, eg. RowGroupMetaData wraps a pointer to data owned by the RowGroupReader.

It's fair to say this looks a little weird that these don't free their native handles though, we should probably at least document the reason for this in the code. It's also a potential footgun as accessing RowGroupMetaData after the RowGroupReader has been disposed can cause a segfault. Introducing some extra checks to prevent that might be a good idea.

Edit: This is kind of related to #243

[Sense545]

Yep. Didn't realize they were owned by the parent object at first.

It was a good learning experience to look into this and get it compiling and tests running locally.

Maybe the handles could be renamed to nonOwnedHandle or subHandle or something...

Regarding accessing the RowGroupMetaData after disposing RowGroupReader:
I would expect it to fail, but safely handling the failure and throwing a sane exception in the managed code might be good, but not strictly necessary.