Re: [HACKERS] pgbench more operators & functions
От | Fabien COELHO |
---|---|
Тема | Re: [HACKERS] pgbench more operators & functions |
Дата | |
Msg-id | alpine.DEB.2.20.1712141506280.13653@lancre обсуждение исходный текст |
Ответ на | Re: [HACKERS] pgbench more operators & functions (Teodor Sigaev <teodor@sigaev.ru>) |
Ответы |
Re: [HACKERS] pgbench more operators & functions
|
Список | pgsql-hackers |
Hello Teodor, > Huh, you are fast. Rebase patch during half an hour. Hmmm... just lucky, and other after lunch tasks were more demanding. > I haven't objection about patch idea, but I see some gotchas in coding. > > 1) /* Try to convert variable to numeric form; return false on failure */ > static bool > makeVariableValue(Variable *var) > > as now, makeVariableValue() convert value of eny type, not only numeric Indeed, the comments need updating. I found a few instances. > 2) In makeVariableValue(): > if (pg_strcasecmp(var->svalue, "null") == 0) > ... > else if (pg_strncasecmp(var->svalue, "true", slen) > > mixing of pg_strcasecmp and pg_strNcasecmp. And, IMHO, result of > pg_strncasecmp("tru", "true", 1) will be 0. Yep, but it cannot be called like that because slen == strlen(var->svalue). > It may be good for 't' of 'f' but it seems too free grammar to accept > 'tr' or 'fa' or even 'o' which actually not known to be on or off. Yes, it really works like that. I tried to make something clearer than "src/bin/psql/variable.c". Maybe I did not succeed. I have added a comment and use some shortened versions in tests, with the -D option. > 3) It seems to me that Variable.has_value could be eliminated and then new > PGBT_NOT_SET is added to PgBenchValueType enum as a first (=0) value. This > allows to shorten code and make it more readable, look > setBoolValue(&var->value, true); > var->has_value = true; > The second line actually doesn't needed. Although I don't insist to fix that. I agree that the redundancy should be avoided. I tried to keep "is_numeric" under some form, but there is no added value doing that. > I actually prefer PGBT_NOT_SET instead of kind of PGBT_UNDEFINED, because I > remember a collision in JavaScript with undefined and null variable. I used "PGBT_NO_VALUE" which seemed clearer otherwise a variable may be set and its value would not "not set" which would look strange. > 4) valueTypeName() > it checks all possible PgBenchValue.type but believes that field could > contain some another value. In all other cases it treats as error or assert. Ok. Treated as an internal error with an assert. -- Fabien.
Вложения
В списке pgsql-hackers по дате отправления: