Re: Re: starting to review the Extend NOT NULL representation to pg_constraint patch

On 14 October 2010 17:40, Bernd Helmle <mailings(at)oopsware(dot)de> wrote:
>
>
> --On 14. Oktober 2010 11:42:27 -0400 Robert Haas <robertmhaas(at)gmail(dot)com>
> wrote:
>
>>> I did a sanity make clean && make && make check before applying the
>>> patch and all the tests passed.  After applying the patch and doing
>>> make clean && make && make check, I got a number of failures of the
>>> form “FAILED (test process exited with exit code 2)”.  The exact
>>> number of failures varies by run, so I’m wondering if I didn’t do
>>> something wrong...
>>
>> That indicates that PostgreSQL is crashing.  So I think this patch is
>> definitely not ready for prime time yet, and needs some debugging.  In
>> view of the fact that we are out of time for this CommitFest, I'm
>> going to mark this Returned with Feedback in the CommitFest
>> application.  Hopefully it will be resubmitted for the next CommitFest
>> after further refinement, because I think this is a good and useful
>> improvement.
>
> Yeah, its crashing but it doesn't do it here on my MacBook.... (passing the
> regression test is one of my top prio when submitting a patch). Must be some
> broken pointer or similar oversight which is triggered on Andrew's box.
> Andrew, which OS and architecture have you tested on?
>

Yeah, it crashes for me too (opensuse 11.2 64-bit) but only in an
optimised build.

There are a couple of compiler warnings:

tablecmds.c: In function 'ATExecSetNotNull':
tablecmds.c:4747: warning: unused variable 'is_new_constraint'
tablecmds.c: In function 'ATExecDropNotNull':
tablecmds.c:4332: warning: 'found' may be used uninitialized in this function
tablecmds.c:4246: warning: 'children' may be used uninitialized in this function

The last 2 look serious enough to cause problems, but fixing them
didn't cure the crash.

Digging deeper, I get a segfault running src/test/regress/sql/alter_table.sql:

Program received signal SIGSEGV, Segmentation fault.
ATExecSetNotNullInternal (is_local=1 '\001',
is_new_constraint=<value optimized out>, atttup=<value optimized out>,
attr_rel=<value optimized out>, rel=<value optimized out>)
at tablecmds.c:4847
4847 Form_pg_constraint constr =
(Form_pg_constraint) GETSTRUCT(copy_tuple);

Looking in that function, there is a similar "found" variable that
isn't being initialised (which my compiler didn't warn about).
Initialising that to false, sems to fix the problem and all the
regression tests then pass.

Regards,
Dean