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 6:

(2 comments)

Thanks for the comments!

http://gerrit.cloudera.org:8080/#/c/24392/5/be/src/runtime/variant-value.cc
File be/src/runtime/variant-value.cc:

http://gerrit.cloudera.org:8080/#/c/24392/5/be/src/runtime/variant-value.cc@605
PS5, Line 605:   constexpr auto default_capacity = 
rapidjson::StringBuffer::kDefaultCapacity;
> nit: the "Just C++ things" part sounds odd. Also, it can be avoided by usin
"Just C++ things" was just my sarcastic comment, removed it :)

I find max() more readable than the ternary expression.


http://gerrit.cloudera.org:8080/#/c/24392/5/be/src/runtime/variant-value.cc@614
PS5, Line 614:   RETURN_IF_ERROR(ValueToJson(*this, *metadata_, &writer));
> This is a noop as it's already sized for Len()*2
Oh, forgot to remove it, thanks.



--
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: 6
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Arnab Karmakar <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Noemi Pap-Takacs <[email protected]>
Gerrit-Reviewer: Peter Rozsa <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Mon, 22 Jun 2026 13:40:00 +0000
Gerrit-HasComments: Yes

Reply via email to