Skip to content

261: /locations should return all parameters available for a station, even when filtering.#298

Open
Jeffrey-Vervoort-KNMI wants to merge 3 commits intomainfrom
261-locations
Open

261: /locations should return all parameters available for a station, even when filtering.#298
Jeffrey-Vervoort-KNMI wants to merge 3 commits intomainfrom
261-locations

Conversation

@Jeffrey-Vervoort-KNMI
Copy link
Copy Markdown
Contributor

@github-actions
Copy link
Copy Markdown

github-actions bot commented Mar 9, 2026

Title Coverage Tests Skipped Failures Errors Time
API Unit Tests Coverage 47 0 💤 0 ❌ 0 🔥 1.602s ⏱️
Ingest Unit Tests Coverage 16 0 💤 0 ❌ 0 🔥 2.484s ⏱️

@@ -36,21 +36,33 @@ func getLocs(

// define and execute query
query := fmt.Sprintf(`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At first glance I feel that the new query is perhaps more complex than necessary. Note: this is not based on an actual understanding of what it does, only the fact that there are now redundant parts etc. Would it be possible to simplify it?
I am going to take a closer look, though. Maybe I find that this is a good solution after all.

Copy link
Copy Markdown
Contributor

@jo-asplin-met-no jo-asplin-met-no Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After spending a bit more time staring at the statement, I now think it makes better sense and is not too complex after all.

Copy link
Copy Markdown
Contributor Author

@Jeffrey-Vervoort-KNMI Jeffrey-Vervoort-KNMI Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it looks more complex. It is a trade-off between simplicity and performance.

The queries performance are the same for small databases, but when having a large number of rows in the tables, it outperforms the simpler query. Which makes it valuable for our production environment.

How it works: It uses CTE to split up the work efficiently, it applies filters early so it has a smaller set when joining.

Copy link
Copy Markdown
Contributor

@jo-asplin-met-no jo-asplin-met-no Mar 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sounds reasonable. But I guess my next question then is why platform is treated in a special way here? In the original issue, the problem was that you only got one parameter?

I mean, intuitively I would expect instead WITH parameter_names AS (SELECT DISTINCT ON (parameter_name) parameter_name ... and so on.

But I might be wrong.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants