Re: [PATCHES] [EMAIL PROTECTED]: Re: [BUGS] Problem identifying constraints which should not be inherited]
Hi Alex,
On Sun, Mar 30, 2008 at 7:10 AM, Alex Hunsaker [EMAIL PROTECTED] wrote:
(trimmed cc's)
Find attached inherited_constraint_v2.patch
Changes since v1:
-rebased against latest HEAD
-changed enum { Anum_pg_constraint_... } back into #define
Anum_pg_constraint_...
-remove whitespace damage I added
-fixed regression tests I added to be more robust
-fixed
create table ac (a int constraint check_a check (a 0));
create table bc (a int constraint check_a check (a 0)) inherits (ac);
so it properly works (removed crud I put into
AddRelationRawConstraints and created a proper fix in DefineRelation)
I was taking a look at this patch to add the pg_dump related changes. Just
wanted to give you a heads up as this patch crashes if we run make
installcheck. Seems there is an issue introduced in the CREATE TABLE
REFERENCES code path due to your patch (this is without my pg_dump changes
just to be sure). Looks like some memory overwrite issue. The trace is as
follows:
Core was generated by `postgres: nikhils regression [local] CREATE
TABLE '.
Program terminated with signal 11, Segmentation fault.
#0 0x08378024 in AllocSetCheck (context=0xa060368) at aset.c:1112
1112if (dsize 0 dsize chsize
*chdata_end != 0x7E)
(gdb) bt
#0 0x08378024 in AllocSetCheck (context=0xa060368) at aset.c:1112
#1 0x0837704f in AllocSetDelete (context=0xa060368) at aset.c:487
#2 0x083783c2 in MemoryContextDelete (context=0xa060368) at mcxt.c:196
#3 0x083797fb in PortalDrop (portal=0xa0845bc, isTopCommit=0 '\0') at
portalmem.c:448
#4 0x08281939 in exec_simple_query (
query_string=0xa07e564 CREATE TABLE enumtest_child (parent rainbow
REFERENCES enumtest_parent);) at postgres.c:992
#5 0x082857d4 in PostgresMain (argc=4, argv=0x9ffbe28, username=0x9ffbcc4
nikhils) at postgres.c:3550
#6 0x0824917b in BackendRun (port=0xa003180) at postmaster.c:3204
#7 0x082486a2 in BackendStartup (port=0xa003180) at postmaster.c:2827
#8 0x08245e9c in ServerLoop () at postmaster.c:1271
#9 0x082457fd in PostmasterMain (argc=3, argv=0x9ff9c60) at postmaster.c
:1019
#10 0x081e1c03 in main (argc=3, argv=0x9ff9c60) at main.c:188
Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] Consistent \d commands in psql
Tom Lane [EMAIL PROTECTED] writes: Hmm. Personally, most of my uses of \df are for the purpose of looking for built-in functions, and so this'd be a step backwards for my usage. Likewise for operators. Maybe I'm in the minority or maybe not. The only one of these things for which the argument seems entirely compelling is comments. I do understand the appeal of consistency but sometimes it's not such a great thing. The problem is that there's absolutely no way to do the equivalent of a plain \dt and get a list of just your user functions. That's a real headache and it gets worse as we add more and more system functions too. It might be cute to see if the pattern matches any user functions and if not try again with system functions. So you would still get results if you did \df rtrim for example. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Consistent \d commands in psql
On 31/03/2008, Gregory Stark [EMAIL PROTECTED] wrote: It might be cute to see if the pattern matches any user functions and if not try again with system functions. So you would still get results if you did \df rtrim for example. Nice idea. +1 for this behaviour. Cheers, BJ -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] float4/float8/int64 passed by value with tsearch fixup
Hi, I tried to split the previous patch up to see where the tsearch regression comes from. So, it turned out that: - float4 conversion is risk free (patch #1) - float8 conversion is okay, too, if coupled with time[stamp[tz]] conversion (patch #2) but with int64 timestamps enabled, the next one is also needed: - int64 conversion (patch #3) is mostly okay but it is the one that's causing the tsearch regression I looked at the tsearch code and found only one thing that can be suspicious, namely: typedef uint64 TSQuerySign; TSQuerySign is handled almost everywhere as an allocated, passed-by-reference value. I converted it with the attached patch (#4) so it uses Int64GetDatum() and DatumGetInt64() functions internally and the regression went away. Please, consider applying all four patches. Best regards, Zoltán Böszörményi -- -- Zoltán Böszörményi Cybertec Schönig Schönig GmbH http://www.postgresql.at/ 01-pg84-passedbyval-float4.patch.gz Description: Unix tar archive 02-pg84-passedbyval-float8.patch.gz Description: Unix tar archive 03-pg84-passedbyval-int64.patch.gz Description: Unix tar archive 04-pg84-passedbyval-tsearch.patch.gz Description: Unix tar archive -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Consistent \d commands in psql
Gregory Stark [EMAIL PROTECTED] writes: It might be cute to see if the pattern matches any user functions and if not try again with system functions. So you would still get results if you did \df rtrim for example. Interesting idea. IIUC, \df would give you either all user functions *or* all system functions depending on the actual catalog contents, while \dfS would always give you just system functions. That means there'd be no way to replicate the all-functions-of-both-types behavior that has been the default in every prior release. That sounds like a recipe for getting complaints --- changing the default behavior is one thing, but making it so that that behavior isn't available at all is surely going to break somebody's code or habitual usage. How about \dfS- sys functions only \dfU- user functions only \dfSU - all functions (should allow \dfUS spelling too) \df - behavior proposed by Greg (and similarly for all other \d commands of course). Then anyone who's depending on the old behavior can still get it with a couple more keystrokes. BTW, should we remove the special hack that discriminates against showing I/O functions (or really anything that touches cstring) in \df? ISTM that was mostly there to reduce clutter, and this proposal solves that problem more neatly. I know I've cursed that behavior under my breath more than once, but again maybe my usage isn't typical. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Consistent \d commands in psql
On 01/04/2008, Tom Lane [EMAIL PROTECTED] wrote: ... That means there'd be no way to replicate the all-functions-of-both-types behavior that has been the default in every prior release. \dfS- sys functions only \dfU- user functions only \dfSU - all functions (should allow \dfUS spelling too) \df - behavior proposed by Greg How about \df* rather than (or in addition to) \dfSU \dfUS? -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
[PATCHES] Fwd: WIP: CASE statement for PL/pgSQL
correct queue
Hello
I finished this patch.
Proposal: http://archives.postgresql.org/pgsql-hackers/2008-01/msg00696.php
It's compatible with PL/SQL (Oracle) and SQL/PSM (ANSI).
CASE statements is parsed and transformed to CASE expression and
statements paths. Result of CASE expression is used as index to array
of statements paths. It's fast but I have to once time reparse SQL
queries - it generate about 150 lines code, because I need to get all
parameter's positions. It's one disadvantage. On second hand, this
statement needs only one expression evaluation.
Sample:
CREATE OR REPLACE FUNCTION foo(int)
RETURNS void AS $$
BEGIN
CASE $1
WHEN 1,2,3 THEN
RAISE NOTICE '1,2';
RAISE NOTICE '3';
WHEN 4 THEN
RAISE NOTICE '4';
ELSE
RAISE NOTICE 'other than 1,2,3,4';
END CASE;
RETURN;
END;
$$ LANGUAGE plpgsql;
This statement is transformated to:
three statement paths:
[0]
RAISE NOTICE 'other than 1,2,3,4';
[1]
RAISE NOTICE '1,2';
RAISE NOTICE '3';
[2]
RAISE NOTICE '4';
and case expression
CASE $1
WHEN 1 THEN 1
WHEN 2 THEN 1
WHEN 3 THEN 1
WHEN 4 THEN 2
END;
When result is NULL then it uses 0 path.
Questions:
a) is possible to use SQL scanner? Now, scanner isn't directly used everywhere.
any notes and comments are welcome
Regards
Pavel Stehule
*** ./gram.y.orig 2008-03-28 17:33:45.0 +0100
--- ./gram.y 2008-03-31 13:46:08.0 +0200
***
*** 15,23
*/
#include plpgsql.h
-
#include parser/parser.h
!
static PLpgSQL_expr *read_sql_construct(int until,
int until2,
--- 15,24
*/
#include plpgsql.h
#include parser/parser.h
! #include errno.h
! #include ctype.h
! #include string.h
static PLpgSQL_expr *read_sql_construct(int until,
int until2,
***
*** 46,52
static char *check_label(const char *yytxt);
static void check_labels(const char *start_label,
const char *end_label);
!
%}
%name-prefix=plpgsql_yy
--- 47,54
static char *check_label(const char *yytxt);
static void check_labels(const char *start_label,
const char *end_label);
! static PLpgSQL_stmt *make_case(int lineno, PLpgSQL_expr *case_expr,
! List *when_clause_list, List *else_stmts);
%}
%name-prefix=plpgsql_yy
***
*** 79,84
--- 81,91
char *end_label;
List *stmts;
} loop_body;
+ struct
+ {
+ List *expr_list;
+ PLpgSQL_expr *expr;
+ } when_expr;
List *list;
PLpgSQL_type *dtype;
PLpgSQL_datum *scalar; /* a VAR, RECFIELD, or TRIGARG */
***
*** 95,100
--- 102,108
PLpgSQL_nsitem *nsitem;
PLpgSQL_diag_item *diagitem;
PLpgSQL_stmt_fetch *fetch;
+ PLpgSQL_when_clause *whenclause;
}
%type declhdr decl_sect
***
*** 109,115
%type str decl_stmts decl_stmt
%type expr expr_until_semi expr_until_rightbracket
! %type expr expr_until_then expr_until_loop
%type expr opt_exitcond
%type ival assign_var
--- 117,123
%type str decl_stmts decl_stmt
%type expr expr_until_semi expr_until_rightbracket
! %type expr expr_until_then expr_until_loop opt_expr_until_when
%type expr opt_exitcond
%type ival assign_var
***
*** 128,133
--- 136,145
%type stmt stmt_return stmt_raise stmt_execsql stmt_execsql_insert
%type stmt stmt_dynexecute stmt_for stmt_perform stmt_getdiag
%type stmt stmt_open stmt_fetch stmt_move stmt_close stmt_null
+ %type stmt stmt_case
+ %type when_expr case_when_expr
+ %type whenclause when_clause
+ %type list when_clause_list opt_case_default
%type list proc_exceptions
%type exception_block exception_sect
***
*** 154,159
--- 166,172
%token K_ASSIGN
%token K_BEGIN
%token K_BY
+ %token K_CASE
%token K_CLOSE
%token K_CONSTANT
%token K_CONTINUE
***
*** 611,616
--- 624,631
{ $$ = $1; }
| stmt_if
{ $$ = $1; }
+ | stmt_case
+ { $$ = $1; }
| stmt_loop
{ $$ = $1; }
| stmt_while
***
*** 809,814
--- 824,869
}
;
+ stmt_case : K_CASE lno opt_expr_until_when when_clause_list opt_case_default K_END K_CASE ';'
+ {
+ $$ = make_case($2, $3, $4, $5);
+ }
+ ;
+
+ opt_case_default :
+ {
+ $$ = NIL;
+ }
+ | K_ELSE proc_stmts
+ {
+ $$ = $2;
+ }
+ ;
+
+ when_clause_list : when_clause_list when_clause
+ {
+ $$ = lappend($1, $2);
+ }
+ | when_clause
+ {
+ $$ = list_make1($1);
+ }
+ ;
+
+ when_clause : K_WHEN lno case_when_expr proc_stmts
+ {
+ PLpgSQL_when_clause *new = palloc0(sizeof(PLpgSQL_when_clause));
+
+ new-cmd_type = PLPGSQL_STMT_WHEN_CLAUSE;
+ new-lineno = $2;
+ new-when_expr = $3.expr;
+
Re: [PATCHES] Consistent \d commands in psql
Tom Lane [EMAIL PROTECTED] writes: Gregory Stark [EMAIL PROTECTED] writes: It might be cute to see if the pattern matches any user functions and if not try again with system functions. So you would still get results if you did \df rtrim for example. Interesting idea. IIUC, \df would give you either all user functions *or* all system functions depending on the actual catalog contents, while \dfS would always give you just system functions. That means there'd be no way to replicate the all-functions-of-both-types behavior that has been the default in every prior release. That sounds like a recipe for getting complaints --- changing the default behavior is one thing, but making it so that that behavior isn't available at all is surely going to break somebody's code or habitual usage. Actually on further thought I wonder if it wouldn't be simpler (and perhaps more consistent with \d) to just list *all* matches iff a pattern is provided but list only user functions if *no* pattern is provided. That would effectively be exactly the current behaviour except that you would have to do \dfS to get a list of system functions. And yeah, you wouldn't be able to get a list of all functions whether system or user functions. I suppose you could do \df * One --perhaps nice, perhaps not-- property of this is that if you defined a function named rtrim and then did \df rtrim it would show you _both_ the system and user function and make it easier to see the conflict. Whereas the other behaviour I proposed would hide the system function which might exacerbate the user's confusion. BTW, should we remove the special hack that discriminates against showing I/O functions (or really anything that touches cstring) in \df? ISTM that was mostly there to reduce clutter, and this proposal solves that problem more neatly. I know I've cursed that behavior under my breath more than once, but again maybe my usage isn't typical. . o O Ohh! That's why I can never find them! -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's On-Demand Production Tuning -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Consistent \d commands in psql
Gregory Stark [EMAIL PROTECTED] writes: One --perhaps nice, perhaps not-- property of this is that if you defined a function named rtrim and then did \df rtrim it would show you _both_ the system and user function and make it easier to see the conflict. Whereas the other behaviour I proposed would hide the system function which might exacerbate the user's confusion. Yeah, that is a very good point indeed. Another way we could approach this is \df - all functions \dfS- sys functions only \dfU- user functions only which avoids falling into the trap Greg mentions. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fwd: WIP: CASE statement for PL/pgSQL
Pavel Stehule wrote: correct queue Hello I finished this patch. Proposal: http://archives.postgresql.org/pgsql-hackers/2008-01/msg00696.php It's compatible with PL/SQL (Oracle) and SQL/PSM (ANSI). At the very least this patch is missing documentation and regression tests. cheers andrew -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Fwd: WIP: CASE statement for PL/pgSQL
Hello On 31/03/2008, Andrew Dunstan [EMAIL PROTECTED] wrote: Pavel Stehule wrote: correct queue Hello I finished this patch. Proposal: http://archives.postgresql.org/pgsql-hackers/2008-01/msg00696.php It's compatible with PL/SQL (Oracle) and SQL/PSM (ANSI). At the very least this patch is missing documentation and regression tests. yes, I know. Regress tests are not problem. This patch is only WIP and I'll to update this patch after commiting EXECUTE USING patch. But somebody maybe can comment this patch now, and I can save some time later. Pavel cheers andrew -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Ordered Append WIP patch v1
Gregory Stark wrote:
Here's the WIP patch I described on -hackers to implemented ordered append
nodes.
I think it would be better to create completely separate executor node
for this, rather than tack this feature on the regular Append node. The
two don't seem to have much in common. Looking at AppendState, for
example, as_firstplan and as_lastplan are not interesting for the
MergeAppend, and likewise MergeAppend needs a whole bunch of fields not
interesting to regular Append. The fact that the first statement in
ExecAppend is if(!node-as_is_ordered), with zero lines of shared
codes between them, also suggests that it would be a good idea to keep
them separate.
As you point out in the comments, it's quite confusing to have indexes
into the slots array and the heap array. I still can't get my head
around that. Would it help to define a struct along the lines of:
struct {
TupleTableSlot *slot;
PlanState *plan;
};
and store pointers to those in the heap?
1) I still haven't completely figured out what to do with equivalence classes.
My hack of just stuffing all the append subrel vars into there seems to
work fine. I need to understand what's going on to see if there's really a
problem with it or not.
I don't understand that well enough to comment... I presume the FIXME in
the patch is related to this?
4) I haven't handled mark/restore or random access. I think they could be
handled and they'll probably be worth the complexity but I'm not sure.
For Mark/restore, I suppose you would mark/restore all subnodes, and
store/restore the heap. For reversing direction, you would reverse the
heap. Doesn't sound too hard, but it's probably better to make it work
without them at first before adding any bells and whistles...
5) Is it considering too many paths? Are there some which maybe aren't worth
considering? For example, maybe it only makes sense to take best start_cost
paths since if that plan doesn't dominate then the best total_cost plan is
likely to be the sequential scans + unordered append + sort.
I don't understand this paragraph. Are you referring to this comment in
the patch:
/* If we can't find any plans with the right order just add the
* cheapest total plan to both paths, we'll have to sort it
* anyways. Perhaps consider not generating a startup path for such
* pathkeys since it'll be unlikely to be the cheapest startup
* cost. */
Having to sort one or two of the sub-plans might not be to be expensive
at all. For example, having to sort the empty parent relation of a
partitioned table is essentially free. It's probably never cheaper to
use the Merge Append if you need to sort *all* of the children, but if
you can avoid sorting even one of them, it might very well be worthwhile.
6) I haven't looked at setops yet but this is particularly attractive for the
UNION (as opposed to UNION ALL) case.
Yes. And DISTINCT.
7) I copied/adapted a bunch of bits from tuplesort to maintain the heap and do
the scankey comparisons. I could refactor that code back into tuplesort but
it would mean twisting around tuplesort quite a bit. Essentially it would
mean introducing a new type of tuplesort which would start off in
FINAL_MERGE state only it would have to behave differently since we don't
want it prefetching lots of records like FINAL_MERGE does, I don't think.
Doesn't seem worthwhile. The heap management part of the patch is quite
short.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Consistent \d commands in psql
Tom Lane wrote: BTW, should we remove the special hack that discriminates against showing I/O functions (or really anything that touches cstring) in \df? ISTM that was mostly there to reduce clutter, and this proposal solves that problem more neatly. I know I've cursed that behavior under my breath more than once, but again maybe my usage isn't typical. +1 -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Consistent \d commands in psql
Tom Lane [EMAIL PROTECTED] writes: Gregory Stark [EMAIL PROTECTED] writes: One --perhaps nice, perhaps not-- property of this is that if you defined a function named rtrim and then did \df rtrim it would show you _both_ the system and user function and make it easier to see the conflict. Whereas the other behaviour I proposed would hide the system function which might exacerbate the user's confusion. Yeah, that is a very good point indeed. Another way we could approach this is \df - all functions \dfS- sys functions only \dfU- user functions only which avoids falling into the trap Greg mentions. That doesn't satisfy the original source of the annoyance which is that \df spams your terminal with ten screens of system functions with your user functions hidden amongst them. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Ask me about EnterpriseDB's PostGIS support! -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] [EMAIL PROTECTED]: Re: [BUGS] Problem identifying constraints which should not be inherited]
On Mon, Mar 31, 2008 at 2:36 AM, NikhilS [EMAIL PROTECTED] wrote:
Hi Alex,
I was taking a look at this patch to add the pg_dump related changes. Just
wanted to give you a heads up as this patch crashes if we run make
installcheck. Seems there is an issue introduced in the CREATE TABLE
REFERENCES code path due to your patch (this is without my pg_dump changes
just to be sure). Looks like some memory overwrite issue. The trace is as
follows:
Ouch, sorry i did not reply sooner... been out with the flu. Oddly
enough make check and make installcheck worked great on my 64 bit box.
But on my laptop(32 bits) make check lights up like a christmas tree.
Which is why I did not notice the problem. :(
Attached is a patch that fixes the problem... (it was debugging from
an earlier version)
diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index f105d39..7d12156 100644
*** a/src/backend/parser/parse_utilcmd.c
--- /bsrc/backend/parser/parse_utilcmd.c
*** transformColumnDefinition(ParseState *ps
*** 409,417
{
constraint = lfirst(clist);
- constraint-is_local = true;
- constraint-inhcount = 0;
-
/*
* If this column constraint is a FOREIGN KEY constraint, then we fill
* in the current attribute's name and throw it into the list of FK
--- 409,414
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches
Re: [PATCHES] Consistent \d commands in psql
Gregory Stark [EMAIL PROTECTED] writes: One --perhaps nice, perhaps not-- property of this is that if you defined a function named rtrim and then did \df rtrim it would show you _both_ the system and user function and make it easier to see the conflict. Whereas the other behaviour I proposed would hide the system function which might exacerbate the user's confusion. Another way we could approach this is ... That doesn't satisfy the original source of the annoyance which is that \df spams your terminal with ten screens of system functions with your user functions hidden amongst them. Sure, but I think the core objection there is that there is no easy way to see only the user-defined functions. Given your point quoted first above, I'm unconvinced that should be the default behavior. regards, tom lane -- Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-patches
