Start erroring out for unsupported lateral subqueries

[JelteF]

DESCRIPTION: Start erroring out for unsupported lateral subqueries

With the introduction of #4385 we inadvertently started allowing and
pushing down certain lateral subqueries that were unsafe to push down.
To be precise the type of LATERAL subqueries that is unsafe to push down
has all of the following properties:

1. The lateral subquery contains some non recurring tuples
2. The lateral subquery references a recurring tuple from
   outside of the subquery (recurringRelids)
3. The lateral subquery requires a merge step (e.g. a LIMIT)
4. The reference to the recurring tuple should be something else than an
   equality check on the distribution column, e.g. equality on a non
   distribution column.

Property number four is considered both hard to detect and probably not
used very often. Thus this PR ignores property number four and causes
query planning to error out if the first three properties hold.

Fixes #5327

[onderkalaci]

bool		lateral;		/* subquery, function, or values is LATERAL? */

From RangeTblEntry

[marcocitus]

do we need to consider something like LEFT JOIN LATERAL fn((SELECT count(*) FROM dist)) ON (true)?

[onderkalaci]

As of today, we replace FUNCTIONs in the JOIN trees with subqueries automatically: https://github.com/citusdata/citus/blob/master/src/backend/distributed/planner/recursive_planning.c#L1588

But seems to ignore LATERALS: https://github.com/citusdata/citus/blob/master/src/backend/distributed/planner/recursive_planning.c#L1803-L1812

So, definitely worth testing but I wouldn't mind if it fails

[marcocitus]

Seems this is incorrectly allowed:

 CREATE OR REPLACE FUNCTION fn(x int)
RETURNS int
AS $$
DECLARE
BEGIN
        RETURN x + 1;
END;
$$ LANGUAGE plpgsql;

SELECT count(*)
FROM ref JOIN test on ref.b = test.x,
    LATERAL fn((
        SELECT
            test_2.x
        FROM test test_2
        WHERE
            test_2.x = test.x
        LIMIT 1
    )) q
;

[JelteF]

Yes, but it's not just this new code that doesn't handle it. It doesn't even get to the DeferredErrorIfUnsupportedLateralSubquery check. This is because RelationInfoContainsOnlyRecurringTuples is not handling this correctly. I discussed with Onder and the following query is also allowed incorrectly, which even uses a LEFT JOIN with a reference table on the left side.

SELECT count(*)
FROM ref LEFT JOIN GREATEST(
        0,
        (SELECT
            test.y
        FROM test
        WHERE
            test.y = ref.a
        LIMIT 1)
    ) ON TRUE;

[JelteF]

Without the left join also returns incorrect results:

SELECT count(*)
FROM ref,
    LATERAL GREATEST(
        0,
        (SELECT
            test.y
        FROM test
        WHERE
            test.y = ref.a
        LIMIT 1)
    );

[JelteF]

Another one that returns wrong results:

SELECT count(*)
FROM (select GREATEST(1, 3)) ref(a),
    LATERAL (
        SELECT
            test.y
        FROM test
        WHERE
            test.y = ref.a
        LIMIT 2
    ) q;

[onderkalaci]

For future reference: Our brief check with Jelte showed that there are SubPlans and they do not show up in plannerInfo->simple_rte_array[relationId]; So, we might need to include the Rtes of subplans on all the places we rely on plannerInfo->simple_rte_array[relationId];

[marcocitus]

some elaboration on the TODO with this context would be helpful

[marcocitus]

this might error on , LATERAL (SELECT .. ) type queries, which work fine so we probably don't want to break those

[JelteF]

There's this query that looks fine, but also fails

SELECT count(*)
FROM test,
    LATERAL (
        SELECT
            test_2.y
        FROM test test_2
        WHERE
            test_2.x = test.x
        LIMIT 2
    ) q
    JOIN ref ON q.y = ref.a;

[codecov]

Codecov Report

Merging #5753 (989761c) into master (3f72cda) will decrease coverage by 0.12%.
The diff coverage is 84.61%.

❗ Current head 989761c differs from pull request most recent head 4af45fd. Consider uploading reports for the commit 4af45fd to get more accurate results

@@            Coverage Diff             @@
##           master    #5753      +/-   ##
==========================================
- Coverage   92.70%   92.58%   -0.13%     
==========================================
  Files         228      226       -2     
  Lines       47865    47426     -439     
==========================================
- Hits        44372    43907     -465     
- Misses       3493     3519      +26     

[marcocitus]

is it possible to cover this code?

[marcocitus]

some elaboration on the TODO with this context would be helpful

[JelteF]

Merging this now so it's included for release testing. I'll add some more coverage somewhere next week.

[onderkalaci]

should we backport this to 10.2?

[marcocitus]

No, some queries might start erroring out.