[jira] [Resolved] (IMPALA-13136) Refactor AnalyzedFunctionCallExpr

["Steve Carlin (Jira)" <ji...@apache.org>]

     [ https://issues.apache.org/jira/browse/IMPALA-13136?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Steve Carlin resolved IMPALA-13136.
-----------------------------------
    Resolution: Fixed

> Refactor AnalyzedFunctionCallExpr
> ---------------------------------
>
>                 Key: IMPALA-13136
>                 URL: https://issues.apache.org/jira/browse/IMPALA-13136
>             Project: IMPALA
>          Issue Type: Improvement
>          Components: Frontend
>            Reporter: Steve Carlin
>            Priority: Major
>
> Copied from code review:
> The part where we immediately analyze as part of the constructor makes for complicated exception handling. RexVisitor doesn't support exceptions, so it adds complication to handle them under those circumstances. I can't really explain why it is necessary.
> Let me sketch out an alternative:
> 1. Construct the whole Expr tree without analyzing it
> 2. Any errors that happen during this process are not usually actionable by the end user. It's good to have a descriptive error message, but it doesn't mean there is something wrong with the SQL. I think that it is ok for this code to throw subclasses of RuntimeException or use Preconditions.checkState() with a good explanation.
> 3. When we get the Expr tree back in CreateExprVisitor::getExpr(), we call analyze() on the root node, which does a recursive analysis of the whole tree.
> 4. The special Expr classes don't run analyze() in the constructor, don't keep a reference to the Analyzer, and don't override resetAnalysisState(). They override analyzeImpl() and they should be idempotent. The clone constructor should not need to do anything special, just do a deep copy.
> I don't want to bog down this review. If we want to address this as a followup, I can live with that, but I don't want us to go too far down this road. (Or if we have a good explanation for why it is necessary, then we can write a good comment and move on.)



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

---------------------------------------------------------------------
To unsubscribe, e-mail: issues-all-unsubscribe@impala.apache.org
For additional commands, e-mail: issues-all-help@impala.apache.org