Fix failed test #91
Fix failed test #91
Conversation
It is necessary for avoiding output Query Identifier while vanille's test are running. (Look at more in explain.c:612. We can get fail condition if only query identifier is not null) Where clause 'NOT LIKE '%Query Identifier%'' is throwed away due to being necessary any more. This addition parameter is appeared if we set compute_query_id parameter with value as 'auto'. Appearance of the parameter is checked in only gucs test.
Delete platform dependent lines containing Memory and add order by command in feature_subspace test for statical result.
Alena0704
force-pushed
the
fix_failed_test
branch
from
7466361 to
e474ecb
Compare
|
|
||
| -- Check AQO addons to explain (the only stable data) | ||
| SELECT str FROM expln(' | ||
| SELECT regexp_replace( |
There was a problem hiding this comment.
А зачем здесь тебе таки показывать Query Identifier ?
There was a problem hiding this comment.
Видимо, проверяется, что он вообще выводится..
There was a problem hiding this comment.
Да, делала проверку на вывод Query Identifier. Показалось логичным оставить это только в gucs тесте.
Специально добавила установку параметра compute_query_id в 'auto'.
В других тестах не добавляла и убрала Query Identifier из-за его ненужности там.
There was a problem hiding this comment.
Видимо, проверяется, что он вообще выводится..
Ну, это достаточно слабое объяснение. Query Identifier вообще ванильная штука, чего его проверять? Я бы скорее сказал, что проверяются другие параметры экплейна, AQO-специфичные. А для них Identifier не нужен
There was a problem hiding this comment.
Тогда здесь можно убрать, как это сделано в остальных тестах?
| max_parallel_maintenance_workers = 1 # switch off parallel workers because of unsteadiness | ||
| aqo.wide_search = 'on' | ||
| aqo.wide_search = 'on' | ||
| compute_query_id = 'regress' |
There was a problem hiding this comment.
А это никак не повлияет на другие тесты, выполняемые installcheck ? И почему?
There was a problem hiding this comment.
У нас сейчас есть schedule-файл с порядком выполнения тестов. Почему бы не устанавливать это значение в первом тесте и не сбрасывать в последнем?
There was a problem hiding this comment.
А это никак не повлияет на другие тесты, выполняемые installcheck ? И почему?
compute_query_id = 'regress' => не выводить query id, даже если он посчитался. Вроде тогда мы и другие тесты не ломаем, и функциональность по факту тестируется?.. (если что-то упадет при подсчете, то это будет заметно?)
There was a problem hiding this comment.
У нас сейчас есть schedule-файл с порядком выполнения тестов. Почему бы не устанавливать это значение в первом тесте и не сбрасывать в последнем?
Во-первых, compute_query_id = 'regress' в любом случае нужен, чтобы не сломался make installcheck в дереве исходников postgres с загруженным aqo. Как писала Алена, мы проверили, что pg_stat_statements от этого спасает только NO_INSTALLCHECK = 1 в contrib/pg_stat_statements/Makefile. aqo себе такого позволить не может.
Во-вторых, вроде каждый тест = отдельное соединение в psql? Т.е. переменная сбросится между тестами?..
There was a problem hiding this comment.
У нас сейчас есть schedule-файл с порядком выполнения тестов. Почему бы не устанавливать это значение в первом тесте и не сбрасывать в последнем?
Не поможет. sheduller настроен на прогон aqo тестов и соответствующие настройки с параметрами, как понимаю, применяются к ним. У нас изначально возникла проблема с ванильными тестами.
Или ты говоришь про sheduller ванильных тестов? Но тогда, это пойдут изменения в ванильный код, чего мы хотим избежать
There was a problem hiding this comment.
А это никак не повлияет на другие тесты, выполняемые installcheck ? И почему?
Ты имеешь в виду другие расширения, которые работают одновременно с AQO и данная настройка их коснется или про тесты в AQO? Или вообще про другое? Можешь уточнить вопрос, пожалуйста.
There was a problem hiding this comment.
А это никак не повлияет на другие тесты, выполняемые installcheck ? И почему?
compute_query_id = 'regress' => не выводить query id, даже если он посчитался. Вроде тогда мы и другие тесты не ломаем, и функциональность по факту тестируется?.. (если что-то упадет при подсчете, то это будет заметно?)
Ну так и сделайте это глобальным параметром своих тестов, если возможно, зачем это в открытое расширение складывать?
There was a problem hiding this comment.
У нас сейчас есть schedule-файл с порядком выполнения тестов. Почему бы не устанавливать это значение в первом тесте и не сбрасывать в последнем?
Во-первых, compute_query_id = 'regress' в любом случае нужен, чтобы не сломался make installcheck в дереве исходников postgres с загруженным aqo. Как писала Алена, мы проверили, что pg_stat_statements от этого спасает только NO_INSTALL CHECK = 1 в contrib/pg_stat_statements/Makefile. aqo себе такого позволить не может.
AQO построен так же, как и pg_stat_statements. По крайней мере гитхабовская версия проекта желательно должна выглядеть традиционно. А патчи для различных энтерпрайзных версий можно и внутри хранить ...Во-вторых, вроде каждый тест = отдельное соединение в psql? Т.е. переменная сбросится между тестами?..
ALTER SYSTEM + pg_reload_conf() там не срабатывает?
There was a problem hiding this comment.
Ну так и сделайте это глобальным параметром своих тестов, если возможно, зачем это в открытое расширение складывать?
Потому что тесты падают только с загруженным расширением (в данном случае - aqo). Без вас все норм.
There was a problem hiding this comment.
ALTER SYSTEM + pg_reload_conf() там не срабатывает?
- pg_reload_conf вроде только отправляет сигнал postmaster? В смысле, надеюсь, в тестах не будет гонок..
- Если запустить паралельный installcheck-world (make -jN installcheck-world), с помощью ALTER SYSTEM вы повлияете на другие тесты?..
It turned out that vanilla regression tests failed using aqo_pg 14.patch in this version. This problem is related to running the Enable Query Id function in _PG_init(). We cannot remove the call to this function due to the launch of assert when query_id_enabled is disabled in query jumble. A similar function to enable query_id_enabled (enable query id) is used in pg_stat_statements, but the Makefile contains the addition of NO_INSTALL CHECK = 1 and does not run regression tests by default.
So, I have prepared changes for fix it.