Push down COALESCE in PostgreSQL connector

[ebyhr]

Description

Push down COALESCE in PostgreSQL connector

Documentation

(x) No documentation is needed.
( ) Sufficient documentation is included in this PR.
( ) Documentation PR is available with #prnumber.
( ) Documentation issue #issuenumber is filed, and can be handled later.

Release notes

( ) No release notes entries required.
(x) Release notes entries required with the following suggested text:

# PostgreSQL
* Improve performance of queries involving `COALESCE` by pushing expression
  computation to the underlying database. ({issue}`issuenumber`)

[findepi]

@ebyhr can you please rebase?

[ebyhr]

@findepi Rebased on upstream.

[findepi]

"Translate COALESCE to connector expressions"

[findepi]

@wendigo 's PR adds some shorthand function for this, am i right, @wendigo ?

no change requested now, but we should unify later

[wendigo]

Yes,I will note it down to unify that after those PRs are merged.

[findepi]

TryExpression is desugared to try function and it's marked as non-deterministic.
We don't push non-deterministic predicates down into the connector, however, they are handled separately from non-translatable ones.

Thus, it's a wrong change, as it changes the semantics of the test case.

[ebyhr]

Changed to SearchedCaseExpression though I have a local patch to support it as connector expressions. Do you have other suggested expressions?

[findepi]

See #11511 (comment) and #11544 (comment)

I don't have a plan for testing things. @martint do you have one?
cc @wendigo @hashhar

[findepi]

@martint by any chance, does coalesce have lazy eval semantics?

cc @kasiafi

[martint]

@martint by any chance, does coalesce have lazy eval semantics?

Yes, coalesce is described by the SQL spec as syntactic sugar for a CASE expression.

<case expression> ::=
    <case abbreviation>
  | <case specification>

<case abbreviation> ::=
    NULLIF <left paren> <value expression> <comma> <value expression> <right paren>
  | COALESCE <left paren> <value expression>
      { <comma> <value expression> }... <right paren>

d) COALESCE (V1, V2) is equivalent to the following <case specification>:

  CASE 
    WHEN V1 IS NOT NULL THEN 
      V1 
    ELSE
      V2 
  END

e) COALESCE (V1, V2, ..., Vn), for n ≥ 3, is equivalent to the following <case specification>:

  CASE 
    WHEN V1 IS NOT NULL THEN 
      V1 
    ELSE 
      COALESCE (V2, ..., Vn) 
  END

[findepi]

If i remember correctly, the CASE's conditions are semantically evaluated eagerly, but the values are conditional.
Since the COALESCE is defined as equivalent to recursive CASE (not flattened), it means the components are semantically evaluated lazily,

E.g. this should not fail

coalesce(CASE WHEN x = 0 THEN 1 ELSE NULL END, 10 / x)

similar to #11699 (comment), this means this should not be modeled as a function call.

OTOH, modeling COALESCE as a recursive CASE in ConnectorExpressions will be harder to consume within connectors.

@martint what about we model COALESCE as a new Coalesce class?

[ebyhr]

(Rebased on upstream to resolve conflicts)

[findepi]

OTOH, modeling COALESCE as a recursive CASE in ConnectorExpressions will be harder to consume within connectors.

@martint what about we model COALESCE as a new Coalesce class?

@martint and I talked about this and because coalesce features lazy eval, the current consensus is that should be modeled as a function that takes lazy value suppliers, i.e. lambdas. The alternative of implementing this as a dedicated form of a ConnectorExpression would work, but we would deviate from "have connector expressions as simple as possible" principle and was currently rejected.

However, lambdas support is not a trivial thing, and we don't think we're currently ready to work on this yet.

[ebyhr]

Thanks for your explanation. Let me close this PR.