westonpace commented on a change in pull request #9100: URL: https://github.com/apache/arrow/pull/9100#discussion_r552047298
########## File path: cpp/src/arrow/util/trie_test.cc ########## @@ -175,6 +175,15 @@ TEST(Trie, EmptyString) { ASSERT_EQ(-1, trie.Find("x")); } +TEST(Trie, LongString) { + TrieBuilder builder; + ASSERT_OK(builder.Append("")); + const Trie trie = builder.Finish(); + auto maxlen = static_cast<size_t>(std::numeric_limits<int16_t>::max()) + 1; Review comment: I did this and split the cases into good cases and "good & bad" cases. For the good cases I confirmed we can insert and match. For the "good & bad" cases I confirmed we don't match the empty string. I tested on Mac and the test failed (it matched the empty string). I then added your patch and it passed (well, it failed because of the >/>= issue but passed after that). I'm testing 2^15-2, 2^15-1, 2^14 (good) and 2^15, 2^16-2 (bad). Do you think that covers it? ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org