Skip to content
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

Engine BuildTable ITableResult should make it intuitive to access table data #202

Open
ivberg opened this issue Apr 29, 2022 · 1 comment
Assignees
Labels
enhancement New feature or request untriaged

Comments

@ivberg
Copy link

ivberg commented Apr 29, 2022

Is your feature request an entirely new concept?
Accessing table data is not new, but I didn't find it intuitive and had to ask the SDK experts for a solution.

Is your feature request related to an existing component?
Yes Engine BuildTable ITableResult

Is your feature request related to a problem? Please describe.
Our Unit Tests need to be able to find bugs & increase code coverage by testing not only RuntimeExecutionResults.QueryOutput but the tables used by the UI code. I have found a lot of bugs in the table code and projectors by only using WPA UI - that we should be able to find faster using Unit Tests on tables. WPA UI slows down dev loop and can throw a lot of unrelated exceptions wasting time in the debugger.

However, accessing table data was not intuitive (to me)

Describe the solution you'd like
@mslukebo pointed at an example
In Perfetto UT we also implemented some extensions to ITableResult based on recommendations

More refined versions of these should be baked into the SDK.

Also, side note - in using Kusto for query data, they had an intuitive solution I liked and that is generic. Just putting here as an example for inspiration. Kusto returns something that implements IDataReader. IDataReader exposes similar concepts to what the SDK is exposing in that it exposes columns and arrays of object data in a column with a row iterator. Things like FieldCount, Columns via GetName, GetFieldType, and ToEnumerableObjectArray via LINQ which made it easy to work with the data

Describe alternatives you've considered
See extensions we have already implemented. But this should be easy and part of SDK

Additional context

@ivberg ivberg added enhancement New feature or request untriaged labels Apr 29, 2022
@ivberg
Copy link
Author

ivberg commented Apr 29, 2022

Just putting this "3rd request" here instead of making a new issue since it's tightly coupled with this issue.

Also related to the above issue and in the exts we should be able to access the data by default within the bounds of DataSourceInfo and set the VisibleDomain.

This would help with #201 problem in that by default accessing data (with that calculating relativestamp data incorrectly bug in place) should have returned 1 row instead of 100 (just like WPA UI). Having the table data filtering by default DataSourceInfo (or VisibleDomain) would also allow things like % CPU usage (PercentGenerator) to work without hacky workaround trying to disable that since VisiableDomain is either not set, or it's a PITA to set.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request untriaged
Projects
None yet
Development

No branches or pull requests

2 participants