Addchild and fetchchild node compatiblity +grow working#41
Conversation
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…ndependent and other small changes
aivillio
changed the title
There was a problem hiding this comment.
Pull request overview
This PR extends the Adaptive Radix Tree (ART) implementation to correctly support addchild/findchild behavior across Node48 and Node256 (including growth from smaller node types), enabling inserts to continue working as node fanout increases.
Changes:
- Added node-type-specific
addchildlogic forNode4/16,Node48, andNode256, with growth support. - Implemented
findchild, prefix checking (checkprefix), and leaf fetching (fetchleaf) helpers used by insert/search. - Added
searchand a small CLI test harness to insert keys and print the tree.
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/art/util.go | Adds core ART utilities: add/find child, prefix compare, grow, and leaf fetching. |
| internal/art/insert.go | Implements insertion logic with prefix compression and node splitting. |
| internal/art/search.go | Implements lookup logic through prefixes and child navigation. |
| internal/art/node.go | Defines node types, constants, and shared node structures. |
| internal/art/node4.go | Constructs Node4 inner nodes. |
| internal/art/node16.go | Constructs Node16 inner nodes. |
| internal/art/node48.go | Constructs Node48 inner nodes with indirection layer. |
| internal/art/node256.go | Constructs Node256 inner nodes with direct child map. |
| internal/art/leaf.go | Defines leaf structure and isleaf helper. |
| internal/art/print_tree.go | Adds a debug printer for visualizing the ART structure. |
| internal/art/art.go | Adds a minimal public Tree API (Insert, Search, Root). |
| cmd/radFS/arttest/main.go | Simple executable to insert sample keys and print the tree. |
| docs/weekly/angelo/week_4.md | Weekly progress notes update. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Get the next byte to branch on at current depth. | ||
| // Returns 0 (terminator) if key is exhausted. | ||
| k := key[depth] | ||
|
|
||
| // Find the child corresponding to byte k and recurse deeper. | ||
| // Return nil if no child exists for this byte. | ||
| next, _ := findchild(k, n) | ||
| if next != nil { | ||
| return search(next, key, depth+1) |
There was a problem hiding this comment.
search indexes key[depth] without checking whether depth < len(key). When searching for a key that is a prefix of an existing path (or an empty key once the tree has internal nodes), this will panic with an out-of-range error. Add an early check for depth == len(key) (and return n.innerNode.leaf if it matches, otherwise nil) before reading key[depth].
| if isleaf(n) { | ||
| if string(n.leaf.key) == string(key) { | ||
| return n | ||
| } | ||
| return nil |
There was a problem hiding this comment.
Leaf key comparison converts both byte slices to strings (string(n.leaf.key) == string(key)), which performs allocations/copies on each search. Using bytes.Equal(n.leaf.key, key) avoids the extra allocations and is the conventional way to compare []byte values.
clamping the Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
|
Closing in favour of #47 |
PR title OR Closes {Link to issue}
Context
Why are these changes being made?
The addchild and fetch child were not compatitble for node48 and node256
Description
How have the changes been made? i.e. steps taken to solve the above issue
Changes
Technical description of the changes made
Addchild {node48}
Addchild {node256}
*similar functions execpt grow
fetchleaf
Tests
Tests ran after the changes and their results
*Ran main.go
Additional info
MERGE THIS AFTER PR37