get_SDA_property: CTEs for SQLite compatibility and query readability #381
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.
This PR replaces
SELECT ... INTO ...
SQL syntax with Common Table Expressions (CTEs) inget_SDA_property()
. Temporary tables are used in the dominant component (numeric) and weighted average aggregation methods.This change has the benefit of increasing the compatibility of
get_SDA_property()
methods with SQLite and other dialects. I've done some benchmarking of old vs. new as well as against SSURGOPortal output. We have 1:1 parity across tools and dialects, and all existing soilDB tests are passing with the new code.In this PR I've also built out the infrastructure that will make it relatively easy implement this approach for the other areas I identified in #250.
Before merging I will be testing this across the country to verify there aren't edge cases when querying multiple properties with very different patterns of missing values, and adding additional tests if needed to make sure that type of edge-case functionality is fully tested.