Combine multiple PEcAn.db calls in a single query
#3312
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is an example demonstration of how we may proceed with Refactor `convert_input.R` to Optimise Workflow PecanProject#3307 Signed-off-by: Abhinav Pandey <[email protected]>
mdietze
reviewed
Jun 24, 2024
Signed-off-by: Abhinav Pandey <[email protected]>
mdietze
approved these changes
Jun 24, 2024
infotroph
reviewed
Jun 28, 2024
| # Retrieve format name and mimetype from the database | ||
| register$format <- PEcAn.DB::db.query( | ||
| paste("SELECT name, mime_type AS mimetype FROM formats WHERE id = ", register$format$id), con | ||
| ) |
There was a problem hiding this comment.
Two problems here:
- Assigning the result to register$format will overwrite the existing format object (which contains at least
idandinputtype-- see lines 25 and 35) with one that contains onlynameandmimetype. Better to query once into a temporary result and then assign from there intoregister$format$nameand `register$format$mimetype. - The existing line 41 is broken because the formats table doesn't have a
mime_typecolumn, it has amimetype_idtht then needs to be looked up in themimetypestable. I think you want"SELECT name, type_string AS mimetype FROM formats JOIN mimetypes ON formats.mimetype_id = mimetypes.id WHERE formats.id ="
There was a problem hiding this comment.
- I seemed to miss it. Fixing it now.
- Ahh, I recall us having a conversation about this earlier and that seems to be the case.
Change broken `mime_type` to `mimetype_id`. Suggested by @infotroph Signed-off-by: Abhinav Pandey <[email protected]>
Sweetdevil144
commented
Jun 28, 2024
infotroph
reviewed
Jul 9, 2024
infotroph
reviewed
Jul 9, 2024
Co-authored-by: Chris Black <[email protected]>
Co-authored-by: Chris Black <[email protected]>
Co-authored-by: Chris Black <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This is an example demonstration of how we may proceed with : Refactor
convert_input.Rto Optimise Workflow #3307Motivation and Context
Review Time Estimate
Types of changes
Checklist: