Thread: Recent updates
(I'm back on-line, I think...)
I committed several changes to the development source tree this morning
(~15 hours ago). The changes affect the following areas:
1) There is an 8-byte integer data type. It needs some testing and
possibly configuration help on most of the supported platforms. Works on
i686/Linux and should work on Alpha.
2) pg_dump now surrounds table and column names with double-quotes, to
preserve case and funny characters through a dump/reload operation. Hope
this is OK with you Bruce; let me know... btw, last time I tested this
code (two weeks ago?) it was still slightly off of a perfect dump/reload
of the regression tests. The test I am doing is to dump the regression
test database, then reload that into a new database, then dump the new
database. The resulting pg_dump output should be the same as the dump of
the original database.
3) some docs sources have been updated.
4) some additional regression tests have been defined to cover the
HAVING clause and the int8 data type.
5) automatic data type conversion now happens in every place it needs
to, I think. The last changes are to get source columns to match target
columns in simple INSERT/FROM statements.
Except for the "random" and "resjunk" regression tests, things look good
on my development machine; I've done a build and regression test
directly from the CVS source tree after these changes with no other
errors noted.
- Tom
> (I'm back on-line, I think...) > I committed several changes to the development source tree this morning > (~15 hours ago). The changes affect the following areas: > > 1) There is an 8-byte integer data type. It needs some testing and > possibly configuration help on most of the supported platforms. Works on > i686/Linux and should work on Alpha. Good. > > 2) pg_dump now surrounds table and column names with double-quotes, to > preserve case and funny characters through a dump/reload operation. Hope > this is OK with you Bruce; let me know... btw, last time I tested this > code (two weeks ago?) it was still slightly off of a perfect dump/reload > of the regression tests. The test I am doing is to dump the regression > test database, then reload that into a new database, then dump the new > database. The resulting pg_dump output should be the same as the dump of > the original database. Yep, that's the ticket. > > 3) some docs sources have been updated. > > 4) some additional regression tests have been defined to cover the > HAVING clause and the int8 data type. Stephan has fixes for the remaining HAVING problems. He has not sent them yet because he is getting problems with psort. > > 5) automatic data type conversion now happens in every place it needs > to, I think. The last changes are to get source columns to match target > columns in simple INSERT/FROM statements. > > Except for the "random" and "resjunk" regression tests, things look good > on my development machine; I've done a build and regression test > directly from the CVS source tree after these changes with no other > errors noted. Good. I assume UNION NULL is still an issue one of us needs to fix. -- Bruce Momjian | 830 Blythe Avenue maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026 + If your life is a hard drive, | (610) 353-9879(w) + Christ can be your backup. | (610) 853-3000(h)
> > 5) automatic data type conversion now happens in every place it
> > needs to, I think.
> Good. I assume UNION NULL is still an issue one of us needs to fix.
Not anymore :)
postgres=> select text 'hi' union select NULL;
?column?
--------
hi
(2 rows)
It was a one-liner addition to check for NULL columns and do nothing for
conversions. Will keep the patch here for a couple of days while doing
more testing...
- Tom
> Good. I assume UNION NULL is still an issue one of us needs to fix.
OK, I just committed changes to parse_clause.c which fix the most
obvious "UNION SELECT NULL" problems:
postgres=> select 1 union select null;
?column?
--------
1
(2 rows)
Other permutations work too. The code also gets the types right if there
are multiple UNION clauses (remember the type of the result should be
the same as the type of the first clause in the UNION):
postgres=> select 1.1 as "float" union select NULL
postgres-> union select 2 union select 3.3;
float
-----
1.1 <--- (this value determined the types)
2 <--- (this was an int after a null)
3.3 <--- (this double came after an int)
<--- (null is here)
(4 rows)
In testing I found at least one remaining case with problems. It
involves a UNION ALL in clauses other than the first:
postgres=> select 1 union select 2 union all select null;
Backend message type 0x44 arrived while idle
pqReadData() -- backend closed the channel unexpectedly.
This probably means the backend terminated abnormally before or while
processing the request.
We have lost the connection to the backend, so further processing is
impossible. Terminating.
At the same time, the backend printed the following:
Too Large Allocation Request("!(0 < (size)
&& (size) <= (0xfffffff)):size=-2 [0xfffffffe]",
File: "mcxt.c", Line: 228)
!(0 < (size) && (size) <= (0xfffffff)) (0)
Do you want to look at this Bruce? I haven't looked at it yet, but think
it might be deeper into the backend than the parser (haven't run into
mcxt.c before).
I am testing on a Friday's version of the cvs tree.
- Tom
> At the same time, the backend printed the following:
>
> Too Large Allocation Request("!(0 < (size)
> && (size) <= (0xfffffff)):size=-2 [0xfffffffe]",
> File: "mcxt.c", Line: 228)
> !(0 < (size) && (size) <= (0xfffffff)) (0)
>
> Do you want to look at this Bruce? I haven't looked at it yet, but think
> it might be deeper into the backend than the parser (haven't run into
> mcxt.c before).
>
> I am testing on a Friday's version of the cvs tree.
Typically a bad malloc. I can check.
--
Bruce Momjian | 830 Blythe Avenue
maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026
+ If your life is a hard drive, | (610) 353-9879(w)
+ Christ can be your backup. | (610) 853-3000(h)
> Do you want to look at this Bruce? I haven't looked at it yet, but think
> it might be deeper into the backend than the parser (haven't run into
> mcxt.c before).
>
> I am testing on a Friday's version of the cvs tree.
Can you check:
test=> select null union select null;
ERROR: type id lookup of 0 failed
--
Bruce Momjian | 830 Blythe Avenue
maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026
+ If your life is a hard drive, | (610) 353-9879(w)
+ Christ can be your backup. | (610) 853-3000(h)
> Do you want to look at this Bruce? I haven't looked at it yet, but think
> it might be deeper into the backend than the parser (haven't run into
> mcxt.c before).
>
> I am testing on a Friday's version of the cvs tree.
Look at this:
test=> select 4 union select 5 union all select null;
?column?
--------
�
(3 rows)
And the character randomly changes depending on the constant you use.
Strange.
--
Bruce Momjian | 830 Blythe Avenue
maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026
+ If your life is a hard drive, | (610) 353-9879(w)
+ Christ can be your backup. | (610) 853-3000(h)
> Do you want to look at this Bruce? I haven't looked at it yet, but think
> it might be deeper into the backend than the parser (haven't run into
> mcxt.c before).
>
> I am testing on a Friday's version of the cvs tree.
The problem appears to be in the sorting of nulls, which is used by
UNION ALL:
test=> select null order by 1;
ERROR: type id lookup of 0 failed
--
Bruce Momjian | 830 Blythe Avenue
maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026
+ If your life is a hard drive, | (610) 353-9879(w)
+ Christ can be your backup. | (610) 853-3000(h)
> test=> select 4 union select 5 union all select null;
> ?column?
> --------
>
> ¼
>
> (3 rows)
>
> And the character randomly changes depending on the constant you use.
> Strange.
Well, this is just another symptom of a lost or uninitialized memory
area; I get
postgres=> select 4 union select 5 union all select null;
?column?
--------
(3 rows)
with apparently blank or null results.
I'll check on the "NULL UNION NULL" problem. Since there is _absolutely
no context_ to assign a type it's a bit strange...
- Tom
> The problem appears to be in the sorting of nulls, which is used by
> UNION ALL:
> test=> select null order by 1;
> ERROR: type id lookup of 0 failed
Hmm. And I've got trouble with the following when I assigned the type
"UNKNOWNOID" to the null fields:
postgres=> select null union select null;
ERROR: Unable to find an ordering operator '<' for type unknown.
Use an explicit ordering operator or modify the query.
With "UNION ALL" it works, since no sorting needs to happen:
postgres=> select null union all select null;
?column?
--------
(2 rows)
An additional problem is that the UNION parsing is done recursively, so
the routine which does the type matching does not see a list of all the
clauses all at once.
Any ideas?
- Tom
> postgres=> select null union all select null; > ?column? > -------- > > > (2 rows) > > An additional problem is that the UNION parsing is done recursively, so > the routine which does the type matching does not see a list of all the > clauses all at once. > > Any ideas? I knew there was a reason I did not support NULL in union. :-) Just kidding. I never thought of testing it, and a good thing too. :-) -- Bruce Momjian | 830 Blythe Avenue maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026 + If your life is a hard drive, | (610) 353-9879(w) + Christ can be your backup. | (610) 853-3000(h)
> > Any ideas?
> I knew there was a reason I did not support NULL in union. :-)
Yeah, it's sticky. Where in the code does the sorting get set up?
- Tom
> > > Any ideas? > > I knew there was a reason I did not support NULL in union. :-) > > Yeah, it's sticky. Where in the code does the sorting get set up? > > - Tom > See optimizer/prep/prepunion.c::plan_union_queries(). You will see me calling transformSortClause() from there to set up a query using UNION/UNION ALL. I think that is where the problem is happening. Whatever you did in the parser to get these types converted is not in that function. Can you check into it? Should I be doing that in another place. I am unsure, but it looks like the best place for it. I think the major problem is the way I am re-ordering the UNION sort to handle the placement of UNION and UNION ALL. I think I need some more code, or perhaps grab some structure you are already populating in the parser in this place. When I required all the types to be the same, it didn't matter how I re-ordered things. -- Bruce Momjian | 830 Blythe Avenue maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026 + If your life is a hard drive, | (610) 353-9879(w) + Christ can be your backup. | (610) 853-3000(h)
> > Yeah, it's sticky. Where in the code does the sorting get set up?
> See optimizer/prep/prepunion.c::plan_union_queries(). You will see me
> calling transformSortClause() from there to set up a query using
> UNION/UNION ALL. I think that is where the problem is happening.
> Whatever you did in the parser to get these types converted is not in
> that function. Can you check into it? Should I be doing that in
> another place. I am unsure, but it looks like the best place for it.
OK, made a change to transformSortClause() called from
plan_union_queries():
postgres=> select null union select null;
?column?
--------
(1 row)
postgres=> select null union select null union all select null;
?column?
--------
(2 rows)
I decided to use the int4 sorting routines when the type is
"InvalidOid", the type apparently assigned to null constants. The sort
routines probably don't get called anyway since everything is a null,
and if they did the "pass by value" int4 routines are probably safest.
Will continue testing, and need to look into this still:
postgres=> select 1 union select null union all select null;
Backend message type 0x44 arrived while idle
...
- Tom
> I decided to use the int4 sorting routines when the type is > "InvalidOid", the type apparently assigned to null constants. The sort > routines probably don't get called anyway since everything is a null, > and if they did the "pass by value" int4 routines are probably safest. Good. That was my suspicion on how to do it. What does 'select null order by 1;' do now? I have renamed the append struct names just now as part of an EXPLAIN fix. Should not affect you. > > Will continue testing, and need to look into this still: > > postgres=> select 1 union select null union all select null; > Backend message type 0x44 arrived while idle -- Bruce Momjian | 830 Blythe Avenue maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026 + If your life is a hard drive, | (610) 353-9879(w) + Christ can be your backup. | (610) 853-3000(h)
> What does 'select null order by 1;' do now?
postgres=> select null order by 1;
ERROR: type id lookup of 0 failed
Darn. That doesn't touch the UNION code, so will need to look elsewhere
I guess.
> I have renamed the append struct names just now as part of an EXPLAIN
> fix. Should not affect you.
OK.
- Tom
> > What does 'select null order by 1;' do now?
>
> postgres=> select null order by 1;
> ERROR: type id lookup of 0 failed
>
> Darn. That doesn't touch the UNION code, so will need to look elsewhere
> I guess.
>
> > I have renamed the append struct names just now as part of an EXPLAIN
> > fix. Should not affect you.
>
> OK.
>
> - Tom
>
It is from here:
#2 0x9aca9 in typeidType (id=0) at parse_type.c:69
#3 0x99d19 in oper (opname=0x8ce13 "<", ltypeId=0, rtypeId=0,
noWarnings=0 '\000') at parse_oper.c:614
#4 0x95a18 in transformSortClause (pstate=0x129f50, orderlist=0x130650,
sortlist=0x0, targetlist=0x130690, uniqueFlag=0x0) at parse_clause.c:330
#5 0x7daed in transformSelectStmt (pstate=0x129f50, stmt=0x2dfb90)
at analyze.c:802
#6 0x7cb99 in transformStmt (pstate=0x129f50, parseTree=0x2dfb90)
at analyze.c:190
#7 0x7c91c in parse_analyze (pl=0x130670, parentParseState=0x0)
at analyze.c:76
Looks easy to fix. The code is:
/* check for exact match on this operator... */
if (HeapTupleIsValid(tup = oper_exact(opname, ltypeId, rtypeId, NULL, NULL,$
{
}
/* try to find a match on likely candidates... */
else if (HeapTupleIsValid(tup = oper_inexact(opname, ltypeId, rtypeId, NULL$
{
}
else if (!noWarnings)
{
elog(ERROR, "Unable to find binary operator '%s' for types %s and %s",
opname, typeTypeName(typeidType(ltypeId)), typeTypeName(typeidType$
}
It can't find operators for NULL, and is bombing when trying to print
the error message. I think we need to handle this query properly,
because some sql's generated by other programs will auto-order by all
the fields. I think your fix that you did with sort perhaps can be done
here.
But actually, the call is coming from transformSortClause(), so parhaps
you can do a NULL test there and prevent oper() from even being called.
Glad you are back on-line.
--
Bruce Momjian | 830 Blythe Avenue
maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026
+ If your life is a hard drive, | (610) 353-9879(w)
+ Christ can be your backup. | (610) 853-3000(h)
> > > What does 'select null order by 1;' do now?
> you can do a NULL test there and prevent oper() from even being
> called.
postgres=> select null order by 1;
?column?
--------
(1 row)
There are three or four cases in transformSortClause() and I had fixed
only one case for UNION. A second case is now fixed, in the same way; I
assigned INT4OID to the column type for the "won't actually happen"
sort. Didn't want to skip the code entirely, since the backend needs to
_try_ a sort to get the NULLs right. I'm not certain under what
circumstances the other cases are invoked; will try some more testing...
Off to work now :)
- Tom
> > > > What does 'select null order by 1;' do now? > > you can do a NULL test there and prevent oper() from even being > > called. > > postgres=> select null order by 1; > ?column? > -------- > > (1 row) > > There are three or four cases in transformSortClause() and I had fixed > only one case for UNION. A second case is now fixed, in the same way; I > assigned INT4OID to the column type for the "won't actually happen" > sort. Didn't want to skip the code entirely, since the backend needs to > _try_ a sort to get the NULLs right. I'm not certain under what > circumstances the other cases are invoked; will try some more testing... > > Off to work now :) Good. Yes, I agree we need to put something in that place. -- Bruce Momjian | 830 Blythe Avenue maillist@candle.pha.pa.us | Drexel Hill, Pennsylvania 19026 + If your life is a hard drive, | (610) 353-9879(w) + Christ can be your backup. | (610) 853-3000(h)