-
Notifications
You must be signed in to change notification settings - Fork 394
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
Modify End Use by Subcategory tables in SQL so they can be queried #7584
Conversation
…egories (in BEPS and Demand End Use) before outputing to SQL/ResultsSchema
…by end use subcat
|
||
// Add informative message if failed | ||
EXPECT_EQ(endUseName, result[0][0]) << "Failed for reportName=" << reportName; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check that the "RowName" isn't empty anymore
" AND ReportName = 'AnnualBuildingUtilityPerformanceSummary'" | ||
" AND ColumnName = 'Subcategory') = 0", | ||
"TabularDataWithStrings"); | ||
ASSERT_EQ(7u, result.size()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This query works in one step to query all entries for a specific end use subcategory.
" WHERE TableName = 'End Uses By Subcategory'" | ||
" AND ReportName = 'AnnualBuildingUtilityPerformanceSummary'" | ||
" AND RowName = '" + endUseName + "'" | ||
" AND ColumnName = 'Electricity'" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the addition here, query for ONE fuel. And Return only "Value" not "*"
@jmarrec Thank you! I'll be able to get rid of my super complicated, hacky code... |
I have not tested it, only looked at @jmarrec's screenshots. That said, now that I get fresh eyes on this... the PR is an incremental improvement that makes it slightly easier to query, but the query is still pretty ugly. It's fine to have this drop in, but I don't feel that the issue has been substantially addressed. The query to obtain this kind of information should not have to be so complicated. Compare this PR's query:
with the two queries I was envisioning as possibilities:
or
|
Extracting the relevant comment from my original PR message:
The |
@jmarrec Would it be useful to duplicate the end-use information in a second well-designed table? Not necessarily as part of this PR. |
Personally, I don’t see why adding a new End Use SubCat column that is often null is so bad. It seems like the most generic and least obtrusive change. And it would be non-breaking. Adding a separate table would be fine, but seems silly to query end use data one place and end use by subcategory data somewhere else. My two cents. |
Not sure how others would feel, but I would be happy with this solution, yes. |
@shorowit see updated solution. check the markdown, I included example queries and table outputs too |
I built this locally and confirmed that the results look correct and the data is now easy to query. @jmarrec, thanks so much for making this change. |
Pull request overview
This Pull Request will properly fill the rowHead (forward fill the values of End Uses basically) before ouputing to SQL but after the HTML file is already written so we don't mess with this.
Before:
If you look at the HTML result, you see that rowHead is empty for display purposes:
This means the SQL one looks very similar, with empty "RowName" cells:
After: All RowNames are filled
It's still not terribly straightforward to get a specific entry for a subcategory end use, but it's doable for sure (and quite fast). I personally like this better than combining both End Use and End Use Subcat in the same RowName, or worse adding a new column (which would create way too many "NULLs").
NOTE: ENHANCEMENTS MUST FOLLOW A SUBMISSION PROCESS INCLUDING A FEATURE PROPOSAL AND DESIGN DOCUMENT PRIOR TO SUBMITTING CODE
Pull Request Author
Add to this list or remove from it as applicable. This is a simple templated set of guidelines.
Reviewer
This will not be exhaustively relevant to every PR.