Remove over-optimistic Assert.

In commit 2489d76c4, I'd thought it'd be safe to assert that a
PlaceHolderVar appearing in a scan-level expression has empty
nullingrels.  However this is not so, as when we determine that a
join relation is certainly empty we'll put its targetlist into a
Result-with-constant-false-qual node, and nothing is done to adjust
the nullingrels of the Vars or PHVs therein.  (Arguably, a Result
used in this way isn't really a scan-level node, but it certainly
isn't an upper node either ...)

It's not clear this is worth any close analysis, so let's just
take out the faulty Assert.

Per report from Robins Tharakan.  I added a test case based on
his example, just in case somebody tries to tighten this up.

Discussion: https://postgr.es/m/CAEP4nAz7Enq3+DEthGG7j27DpuwSRZnW0Nh6jtNh75yErQ_nbA@mail.gmail.com

diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 4348bfef60111593676f7b9bf246f39bf7402424..186fc8014b6203e2d6263ff384b1f77607db0cb0 100644 (file)
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -2205,7 +2205,7 @@ fix_scan_expr_mutator(Node *node, fix_scan_expr_context *context)
        /* At scan level, we should always just evaluate the contained expr */
        PlaceHolderVar *phv = (PlaceHolderVar *) node;
 
-       Assert(phv->phnullingrels == NULL);
+       /* XXX can we assert something about phnullingrels? */
        return fix_scan_expr_mutator((Node *) phv->phexpr, context);
    }
    if (IsA(node, AlternativeSubPlan))

diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index 87e12768042a02d348047369dee3375799f05e56..e73ba93ca3c1b16933c677c1e27bd2b568e27fec 100644 (file)
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -3599,6 +3599,20 @@ where f1 in ( select c3.f1 from ctetable c2 full join ctetable c3 on true );
   1
 (1 row)
 
+-- Test PHV that winds up in a Result node, despite having nonempty nullingrels
+explain (verbose, costs off)
+select table_catalog, table_name
+from int4_tbl t1
+  inner join (int8_tbl t2
+              left join information_schema.column_udt_usage on null)
+  on null;
+                                                    QUERY PLAN                                                     
+-------------------------------------------------------------------------------------------------------------------
+ Result
+   Output: (current_database())::information_schema.sql_identifier, (c.relname)::information_schema.sql_identifier
+   One-Time Filter: false
+(3 rows)
+
 --
 -- test inlining of immutable functions
 --

diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index 2bb24dbfcdc64c76815d2ee37a8dc51b658b62a2..ed26ffd8a86cf8c9199fac9eb875f68a6b48ed10 100644 (file)
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1181,6 +1181,14 @@ with ctetable as not materialized ( select 1 as f1 )
 select * from ctetable c1
 where f1 in ( select c3.f1 from ctetable c2 full join ctetable c3 on true );
 
+-- Test PHV that winds up in a Result node, despite having nonempty nullingrels
+explain (verbose, costs off)
+select table_catalog, table_name
+from int4_tbl t1
+  inner join (int8_tbl t2
+              left join information_schema.column_udt_usage on null)
+  on null;
+
 --
 -- test inlining of immutable functions
 --