- Notifications
You must be signed in to change notification settings - Fork86
REL_2_5-PBCKP-236#531
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
…t_remote_major_pg fixes
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
В целом, хорошо.
Я попрошу учесть замечания, хоть они больше стилистические, чем по существу.
src/utils/remote.c Outdated
static bool check_certified() | ||
{ | ||
return strstr(PGPRO_VERSION_STR, "(certified)") || | ||
strstr(PGPRO_VERSION_STR, ("(standard certified)")); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
А зачем тут скобочки вокруг строки?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
PGPRO_VERSION_STR выглядит как "PostgresPro 11.17.1 (certified) on x86_64-pc-linux-gnu, ...", скобочки отсюда, чтобы быть более strict.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
src/utils/remote.c Outdated
for (int i = 0; i <compatibility_params_array_size; i+=2) | ||
for (int i = 0; i <sizeof compatibility_params; i+=2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Было правильно, зря убрал.
src/utils/remote.c Outdated
{ | ||
if (compatibility_params[i].strval != NULL) | ||
result_size += snprintf(compatibility_buf + result_size, compatibility_buf_size - result_size, | ||
"%s=%s/n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Слэш в другую сторону. И ниже тоже.
"CREATE TABLE ultimate_question2 AS SELECT 42 AS answer") | ||
# do delta catchup with remote pg_probackup agent with another postgres major version | ||
# this DELTA backup should fail without PBCKP-236 patch. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
А тест не должен падать в любом случае? По крайней мере, кажется что с патчем-то он точно должен падать.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
должен, он вообще не должен срабатывать, так как ссылается на абсолютный путь на бинарь другой версии. мне не понятно почему CI не репортит это. на планерке спрошу гипотезы.
Uh oh!
There was an error while loading.Please reload this page.
@funny-falcon please review