Discussion threadhere (<- link to https://lists.apache.org/list.html?dev@flink.apache.org)
Vote threadhere (<- link to https://lists.apache.org/list.html?dev@flink.apache.org)
JIRAhere (<- link to https://issues.apache.org/jira/browse/FLINK-XXXX)
Release<Flink Version>

Please keep the discussion on the mailing list rather than commenting on the wiki (wiki discussions get unwieldy fast).

Motivation

When doing the planning phase of a Flink Table job, among other things, we execute an expression reducer, which aims to do constant folding. One of these optimizations is to invoke deterministic functions if all of their arguments are constants.

This is fine for local functions, but becomes more troublesome for UDFs which invoke RPCs or have other side effects, which you either don’t want to occur during planning time, or where it’s important that it happens on a per result row basis.

Currently, FunctionDefinition.isDeterministic() exists, where you can override a definition to indicate that a function call is not deterministic, and this will avoid invoking the expression reducer. Unfortunately, this also indicates that the results are not deterministic and will have a material effect on the planning of the job.

A new mechanism is required to differentiate the scenario where an expression with a function call is deterministic, but still should not be reduced anyway.

Public Interfaces

The proposal is to add a method call to FunctionDefinition :

public interface FunctionDefinition {
...
    /**
     * If the constant-folding should be run during planning time on calls to
     * this function. If not, the expression will be left as-is and the call
     * will be made during runtime.
     *
     * @return Whether invocations can be reduced during planning time
     */
    default boolean supportsConstantFolding() {
        return true;
    }
}

Proposed Changes

ExpressionReducer already supports avoiding constant-folding of certain expressions, so this needs to be extended to support function calls. To do this, we need to be able to identify an expression with a function call which has supportsConstantFolding() returning false. Such an visitor could be written as below.

private static class SupportsConstantFoldingExpressionVisitor extends RexDefaultVisitor<Boolean> {
    @Override
    public Boolean visitNode(RexNode rexNode) {
        return true;
    }
    private boolean supportsConstantFolding(RexCall call) {
        FunctionDefinition definition = ShortcutUtils.unwrapFunctionDefinition(call);
        return definition == null || definition.supportsConstantFolding();
    }
    @Override
    public Boolean visitCall(RexCall call) {
        boolean supportsConstantFolding = supportsConstantFolding(call);
        return supportsConstantFolding
				&& (call.getOperands().stream().allMatch(node -> node.accept(this)));
    }
}

Compatibility, Deprecation, and Migration Plan

Since all existing FunctionDefinitions will have a default supportsConstantFolding() returning true, the behavior will default to what it is today, that the ExpressionReducer can do constant-folding and invocation.

Test Plan

Change will be covered by unit tests which inspect the plans for function calls which both have supportsConstantFolding() return both possibilities, and validate the use of constant-folding.

Rejected Alternatives

None