Copilot commented on code in PR #2407:
URL: https://github.com/apache/age/pull/2407#discussion_r3150523239
##########
src/backend/utils/adt/agtype_ops.c:
##########
@@ -2098,7 +2098,7 @@ static Datum get_agtype_path_all(FunctionCallInfo fcinfo,
bool as_text)
char* str = NULL;
lindex = strtol(cur_key->val.string.val, &str, 10);
- if (strcmp(str, ""))
+ if (strcmp(str, "") != 0)
{
PG_RETURN_NULL();
}
Review Comment:
This change is fixing path/index parsing for the `#>` / `#>>` operators, and
the repo already has regression coverage for these operators (e.g.,
`regress/sql/jsonb_operators.sql` section "Agtype path extraction operators").
Please add a regression test case that asserts `... #> '[""]'` returns NULL
(and ideally that a large out-of-range numeric string is rejected), to prevent
this from regressing again.
##########
src/backend/utils/adt/agtype_ops.c:
##########
@@ -2098,7 +2098,7 @@ static Datum get_agtype_path_all(FunctionCallInfo fcinfo,
bool as_text)
char* str = NULL;
lindex = strtol(cur_key->val.string.val, &str, 10);
- if (strcmp(str, ""))
+ if (strcmp(str, "") != 0)
{
PG_RETURN_NULL();
}
Review Comment:
The new condition `strcmp(str, "") != 0` is logically equivalent to the
previous `if (strcmp(str, ""))` and still does not reject the empty-string case
described in the PR. When the input is "" strtol() leaves `str` pointing at the
start of the string, so `strcmp(str, "")` is 0 and this check is bypassed,
accepting index 0. To reject empty strings (and generally ensure at least one
digit was consumed), check `str == cur_key->val.string.val` (endptr == start)
in addition to requiring endptr to be at the string terminator.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
