Re: Event triggers + table partitioning cause server crash in current master

On Mon, May 15, 2017 at 03:02:54PM +0900, Amit Langote wrote:
> On 2017/05/14 12:03, Mark Dilger wrote:
> > I discovered a reproducible crash using event triggers in the current
> > development version, 29c7d5e4844443acaa74a0d06dd6c70b320bb315.
> > I was getting a crash before this version, and cloned a fresh copy of
> > the sources to be sure I was up to date, so I don't think the bug can be
> > attributed to Andres' commit. (The prior version I was testing against
> > was heavily modified by me, so I recreated the bug using the latest
> > standard, unmodified sources.)
> >
> > I create both before and after event triggers early in the regression test
> > schedule, which then fire here and there during the following tests, leading
> > fairly reproducibly to the server crashing somewhere during the test suite.
> > These crashes do not happen for me without the event triggers being added
> > to the tests. Many tests show as 'FAILED' simply because the logging
> > that happens in the event triggers creates unexpected output for the test.
> > Those "failures" are expected. The server crashes are not.
> >
> > The server logs suggest the crashes might be related to partitioned tables.
> >
> > Please find attached the patch that includes my changes to the sources
> > for recreating this bug. The logs and regression.diffs are a bit large; let
> > me know if you need them.
> >
> > I built using the command
> >
> > ./configure --enable-cassert --enable-tap-tests && make -j4 && make check
>
> Thanks for the report and providing steps to reproduce.
>
> It seems that it is indeed a bug related to creating range-partitioned
> tables. DefineRelation() calls AlterTableInternal() to add NOT NULL
> constraints on the range partition key columns, but the code fails to
> first initialize the event trigger context information. Attached patch
> should fix that.
>
> Thanks to the above test case, I also discovered that in the case of
> creating a partition, manipulations performed by MergeAttributes() on the
> input schema list may cause it to become invalid, that is, the List
> metadata (length) will no longer match the reality, because while the
> ListCells are deleted from the input list, the List pointer passed to
> list_delete_cell does not point to the same list. This caused a crash
> when the CreateStmt in question was subsequently passed to copyObject,
> which tried to access CreateStmt.tableElts that has become invalid as just
> described. The attached patch also takes care of that.

[Action required within three days. This is a generic notification.]

The above-described topic is currently a PostgreSQL 10 open item. Robert,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.

[1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com