Skip to content

Conversation

lippserd
Copy link
Member

Some versions of these RDBMS perform poorly with history queries, particularly when the optimizer changes join order or uses block nested loop joins. Ideally, testing across all RDBMS versions to identify when the optimizer fails and adjusting queries or using optimizer switches would be preferable, but this level of effort is not justified at the moment.

Optimizer is disabled via config:

/etc/icingaweb2/modules/icingadb/config.ini:

[icingadb]
...
disable_optimizer_for_history_queries=1

refs #1187

@lippserd lippserd requested a review from nilmerg June 13, 2025 09:39
@cla-bot cla-bot bot added the cla/signed CLA is signed by all contributors of a PR label Jun 13, 2025
@nilmerg
Copy link
Member

nilmerg commented Jun 13, 2025

I think the config option should be documented.

{
if ($q->getDb()->getAdapter() instanceof Mysql) {
// Locates the first string column, prepends the optimizer hint,
// and resets columns to ensure it appears first in the SELECT statement:
Copy link
Member

Choose a reason for hiding this comment

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

why the first string column? Does the query hint not work if placed before an (aliased) expression?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure it works, but it will also increase complexity. My thought was that the hint should work (or rather not add crap) for every query scenario, even if it can't be added at all as with SELECT 1 (unaliased expression). Considering that all other queries will select real columns, I decided to use this implementation.

Copy link
Member

Choose a reason for hiding this comment

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

Aye. But shouldn't it throw an error just in case the first column is not a string? I didn't try, but doubt that this works:

SELECT 1 + 1 as two, /*+ NO_BNL() */ STRAIGHT_JOIN test.name as test_name from test

Some versions of these RDBMS perform poorly with history queries, particularly
when the optimizer changes join order or uses block nested loop joins. Ideally,
testing across all RDBMS versions to identify when the optimizer fails and
adjusting queries or using optimizer switches would be preferable, but this
level of effort is not justified at the moment.

Optimizer is disabled via config:

`/etc/icingaweb2/modules/icingadb/config.ini`:

```
[icingadb]
...
disable_optimizer_for_history_queries=1
```
@lippserd lippserd force-pushed the history-query-optimizer-hints branch from 26f7b6b to d42be6b Compare June 13, 2025 13:56
@lippserd lippserd requested a review from nilmerg June 13, 2025 13:57
{
if ($q->getDb()->getAdapter() instanceof Mysql) {
// Locates the first string column, prepends the optimizer hint,
// and resets columns to ensure it appears first in the SELECT statement:
Copy link
Member

Choose a reason for hiding this comment

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

Aye. But shouldn't it throw an error just in case the first column is not a string? I didn't try, but doubt that this works:

SELECT 1 + 1 as two, /*+ NO_BNL() */ STRAIGHT_JOIN test.name as test_name from test

*/
protected static function shouldDisableOptimizerForHistoryQueries(): bool
{
return Config::module('icingadb')->get('icingadb', 'disable_optimizer_for_history_queries', false);
Copy link
Member

Choose a reason for hiding this comment

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

I think the settings section fits better. There's a doc area for it as well already.

@nilmerg nilmerg modified the milestones: 1.3.0, 1.2.0 Jun 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla/signed CLA is signed by all contributors of a PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants