Peter Rozsa 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 5: (5 comments) Part 2, still working on it http://gerrit.cloudera.org:8080/#/c/24557/5/be/src/service/hs2-util.cc File be/src/service/hs2-util.cc: http://gerrit.cloudera.org:8080/#/c/24557/5/be/src/service/hs2-util.cc@415 PS5, Line 415: VARIANT is a first-class expression type: nit: it should be only commented on the type definition, same goes for query-result-set.cc http://gerrit.cloudera.org:8080/#/c/24557/5/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/5/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@856 PS5, Line 856: private boolean analyzeVariantGet(Analyzer analyzer, FeDb db) : This methods feels like it's doing at least 3 different things with argument checks/returing bools/path validation http://gerrit.cloudera.org:8080/#/c/24557/5/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@872 PS5, Line 872: StringLiteral nit: StringLiteral stringLiteral http://gerrit.cloudera.org:8080/#/c/24557/5/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@874 PS5, Line 874: startsWith nit:maybe a nullcheck before? http://gerrit.cloudera.org:8080/#/c/24557/5/fe/src/main/java/org/apache/impala/analysis/FunctionCallExpr.java@902 PS5, Line 902: if (params_.isIgnoreNulls()) { : throw new AnalysisException("Function " + fnName_.getFunction().toUpperCase() : + " does not accept the keyword IGNORE NULLS."); : } It' duplicated from analyzeImpl -- 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: 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-Comment-Date: Fri, 28 Aug 2026 13:42:31 +0000 Gerrit-HasComments: Yes
