Abhishek Chennaka has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/23056 )

Change subject: KUDU-1261 introduce Flatbuffers into thirdparty
......................................................................


Patch Set 10:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/23056/10/src/kudu/benchmarks/serdes/serdes-test.cc
File src/kudu/benchmarks/serdes/serdes-test.cc:

http://gerrit.cloudera.org:8080/#/c/23056/10/src/kudu/benchmarks/serdes/serdes-test.cc@117
PS10, Line 117: ASSERT_EQ(validity_src.size(), validity->size());
Maybe compare the actual contents and not just sizes to be sure.


http://gerrit.cloudera.org:8080/#/c/23056/10/src/kudu/benchmarks/serdes/serdes-test.cc@146
PS10, Line 146:   // Extract data from the buffer and verify it matches the 
source.
              :   const uint8_t* buf = builder.GetBufferPointer();
              :   {
              :     Verifier verifier(buf, kBufSize);
              :     ASSERT_TRUE(VerifyContentBuffer(verifier));
              :
              :     const Content* content = GetContent(buf);
              :     ASSERT_NE(nullptr, content);
              :     const auto array_type = content->data_type();
              :     ASSERT_EQ(ScalarArray::Int16, array_type);
              :
              :     const auto* values = content->data_as<Int16>()->values();
              :     ASSERT_NE(nullptr, values);
              :     ASSERT_EQ(7, values->size());
              :     for (size_t i = 0; i < values->size(); ++i) {
              :       ASSERT_EQ(i + 1, values->Get(i));
              :     }
              :
              :     const auto* validity = content->validity();
              :     ASSERT_EQ(validity_src.size(), validity->size());
              :
              :     // Verify raw data access.
              :     const int16_t* data_src = values_src.data();
              :     const int16_t* data = values->data();
              :     for (size_t i = 0; i < 7; ++i) {
              :       SCOPED_TRACE(Substitute("array index: $0", i));
              :       ASSERT_TRUE(*data_src++ == *data++);
              :     }
nit: Maybe consider templating this part to avoid repetition.



--
To view, visit http://gerrit.cloudera.org:8080/23056
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I89c697b8d80cbbd2af4233d16806a230cedaa81a
Gerrit-Change-Number: 23056
Gerrit-PatchSet: 10
Gerrit-Owner: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Abhishek Chennaka <[email protected]>
Gerrit-Reviewer: Alexey Serbin <[email protected]>
Gerrit-Reviewer: Ashwani Raina <[email protected]>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Wed, 03 Sep 2025 22:23:42 +0000
Gerrit-HasComments: Yes

Reply via email to