-
Notifications
You must be signed in to change notification settings - Fork 1.8k
ra: add support for direct array access notation #10007
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Jorge Niedbalski <[email protected]>
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.
LGTM
Must applaud the rigorous testing. Good work!
} | ||
|
||
/* Handle map objects */ | ||
if (cur.type != MSGPACK_OBJECT_MAP) { |
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.
in this case, what will happen if the accessed value is another array ?
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.
Good question. When the accessed value is another array, the function will process it in the next iteration through if (entry->type == FLB_RA_KEY_ARRAY), which runs before the map type check. This is validated in cb_nested_array_access() test, which successfully accesses $matrix[1][2] from {"matrix": [[1, 2, 3], [4, 5, 6], [7, 8, 9]]} to get the value 6.
Record Accessor API couldn't handle direct array access patterns like $array[0]. This fix changes the condition that checks if a lookup succeeded by replacing !found with matched == 0, which properly tracks array element access. Added test cases to verify array access works: - Direct array access ($array[0]) - Nested array access ($matrix[1][2]) - Mixed array and map access ($records[1]['name']) - Access with out-of-bounds indices - Nonexistent key access - Type mismatch handling - Nested path failures Fixes #9958 Signed-off-by: Jorge Niedbalski <[email protected]>
@edsiper @cosmo0920 I added a few extra test cases to cover cfl conversion as well. PTAL |
Description
This PR fixes an issue where the Record Accessor API couldn't handle direct array access patterns like
$array[0]
.The fix changes the condition that checks if a lookup succeeded by replacing
!found
withmatched == 0
, which properly tracks array element access.Added test cases to verify array access works:
$array[0]
)$matrix[1][2]
)$records[1]['name']
)Fixes #9958
Testing
Attached Valgrind output that shows no leaks or memory corruption was found
[N/A]
Run local packaging test showing all targets build
[N/A]
Documentation
[N/A]
Backporting
[N/A]