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


Reply via email to