Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/24557 )

Change subject: IMPALA-15057: Add variant_get() builtin and first-class VARIANT 
expressions
......................................................................


Patch Set 6:

(13 comments)

gerrit-auto-critic failed. You can reproduce it locally using command:

  python3 bin/jenkins/critique-gerrit-review.py --dryrun

To run it, you might need a virtual env with Python3's venv installed.

http://gerrit.cloudera.org:8080/#/c/24557/6/be/src/exprs/anyval-util.h
File be/src/exprs/anyval-util.h:

http://gerrit.cloudera.org:8080/#/c/24557/6/be/src/exprs/anyval-util.h@347
PS6, Line 347:         // A raw VARIANT slot is never converted here; VARIANT 
expression values are staged
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/24557/6/be/src/exprs/anyval-util.h@356
PS6, Line 356:   /// Like SetAnyVal(), but for VARIANT: GetValue() returns a 
VariantVal already in ABI form
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/24557/6/be/src/exprs/anyval-util.h@357
PS6, Line 357:   /// (&result_.variant_val), so it is copied directly rather 
than converted from a raw slot.
line too long (93 > 90)


http://gerrit.cloudera.org:8080/#/c/24557/6/be/src/exprs/scalar-expr-evaluator.h
File be/src/exprs/scalar-expr-evaluator.h:

http://gerrit.cloudera.org:8080/#/c/24557/6/be/src/exprs/scalar-expr-evaluator.h@167
PS6, Line 167:   // VARIANT is a first-class expression type: any expression (a 
scan SlotRef or a function
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/24557/6/be/src/exprs/scalar-expr-evaluator.h@168
PS6, Line 168:   // such as variant_get()) produces a VariantVal, obtained 
through this method rather than
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/24557/6/be/src/exprs/variant-functions-ir.cc
File be/src/exprs/variant-functions-ir.cc:

http://gerrit.cloudera.org:8080/#/c/24557/6/be/src/exprs/variant-functions-ir.cc@211
PS6, Line 211:   // zero-copy slice of the parent value blob. The slice need 
only stay valid for this row;
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/24557/6/be/src/service/hs2-util.cc
File be/src/service/hs2-util.cc:

http://gerrit.cloudera.org:8080/#/c/24557/6/be/src/service/hs2-util.cc@415
PS6, Line 415:     // Every VARIANT value reaches us as a VariantVal via 
GetVariantVal(), never a raw slot.
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/24557/6/be/src/service/query-result-set.cc
File be/src/service/query-result-set.cc:

http://gerrit.cloudera.org:8080/#/c/24557/6/be/src/service/query-result-set.cc@243
PS6, Line 243:     // Handle VARIANT before the SlotRef-only assumption relied 
on by the STRUCT/collection
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/24557/6/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java
File fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java:

http://gerrit.cloudera.org:8080/#/c/24557/6/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@846
PS6, Line 846:    * user-defined function that happens to share the name in 
another database is not caught.
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/24557/6/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@867
PS6, Line 867:    * Validates the arguments common to every variant_get() form: 
the first argument must be
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/24557/6/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@901
PS6, Line 901:     // getStringValue() returns null for a non-UTF8 binary 
literal; treat it as unsupported.
line too long (92 > 90)


http://gerrit.cloudera.org:8080/#/c/24557/6/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@914
PS6, Line 914:     // the shared tail in analyzeImpl() would otherwise validate 
(this path returns early).
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/24557/6/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@928
PS6, Line 928:    * Builds the exception thrown when IGNORE NULLS decorates a 
function that does not accept
line too long (92 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I55bfed394ac2fb57135fadedd489f59e4cc10de4
Gerrit-Change-Number: 24557
Gerrit-PatchSet: 6
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-Comment-Date: Wed, 02 Sep 2026 12:45:16 +0000
Gerrit-HasComments: Yes

Reply via email to