Zoltan Borok-Nagy has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24635 )

Change subject: IMPALA-15221: Harden VARIANT decoding against corrupt input
......................................................................


Patch Set 5:

(5 comments)

Thanks for the comments!

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

http://gerrit.cloudera.org:8080/#/c/24635/3/be/src/runtime/variant-value.cc@89
PS3, Line 89: ts
> can it be == ? I assume that keys can't be 0 bytes
No, {"":1} is a valid JSON (and VARIANT). Added comment.


http://gerrit.cloudera.org:8080/#/c/24635/3/be/src/runtime/variant-value.cc@123
PS3, Line 123: }
> Can code actually call with larger indexes? My impression is that DCHECK wa
Done


http://gerrit.cloudera.org:8080/#/c/24635/3/be/src/runtime/variant-value.cc@235
PS3, Line 235:     return false;
             :   }
> Merge to bool checkType(VariantPhysicalType expected)? Or even better, merg
Done


http://gerrit.cloudera.org:8080/#/c/24635/3/be/src/runtime/variant-value.cc@561
PS3, Line 561:           if (UNLIKELY(!val.ReadValue(&unscaled, 2))) {
             :             return Status("Malformed variant DECIMAL8");
             :           }
> Not sure what is the best way here, but it is awkward that an out of range
Return ERROR here, which becomes SQL NULL, but we still add a warning to 'ctx'.


http://gerrit.cloudera.org:8080/#/c/24635/3/be/src/util/variant-util-test.cc
File be/src/util/variant-util-test.cc:

http://gerrit.cloudera.org:8080/#/c/24635/3/be/src/util/variant-util-test.cc@869
PS3, Line 869: TEST(VariantUtilTest, MetadataCorruptionRejected) {
> The syntax loks a bit weird, why not reuse the same "d" bector multiple tim
Done



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I3a22bb16cd05854a9afb56ebd27806ac52670bca
Gerrit-Change-Number: 24635
Gerrit-PatchSet: 5
Gerrit-Owner: Zoltan Borok-Nagy <[email protected]>
Gerrit-Reviewer: Csaba Ringhofer <[email protected]>
Gerrit-Reviewer: Impala Public Jenkins <[email protected]>
Gerrit-Reviewer: Peter Rozsa <[email protected]>
Gerrit-Reviewer: Zoltan Borok-Nagy <[email protected]>
Gerrit-Comment-Date: Tue, 28 Jul 2026 14:20:43 +0000
Gerrit-HasComments: Yes

Reply via email to