Re: [HACKERS] tripping an assert in 8.1.6 (more info)

2007-01-23 Thread Tom Lane
I wrote:
> After further thought I've developed a modified version of Brian's case
> that crashes 8.2 and HEAD but not 8.1 --- it turns the situation around
> by having the view fall back to typmod -1.  So what I'm now thinking
> is that the real problem is that an Append path generates its Vars by
> copying the first input, and in these sorts of situations that might not
> produce the correct typmod.  Back to the drawing board ...

Nope, that's not right either.  What it seems to boil down to is what I
said the first time: removal of trivial subqueryscans can leave us with
a plan that uses an above-a-union Var to describe a value from one of
the union inputs.  Some other 8.2 changes moved the subqueryscans
around, which is why the cases that manifest are a bit different, but
in the end it's the same issue.

I've concluded that this isn't really wrong; it's certainly not worth
disabling the trivial-subqueryscan optimization for, and I think getting
the planner to avoid the problem without that would be tricky.
Accordingly, I've applied the attached patch that just relaxes the
Assert.

regards, tom lane


Index: execScan.c
===
RCS file: /cvsroot/pgsql/src/backend/executor/execScan.c,v
retrieving revision 1.39
diff -c -r1.39 execScan.c
*** execScan.c  5 Jan 2007 22:19:27 -   1.39
--- execScan.c  24 Jan 2007 01:03:03 -
***
*** 215,222 
return false;   /* out of order */
if (att_tup->attisdropped)
return false;   /* table contains dropped 
columns */
Assert(var->vartype == att_tup->atttypid);
!   Assert(var->vartypmod == att_tup->atttypmod);
  
tlist_item = lnext(tlist_item);
}
--- 215,232 
return false;   /* out of order */
if (att_tup->attisdropped)
return false;   /* table contains dropped 
columns */
+   /*
+* Note: usually the Var's type should match the tupdesc 
exactly,
+* but in situations involving unions of columns that have 
different
+* typmods, the Var may have come from above the union and 
hence have
+* typmod -1.  This is a legitimate situation since the Var 
still
+* describes the column, just not as exactly as the tupdesc 
does.
+* We could change the planner to prevent it, but it'd then 
insert
+* projection steps just to convert from specific typmod to 
typmod -1,
+* which is pretty silly.
+*/
Assert(var->vartype == att_tup->atttypid);
!   Assert(var->vartypmod == att_tup->atttypmod || var->vartypmod 
== -1);
  
tlist_item = lnext(tlist_item);
}

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


Re: [HACKERS] tripping an assert in 8.1.6 (more info)

2007-01-23 Thread Tom Lane
I wrote:
> I'm tempted to suggest that we just remove the Assert on vartypmod in
> the 8.1 branch.  The Assert on vartype is doing as much as is really
> important to check, and I don't want to disable the trivial_subqueryscan
> optimization, which seems the only other low-risk fix.

After further thought I've developed a modified version of Brian's case
that crashes 8.2 and HEAD but not 8.1 --- it turns the situation around
by having the view fall back to typmod -1.  So what I'm now thinking
is that the real problem is that an Append path generates its Vars by
copying the first input, and in these sorts of situations that might not
produce the correct typmod.  Back to the drawing board ...

regards, tom lane


CREATE TABLE foo
(
x_data varchar(32),
row_date date
);

CREATE TABLE bar
(
x_data varchar(36),
row_date date
);

CREATE OR REPLACE VIEW bazz AS
SELECT
('bar: ' || bar.row_date) :: TEXT AS action,
bar.x_data AS more_data,
row_date
FROM
bar
UNION ALL
SELECT
('foo: ' || foo.row_date) :: TEXT AS action,
foo.x_data AS more_data,
row_date
FROM
foo
;

SELECT action, more_data FROM bazz;

---(end of broadcast)---
TIP 7: You can help support the PostgreSQL project by donating at

http://www.postgresql.org/about/donate


Re: [HACKERS] tripping an assert in 8.1.6 (more info)

2007-01-23 Thread Tom Lane
Brian Hurt <[EMAIL PROTECTED]> writes:
> Tom Lane wrote:
>> Hmm, I thought that stack trace looked a bit familiar --- we seem to
>> have fixed the problem as of 8.2.  Unfortunately I can't recall what
>> the relevant change was exactly; time for some digging in the CVS logs.

> Any hope of getting the fix back-ported into the 8.1 tree?

The bug I was remembering is this one:
http://archives.postgresql.org/pgsql-hackers/2006-12/msg00313.php
http://archives.postgresql.org/pgsql-committers/2006-12/msg00055.php
which happens to fail at the same place but the cause is quite different
--- the fix was in code that didn't even exist in 8.1.

What seems to be happening here is that since 8.1 did not try to track
typmod in UNION trees, the view's column is declared as having type
varchar with typmod -1, and so the upper-level Var for the column has
that too.  But because 8.1 strips out "unnecessary" SubqueryScan nodes,
that Var ends up just one level above the outputs of the MergeJoins,
which show the correct typmod from the underlying tables.  So that
Assert gets upset.  (You don't see the bug in 8.0 because it didn't
have that optimization.)

I'm tempted to suggest that we just remove the Assert on vartypmod in
the 8.1 branch.  The Assert on vartype is doing as much as is really
important to check, and I don't want to disable the trivial_subqueryscan
optimization, which seems the only other low-risk fix.

regards, tom lane

---(end of broadcast)---
TIP 4: Have you searched our list archives?

   http://archives.postgresql.org


Re: [HACKERS] tripping an assert in 8.1.6 (more info)

2007-01-23 Thread Brian Hurt

Tom Lane wrote:


Brian Hurt <[EMAIL PROTECTED]> writes:
 


Execute, on a fresh database, the following sql, to recreate the bug:
   



Hmm, I thought that stack trace looked a bit familiar --- we seem to
have fixed the problem as of 8.2.  Unfortunately I can't recall what
the relevant change was exactly; time for some digging in the CVS logs.





Any hope of getting the fix back-ported into the 8.1 tree?

Brian



Re: [HACKERS] tripping an assert in 8.1.6 (more info)

2007-01-23 Thread Tom Lane
Brian Hurt <[EMAIL PROTECTED]> writes:
> Execute, on a fresh database, the following sql, to recreate the bug:

Hmm, I thought that stack trace looked a bit familiar --- we seem to
have fixed the problem as of 8.2.  Unfortunately I can't recall what
the relevant change was exactly; time for some digging in the CVS logs.

regards, tom lane

---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

   http://www.postgresql.org/docs/faq


Re: [HACKERS] tripping an assert in 8.1.6 (more info)

2007-01-23 Thread Brian Hurt
I forgot to mention: core dumps available upon request (obviously I 
don't want to post them to the list).


Brian


---(end of broadcast)---
TIP 3: Have you checked our extensive FAQ?

  http://www.postgresql.org/docs/faq


Re: [HACKERS] tripping an assert in 8.1.6 (more info)

2007-01-23 Thread Brian Hurt

More info on that assert I've hit.

Compile 8.1.6 with configuration options:

./configure --with-perl --enable-debug --enable-cassert

(not sure if --perl is relevent or not, I think not).

This is on Fedora Core 5 on x86-32.

Execute, on a fresh database, the following sql, to recreate the bug:

CREATE TABLE foo
(
some_data VARCHAR(32)
,row_date DATE
);

CREATE TABLE bar
(
some_data VARCHAR(32)
,row_date DATE
);

CREATE TABLE quux
(
some_data VARCHAR(32)
,more_data VARCHAR(32)
);


CREATE OR REPLACE VIEW bazz AS
SELECT
   ('bar: ' || bar.row_date) :: TEXT AS action,
   quux.more_data AS more_data,
   bar.row_date AS row_date
FROM
   bar
   JOIN quux
   ON bar.some_data = quux.some_data
UNION ALL
SELECT
   ('foo: ' || foo.row_date) :: TEXT AS action,
   quux.more_data AS more_data,
   foo.row_date AS row_date
FROM
   foo
   JOIN quux
   ON foo.some_data = quux.some_data
;

SELECT action, more_data FROM bazz;



---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
  choose an index scan if your joining column's datatypes do not
  match