Crash if more than one INITPLAN function is used in query

[hlinnaka]

Greenplum version or build

master, 6X_STABLE

Step to reproduce the behavior

create table foo (i int);
insert into foo select generate_series(1, 5);
create table bar (t text);
insert into bar values ('foobar');


CREATE FUNCTION get_foo() RETURNS SETOF int
AS $$
begin
  RETURN QUERY SELECT * FROM foo;
END; $$
LANGUAGE 'plpgsql' EXECUTE ON INITPLAN;

CREATE FUNCTION get_bar() RETURNS SETOF text
AS $$
begin
  RETURN QUERY SELECT * FROM bar;
END; $$
LANGUAGE 'plpgsql' EXECUTE ON INITPLAN;

create table t as select * from get_bar(), get_foo();

The INITPLAN mechanism isn't prepared to deal two INITPLAN functions being used in the same query, and it mixes up the tuplestores of the two. In this case, it leads to a crash, when an integer is interpreted as a text pointer. With different datatypes, you would get incorrect query results or other errors instead.

[d]

Huh? This feature went into master but I'm a bit surprised we are seeing this bug on 6X_STABLE...

[hlinnaka]

I bumped into this while refactoring ShareInputScans. This function INITPLAN sharing mechanism is quite similar to the way ShareInputScans work. One important difference is that the tuplestore created for INITPLAN function is carried oer from the gang that exeucutes the init plan, to the gang that executes the main plan. A similar scenario would actually be possible with ShareInputScans too, if you have a query with a CTE that's used both in an InitPlan, and the main plan. To avoid that situation, the CTE planning code prevents CTE sharing from happening across subplans.

I think there's an opportunity here to handle both cases: INITPLAN functions, and CTE sharing from InitPlans, with the same code. This INITPLAN handling in FunctionScans is a pretty big hack, but perhaps we could use the ShareInputScan infrastructure we have? So instead of having special code in FunctionScan, the planner would generate a ShareInputScan on top of the FunctionScan. So the plan would look like this:

postgres=# explain create table t as select * from get_bar();
                    QUERY PLAN                    
--------------------------------------------------
 Redistribute Motion 1:3  (slice1)
   Hash Key: get_bar.get_bar
   ->  Share Input Scan on get_bar
         InitPlan 1 (returns $0)  (slice2)
           ->  Share Input Scan on get_bar
                ->  Function Scan on get_bar get_bar_1
 Optimizer: Postgres query optimizer
(6 rows)

Now, as I mentioned earlier, ShareInputScans don't actually support that at the moment. Even though the InitPlan would be executed and the tuplestore generated, it would get destroyed after the InitPlan was executed. But maybe we could find a way to fix that.

It would be pretty straightforward to just not destroy the tuplestore at the end of the InitPlan execution. But then it will get leaked on abort. And actually, the FunctionScan hack has that exact same problem; if the FunctionScan doesn't get executed in the main plan slice, the temporary file is leaked. For example:

postgres=#   create table t as select (1 / (0* random()))::text union all select * from get_bar();
psql: NOTICE:  Table doesn't have 'DISTRIBUTED BY' clause -- Using column(s) named 'text' as the Greenplum Database data distribution key for this table.
HINT:  The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew.
psql: ERROR:  division by zero  (entry db 127.0.0.1:5432 pid=21673)

~/gpdb.master$ ls -l data-master/base/pgsql_tmp/
total 32
-rw------- 1 heikki heikki 32768 Apr 17 23:21 pgsql_tmp_FUNCTION_SCAN_8
-rw------- 1 heikki heikki     0 Apr 17 23:21 pgsql_tmp_FUNCTION_SCAN_8_LOB

[hlinnaka]

Huh? This feature went into master but I'm a bit surprised we are seeing this bug on 6X_STABLE...

I admit I didn't test it on 6X_STABLE, but the feature was backpatched so I assumed it's similarly buggy on 6X_STABLE.

I was very surprised to see that it was backpatched to 6X_STABLE, though.

[hlinnaka]

@zhangh43

[zhangh43]

Thanks @hlinnaka !
The original commit is just focus on supporting one INITPLAN function. My mistake for this PANIC issue.

I fixed the multiple INITPLAN function by introducing the initplan_id for each tuplestore file. So different INITPLAN fucntions could use different tuplestore files. Details in #10156

I haven't tried your Share Input Scan + Function scan solution(possible in my first glance), I opened a new issue #10157 to track this idea.

[hlinnaka]

@zhangh43, why closed?

[hlinnaka]

Ah, this was fixed in #10156. Gotcha