Zoltan Borok-Nagy has posted comments on this change. ( http://gerrit.cloudera.org:8080/24392 )
Change subject: IMPALA-15067: Add VariantValue and basic decoding functions ...................................................................... Patch Set 3: (4 comments) Thanks for the comments! http://gerrit.cloudera.org:8080/#/c/24392/2/be/src/runtime/variant-value.cc File be/src/runtime/variant-value.cc: http://gerrit.cloudera.org:8080/#/c/24392/2/be/src/runtime/variant-value.cc@43 PS2, Line 43: offset_size_ = ((header >> 6) & 0x03) + 1; > from Metadata encoding grammar: Great catch! Done. http://gerrit.cloudera.org:8080/#/c/24392/2/be/src/runtime/variant-value.cc@203 PS2, Line 203: int VariantValue::ObjectFieldIdSize() const { : DCHECK_EQ(GetBasicType(), VariantBasicType::OBJECT); : return ((data_[0] >> 4) & 0x03) + 1; : } : : int VariantValue::ObjectOffsetSize() const { : DCHECK_EQ(GetBasicType(), VariantBasicType::OBJECT); : return ((data_[0] >> 2) & 0x03) + 1; : } > As per the variant encoding spec https://github.com/apache/parquet-format/b Great catch! Done. http://gerrit.cloudera.org:8080/#/c/24392/2/be/src/util/variant-util-test.cc File be/src/util/variant-util-test.cc: PS2: > we could cover some gaps in testing: Thanks for the recommendations, added tests for these. http://gerrit.cloudera.org:8080/#/c/24392/2/be/src/util/variant-util.cc File be/src/util/variant-util.cc: http://gerrit.cloudera.org:8080/#/c/24392/2/be/src/util/variant-util.cc@21 PS2, Line 21: > Unused import. Done -- To view, visit http://gerrit.cloudera.org:8080/24392 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I904618570e8c21d099c9a96b496d85e9246483de Gerrit-Change-Number: 24392 Gerrit-PatchSet: 3 Gerrit-Owner: Zoltan Borok-Nagy <[email protected]> Gerrit-Reviewer: Arnab Karmakar <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]> Gerrit-Comment-Date: Tue, 09 Jun 2026 15:49:47 +0000 Gerrit-HasComments: Yes
