Re: concerns around pg_lsn
| От | Jeevan Ladhe |
|---|---|
| Тема | Re: concerns around pg_lsn |
| Дата | |
| Msg-id | CAOgcT0Mp8uQ44xmjb8Whf16U7dELETe6BquyBKL4hWjkuArPUA@mail.gmail.com обсуждение исходный текст |
| Ответ на | Re: concerns around pg_lsn (Michael Paquier <michael@paquier.xyz>) |
| Ответы |
Re: concerns around pg_lsn
(Jeevan Ladhe <jeevan.ladhe@enterprisedb.com>)
Re: concerns around pg_lsn (Michael Paquier <michael@paquier.xyz>) |
| Список | pgsql-hackers |
Hi Michael,
On further review of the first patch, I think that it could be a good
idea to apply the same safeguards within float8in_internal_opt_error.
Jeevan, what do you think?
Sure, agree, it makes sense to address float8in_internal_opt_error(),
there might be more occurrences of such instances in other functions
as well. I think if we agree, as and when encounter them while touching
those areas we should fix them.
What is more dangerous with float8in_internal_opt_error() is, it has
the have_error flag, which is never ever set or used in that function. Further
more risks are - the callers of this function e.g. executeItemOptUnwrapTarget()
are passing a non-null pointer to it(default set to false) and expect to throw
an error if it sees some error during float8in_internal_opt_error(), *but*
float8in_internal_opt_error() has actually never touched the have_error flag.
So, in this case it is fine because the flag was set to false, if it was not
set, then the garbage value would always result in true and keep on throwing
an error!
Here is relevant code from function executeItemOptUnwrapTarget():
{code}
975 if (jb->type == jbvNumeric)
976 {
977 char *tmp = DatumGetCString(DirectFunctionCall1(numeric_out,
978 NumericGetDatum(jb->val.numeric)));
979 bool have_error = false;
980
981 (void) float8in_internal_opt_error(tmp,
982 NULL,
983 "double precision",
984 tmp,
985 &have_error);
986
987 if (have_error)
988 RETURN_ERROR(ereport(ERROR,
989 (errcode(ERRCODE_NON_NUMERIC_JSON_ITEM),
990 errmsg("jsonpath item method .%s() can only be applied to a numeric value",
991 jspOperationName(jsp->type)))));
992 res = jperOk;
993 }
994 else if (jb->type == jbvString)
995 {
996 /* cast string as double */
997 double val;
998 char *tmp = pnstrdup(jb->val.string.val,
999 jb->val.string.len);
1000 bool have_error = false;
1001
1002 val = float8in_internal_opt_error(tmp,
1003 NULL,
1004 "double precision",
1005 tmp,
1006 &have_error);
1007
1008 if (have_error || isinf(val))
1009 RETURN_ERROR(ereport(ERROR,
1010 (errcode(ERRCODE_NON_NUMERIC_JSON_ITEM),
1011 errmsg("jsonpath item method .%s() can only be applied to a numeric value",
1012 jspOperationName(jsp->type)))));
1013
1014 jb = &jbv;
1015 jb->type = jbvNumeric;
1016 jb->val.numeric = DatumGetNumeric(DirectFunctionCall1(float8_numeric,
1017 Float8GetDatum(val)));
1018 res = jperOk;
1019 }
{code}
I will further check if by mistake any further commits have removed references
to assignments from float8in_internal_opt_error(), evaluate it, and set out a
patch.
This is one of the reason, I was saying it can be taken as a good practice to
let the function who is accepting an out parameter sets the value for sure to
some or other value.
Regards,
Jeevan Ladhe
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Thomas MunroДата:
Сообщение: Re: Rearranging ALTER TABLE to avoid multi-operations bugs