New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make Row Disposable #1824

Closed
TomFinley opened this Issue Dec 5, 2018 · 0 comments

Comments

Projects
1 participant
@TomFinley
Contributor

TomFinley commented Dec 5, 2018

We have previously used what was IRow, and now Row, for two purposes:

  1. It was a "restricted" row cursor that allowed inspection but not movement. Nearly all of our transforms perform their data transformations through this mechanism, which is fine.

  2. It was a convenient way to store what amounted to a property bag that had the benefit of using the IDataView type system and familiar APIs.

The trouble was, that for scenario 2 it would have been incredibly awkward to have IRow be disposable. But for scenario 1 we absolutely required it. So we did awkward things like this:

Row GetRow(Row input, Func<int, bool> active, out Action disposer);

"Here's a Row, and by the way, when you're done, if this thing is non-null, you should treat it as disposable." Kind of silly. But this was able to be done, because these Row objects were "driving" RowCursor implementations, and those were disposable, so we were able to hide this complexity from people. But, since Row must be a public type in our API (just as RowCursor must be), we must change it.

We have now actually resolved scenario 2 by inventing a new type of container, called Metadata, that operates kind of like a Row, but isn't quite one. And this leaves us free now, I think, to make Row disposable.

  • Anything that is still a Row that should become a Metadata should start being a Metadata. (This includes anything that currently yields Rows that are understood by convention to be immovable and where all columns are active... that is, simple property bags.)

  • Much of the disposer logic currently on RowCursor can be moved to Row.

  • Implement dispose pattern on Row.

This issue is an explication of a minor subpoint of #1532 .

@TomFinley TomFinley added the api label Dec 5, 2018

@TomFinley TomFinley self-assigned this Dec 5, 2018

@TomFinley TomFinley added this to To do in Project 13 via automation Dec 5, 2018

Project 13 automation moved this from To do to Done Dec 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment