pg.ddx.io pgsql-patches@postgresql.org mailing list archive
help / color / mirror / Atom feedAuto Partitioning Patch - WIP version 1
59+ messages / 15 participants
[nested] [flat]
* Auto Partitioning Patch - WIP version 1
@ 2007-03-30 06:58 NikhilS <nikkhils@gmail.com>
0 siblings, 4 replies; 59+ messages in thread
From: NikhilS @ 2007-03-30 06:58 UTC (permalink / raw)
To: pgsql-patches
Hi,
Please find attached the WIP version 1 of the auto partitioning patch. There
was discussion on this a while back on -hackers at:
http://archives.postgresql.org/pgsql-hackers/2007-03/msg00375.php
Please note that this patch tries to automate the activities that currently
are carried out manually. It does nothing fancy beyond that for now. There
were a lot of good suggestions, I have noted them down but for now I have
tried to stick to the initial goal of automating existing steps for
providing partitioning.
Things that this patch does:
i) Handle new syntax to provide partitioning:
CREATE TABLE tabname (
...
) PARTITION BY
RANGE(ColId)
| LIST(ColId)
(
PARTITION partition_name CHECK(...),
PARTITION partition_name CHECK(...)
...
);
ii) Create master table.
iii) Create children tables based on the number of partitions specified and
make them inherit from the master table.
The following things are TODOs:
iv) Auto generate rules using the checks mentioned for the partitions, to
handle INSERTs/DELETEs/UPDATEs to navigate them to the appropriate child.
Note that checks specified directly on the master table will get inherited
automatically.
v) Based on the PRIMARY, UNIQUE information specified, pass it on to the
children tables.
vi) [stretch goal] Support HASH partitions
Will try to complete the above mentioned TODOs as soon as is possible.
Comments, feedback appreciated.
Thanks and Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com
Attachments:
auto-partition-v1.0.patch (text/x-patch, 16.8K) download
Index: src/backend/commands/tablecmds.c
===================================================================
RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/commands/tablecmds.c,v
retrieving revision 1.218
diff -c -r1.218 tablecmds.c
*** src/backend/commands/tablecmds.c 19 Mar 2007 23:38:29 -0000 1.218
--- src/backend/commands/tablecmds.c 30 Mar 2007 06:31:37 -0000
***************
*** 6864,6866 ****
--- 6864,6945 ----
}
}
}
+
+ /*
+ * ------------------------------------------------------------------------------
+ * DefinePartitions
+ * Create new partitions. They end up inheriting from the parent
+ * relation.
+ * Once they have been created, rules need to be assigned to the parent to
+ * provide the UPDATEs/INSERTs/DELETEs to percolate down to the children
+ * Callers expect this function to end with CommandCounterIncrement if it
+ * makes any changes.
+ * ------------------------------------------------------------------------------
+ */
+ void
+ DefinePartitions(CreateStmt *stmt)
+ {
+ CreateStmt *childStmt;
+ RangeVar inr;
+ Oid childOid;
+ PartitionAttrs *partAttr;
+
+ if (stmt->partAttr == NULL)
+ return;
+
+ Assert(IsA(stmt->partAttr, PartitionAttrs));
+ partAttr = (PartitionAttrs *)(stmt->partAttr);
+
+ /*
+ * All the partitions will inherit from the parent, set the parent in the
+ * inhRelations structure
+ */
+ inr = *stmt->relation;
+
+
+ /*
+ * Create the children tables. The parser has already made sure that we
+ * have atleast one partition in the list
+ */
+ if (partAttr->partFunc == PART_LIST || partAttr->partFunc == PART_RANGE)
+ {
+ List *partitionList = partAttr->partitions;
+ ListCell *temp_part;
+
+ Assert(list_length(partitionList) > 0);
+ if (list_length(partitionList) > 0)
+ {
+ foreach(temp_part, partitionList)
+ {
+ Partition *temp_partition = lfirst(temp_part);
+ /*
+ * Create a working copy for each child
+ */
+ childStmt = (CreateStmt *)copyObject((void *)stmt);
+
+ /*
+ * Child has to use all columns from the parent, otherwise we will get
+ * unnecessary merging columns notices as part of the
+ * DefineRelation
+ */
+ childStmt->tableElts = NIL;
+
+ childStmt->inhRelations = lappend(NULL, &inr);
+ childStmt->relation->relname = temp_partition->partName->relname;
+ childOid = DefineRelation(childStmt, RELKIND_RELATION);
+ }
+ }
+ }
+ else
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("Invalid PARTITION type specified")));
+
+ /*
+ *TODO: Add logic to create rules on the parent table
+ */
+ /*
+ * Make the changes carried out so far visible
+ */
+ CommandCounterIncrement();
+ }
Index: src/backend/nodes/copyfuncs.c
===================================================================
RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/nodes/copyfuncs.c,v
retrieving revision 1.372
diff -c -r1.372 copyfuncs.c
*** src/backend/nodes/copyfuncs.c 27 Mar 2007 23:21:09 -0000 1.372
--- src/backend/nodes/copyfuncs.c 30 Mar 2007 06:31:37 -0000
***************
*** 2070,2075 ****
--- 2070,2078 ----
COPY_NODE_FIELD(options);
COPY_SCALAR_FIELD(oncommit);
COPY_STRING_FIELD(tablespacename);
+ /*
+ * There is no need to copy partAttr as of now
+ */
return newnode;
}
Index: src/backend/parser/analyze.c
===================================================================
RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/parser/analyze.c,v
retrieving revision 1.362
diff -c -r1.362 analyze.c
*** src/backend/parser/analyze.c 13 Mar 2007 00:33:41 -0000 1.362
--- src/backend/parser/analyze.c 30 Mar 2007 06:31:37 -0000
***************
*** 95,100 ****
--- 95,101 ----
List *alist; /* "after list" of things to do after creating
* the table */
IndexStmt *pkey; /* PRIMARY KEY index, if any */
+ PartitionAttrs *partAttr; /* partitioning related info */
} CreateStmtContext;
typedef struct
***************
*** 136,141 ****
--- 137,143 ----
CreateStmtContext *cxt,
bool skipValidation,
bool isAddConstraint);
+ static void transformPartitionData(ParseState *pstate, CreateStmtContext *cxt);
static void applyColumnNames(List *dst, List *src);
static void getSetColTypes(ParseState *pstate, Node *node,
List **colTypes, List **colTypmods);
***************
*** 860,865 ****
--- 862,868 ----
cxt.alist = NIL;
cxt.pkey = NULL;
cxt.hasoids = interpretOidsOption(stmt->options);
+ cxt.partAttr = (PartitionAttrs *)stmt->partAttr;
/*
* Run through each primary element in the table creation clause. Separate
***************
*** 919,924 ****
--- 922,932 ----
transformFKConstraints(pstate, &cxt, true, false);
/*
+ * Postprocess partition related information
+ */
+ transformPartitionData(pstate, &cxt);
+
+ /*
* Output results.
*/
q = makeNode(Query);
***************
*** 1365,1370 ****
--- 1373,1426 ----
heap_close(relation, NoLock);
}
+ /*
+ * transformPartitionData:
+ * If partitioning is specified, carry out checks on the supplied create stmt
+ */
+ static void
+ transformPartitionData(ParseState *pstate, CreateStmtContext *cxt)
+ {
+ PartitionAttrs *partAttr = (PartitionAttrs *)cxt->partAttr;
+ ListCell *lc;
+
+ if (partAttr == NULL)
+ return;
+
+ /* Foreign key constraints not supported with partitioning */
+ if(cxt->fkconstraints)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("FOREIGN KEY constraints not supported with partitioning")));
+
+ /*
+ * Check if the primary key column is the one on which partitioning
+ * is being done
+ */
+
+ foreach(lc, cxt->ixconstraints)
+ {
+ Constraint *constraint = lfirst(lc);
+ Assert(IsA(constraint, Constraint));
+ if(constraint->contype == CONSTR_PRIMARY ||
+ constraint->contype == CONSTR_UNIQUE)
+ {
+ if (constraint->keys->length > 1)
+ {
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("PRIMARY/UNIQUE KEY with partitioning can specify"
+ " single attribute only")));
+ }
+ if(strcmp(partAttr->colName,
+ strVal(lfirst(list_head(constraint->keys)))) != 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+ errmsg("PRIMARY/UNIQUE KEY not the same as"
+ " the specified partitioning key (%s)", partAttr->colName)));
+ }
+ }
+ }
+
static void
transformIndexConstraints(ParseState *pstate, CreateStmtContext *cxt)
{
Index: src/backend/parser/gram.y
===================================================================
RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/parser/gram.y,v
retrieving revision 2.584
diff -c -r2.584 gram.y
*** src/backend/parser/gram.y 26 Mar 2007 16:58:39 -0000 2.584
--- src/backend/parser/gram.y 30 Mar 2007 06:31:37 -0000
***************
*** 146,151 ****
--- 146,153 ----
InsertStmt *istmt;
VariableSetStmt *vsetstmt;
+
+ PartitionFunction partFunc;
}
%type <node> stmt schema_stmt
***************
*** 355,360 ****
--- 357,365 ----
%type <ival> document_or_content
%type <boolean> xml_whitespace_option
+ %type <list> PartitionList
+ %type <node> PartitioningData OptPartition Partition_elem
+ %type <partFunc> Partitioning_function
/*
* If you make any token changes, update the keyword table in
***************
*** 403,409 ****
KEY
LANCOMPILER LANGUAGE LARGE_P LAST_P LEADING LEAST LEFT LEVEL
! LIKE LIMIT LISTEN LOAD LOCAL LOCALTIME LOCALTIMESTAMP LOCATION
LOCK_P LOGIN_P
MATCH MAXVALUE MINUTE_P MINVALUE MODE MONTH_P MOVE
--- 408,414 ----
KEY
LANCOMPILER LANGUAGE LARGE_P LAST_P LEADING LEAST LEFT LEVEL
! LIKE LIMIT LIST LISTEN LOAD LOCAL LOCALTIME LOCALTIMESTAMP LOCATION
LOCK_P LOGIN_P
MATCH MAXVALUE MINUTE_P MINVALUE MODE MONTH_P MOVE
***************
*** 415,427 ****
OBJECT_P OF OFF OFFSET OIDS OLD ON ONLY OPERATOR OPTION OR
ORDER OUT_P OUTER_P OVERLAPS OVERLAY OWNED OWNER
! PARTIAL PASSWORD PLACING POSITION
PRECISION PRESERVE PREPARE PREPARED PRIMARY
PRIOR PRIVILEGES PROCEDURAL PROCEDURE
QUOTE
! READ REAL REASSIGN RECHECK REFERENCES REINDEX RELATIVE_P RELEASE RENAME
REPEATABLE REPLACE REPLICA RESET RESTART RESTRICT RETURNING RETURNS REVOKE
RIGHT ROLE ROLLBACK ROW ROWS RULE
--- 420,432 ----
OBJECT_P OF OFF OFFSET OIDS OLD ON ONLY OPERATOR OPTION OR
ORDER OUT_P OUTER_P OVERLAPS OVERLAY OWNED OWNER
! PARTIAL PARTITION PARTITIONS PASSWORD PLACING POSITION
PRECISION PRESERVE PREPARE PREPARED PRIMARY
PRIOR PRIVILEGES PROCEDURAL PROCEDURE
QUOTE
! RANGE READ REAL REASSIGN RECHECK REFERENCES REINDEX RELATIVE_P RELEASE RENAME
REPEATABLE REPLACE REPLICA RESET RESTART RESTRICT RETURNING RETURNS REVOKE
RIGHT ROLE ROLLBACK ROW ROWS RULE
***************
*** 1826,1843 ****
*
*****************************************************************************/
! CreateStmt: CREATE OptTemp TABLE qualified_name '(' OptTableElementList ')'
! OptInherit OptWith OnCommitOption OptTableSpace
{
CreateStmt *n = makeNode(CreateStmt);
$4->istemp = $2;
n->relation = $4;
n->tableElts = $6;
! n->inhRelations = $8;
n->constraints = NIL;
! n->options = $9;
! n->oncommit = $10;
! n->tablespacename = $11;
$$ = (Node *)n;
}
| CREATE OptTemp TABLE qualified_name OF qualified_name
--- 1831,1849 ----
*
*****************************************************************************/
! CreateStmt: CREATE OptTemp TABLE qualified_name '(' OptTableElementList ')'
! OptPartition OptInherit OptWith OnCommitOption OptTableSpace
{
CreateStmt *n = makeNode(CreateStmt);
$4->istemp = $2;
n->relation = $4;
n->tableElts = $6;
! n->partAttr = $8;
! n->inhRelations = $9;
n->constraints = NIL;
! n->options = $10;
! n->oncommit = $11;
! n->tablespacename = $12;
$$ = (Node *)n;
}
| CREATE OptTemp TABLE qualified_name OF qualified_name
***************
*** 1855,1864 ****
--- 1861,1915 ----
n->options = $10;
n->oncommit = $11;
n->tablespacename = $12;
+ n->partAttr = NULL;
$$ = (Node *)n;
}
;
+ OptPartition: PARTITION BY PartitioningData { $$ = $3; }
+ | {$$ = NULL;}
+ ;
+
+ PartitioningData: Partitioning_function '(' ColId ')' '(' PartitionList ')'
+ {
+ PartitionAttrs *partData = makeNode(PartitionAttrs);
+ partData->partFunc = $1;
+ partData->numberOfPartitions = 0;
+ partData->colName = $3;
+ partData->partitions = $6;
+ $$ = (Node *)partData;
+ }
+ ;
+
+ Partitioning_function: LIST {$$ = PART_LIST;}
+ | RANGE {$$ = PART_RANGE;}
+ ;
+
+ PartitionList: Partition_elem { $$ = list_make1($1); }
+ | PartitionList ',' Partition_elem { $$ = lappend($1, $3); }
+
+ Partition_elem: PARTITION qualified_name CHECK '(' a_expr ')'
+ {
+
+ Partition *partition = makeNode(Partition);
+ Constraint *constraint = NULL;
+
+ partition->partName = $2;
+
+ /* Make the constraint node */
+ constraint = makeNode(Constraint);
+ constraint->contype = CONSTR_CHECK;
+ constraint->name = NULL;
+ constraint->raw_expr = $5;
+ constraint->cooked_expr = NULL;
+ constraint->keys = NULL;
+ constraint->indexspace = NULL;
+
+ partition->partitionCheck = constraint;
+ $$ = (Node *)partition;
+ }
+ ;
+
/*
* Redundancy here is needed to avoid shift/reduce conflicts,
* since TEMP is not a reserved word. See also OptTempTableName.
***************
*** 9005,9014 ****
--- 9056,9067 ----
| JOIN
| LEFT
| LIKE
+ | LIST
| NATURAL
| NOTNULL
| OUTER_P
| OVERLAPS
+ | RANGE
| RIGHT
| SIMILAR
| VERBOSE
***************
*** 9076,9081 ****
--- 9129,9136 ----
| ONLY
| OR
| ORDER
+ | PARTITION
+ | PARTITIONS
| PLACING
| PRIMARY
| REFERENCES
Index: src/backend/parser/keywords.c
===================================================================
RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/parser/keywords.c,v
retrieving revision 1.185
diff -c -r1.185 keywords.c
*** src/backend/parser/keywords.c 19 Mar 2007 23:38:29 -0000 1.185
--- src/backend/parser/keywords.c 30 Mar 2007 06:31:37 -0000
***************
*** 208,213 ****
--- 208,214 ----
{"level", LEVEL},
{"like", LIKE},
{"limit", LIMIT},
+ {"list", LIST},
{"listen", LISTEN},
{"load", LOAD},
{"local", LOCAL},
***************
*** 266,271 ****
--- 267,274 ----
{"owned", OWNED},
{"owner", OWNER},
{"partial", PARTIAL},
+ {"partition", PARTITION},
+ {"partitions", PARTITIONS},
{"password", PASSWORD},
{"placing", PLACING},
{"position", POSITION},
***************
*** 279,284 ****
--- 282,288 ----
{"procedural", PROCEDURAL},
{"procedure", PROCEDURE},
{"quote", QUOTE},
+ {"range", RANGE},
{"read", READ},
{"real", REAL},
{"reassign", REASSIGN},
Index: src/backend/tcop/utility.c
===================================================================
RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/tcop/utility.c,v
retrieving revision 1.275
diff -c -r1.275 utility.c
*** src/backend/tcop/utility.c 26 Mar 2007 16:58:39 -0000 1.275
--- src/backend/tcop/utility.c 30 Mar 2007 06:31:38 -0000
***************
*** 537,547 ****
relOid = DefineRelation((CreateStmt *) parsetree,
RELKIND_RELATION);
/*
* Let AlterTableCreateToastTable decide if this one needs a
* secondary relation too.
*/
- CommandCounterIncrement();
AlterTableCreateToastTable(relOid);
}
break;
--- 537,552 ----
relOid = DefineRelation((CreateStmt *) parsetree,
RELKIND_RELATION);
+ CommandCounterIncrement();
+ /*
+ * Let DefinePartitions decide if partitions need to be created
+ * for this table
+ */
+ DefinePartitions((CreateStmt *)parsetree);
/*
* Let AlterTableCreateToastTable decide if this one needs a
* secondary relation too.
*/
AlterTableCreateToastTable(relOid);
}
break;
Index: src/include/commands/tablecmds.h
===================================================================
RCS file: /repositories/postgreshome/cvs/pgsql/src/include/commands/tablecmds.h,v
retrieving revision 1.32
diff -c -r1.32 tablecmds.h
*** src/include/commands/tablecmds.h 5 Jan 2007 22:19:54 -0000 1.32
--- src/include/commands/tablecmds.h 30 Mar 2007 06:31:38 -0000
***************
*** 58,61 ****
--- 58,62 ----
SubTransactionId mySubid,
SubTransactionId parentSubid);
+ extern void DefinePartitions(CreateStmt *stmt);
#endif /* TABLECMDS_H */
Index: src/include/nodes/nodes.h
===================================================================
RCS file: /repositories/postgreshome/cvs/pgsql/src/include/nodes/nodes.h,v
retrieving revision 1.197
diff -c -r1.197 nodes.h
*** src/include/nodes/nodes.h 27 Mar 2007 23:21:12 -0000 1.197
--- src/include/nodes/nodes.h 30 Mar 2007 06:31:38 -0000
***************
*** 341,346 ****
--- 341,348 ----
T_LockingClause,
T_RowMarkClause,
T_XmlSerialize,
+ T_PartitionAttrs,
+ T_Partition,
/*
* TAGS FOR RANDOM OTHER STUFF
Index: src/include/nodes/parsenodes.h
===================================================================
RCS file: /repositories/postgreshome/cvs/pgsql/src/include/nodes/parsenodes.h,v
retrieving revision 1.343
diff -c -r1.343 parsenodes.h
*** src/include/nodes/parsenodes.h 19 Mar 2007 23:38:32 -0000 1.343
--- src/include/nodes/parsenodes.h 30 Mar 2007 06:31:38 -0000
***************
*** 1064,1069 ****
--- 1064,1070 ----
List *options; /* options from WITH clause */
OnCommitAction oncommit; /* what do we do at COMMIT? */
char *tablespacename; /* table space to use, or NULL */
+ Node *partAttr; /* partitioning related information */
} CreateStmt;
/* ----------
***************
*** 1115,1120 ****
--- 1116,1145 ----
* constraints; NULL for default */
} Constraint;
+ /*
+ * Enumeration which defines the various possibilites for the partitioning
+ * function.
+ */
+ typedef enum PartitionFunction {
+ PART_UNDEFINED,
+ PART_LIST,
+ PART_RANGE,
+ } PartitionFunction;
+
+ typedef struct Partition {
+ NodeTag type;
+ RangeVar *partName; /* Name of the partition */
+ Constraint *partitionCheck; /* per partition constraint */
+ } Partition;
+
+ typedef struct PartitionAttrs {
+ NodeTag type;
+ int numberOfPartitions; /* Number of partitions */
+ List *partitions; /* The list of partitions */
+ PartitionFunction partFunc; /* type of partition */
+ const char *colName; /* partition column name */
+ } PartitionAttrs;
+
/* ----------
* Definitions for FOREIGN KEY constraints in CreateStmt
*
* Re: Auto Partitioning Patch - WIP version 1
@ 2007-04-03 15:36 NikhilS <nikkhils@gmail.com>
parent: NikhilS <nikkhils@gmail.com>
3 siblings, 0 replies; 59+ messages in thread
From: NikhilS @ 2007-04-03 15:36 UTC (permalink / raw)
To: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Hi,
>
> The following things are TODOs:
>
> iv) Auto generate rules using the checks mentioned for the partitions, to
> handle INSERTs/DELETEs/UPDATEs to navigate them to the appropriate child.
> Note that checks specified directly on the master table will get inherited
> automatically.
Am planning to do the above by using the check constraint specified for each
partition. This constraint's raw_expr field ends up becoming the whereClause
for the rule specific to that partition.
One question is whether we should we allow auto creation of UPDATE rules
given that updates can end up spanning multiple partitions if the column on
which partitioning is specified gets updated?
Also if we decide to auto - add rules for UPDATE, the raw_expr will need to
be modified to refer to "OLD."col, which can be quite a headache. We do not
have parsetree walker/mutator functions as far as I could see in the code.
Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2007-04-26 23:08 Bruce Momjian <bruce@momjian.us>
parent: NikhilS <nikkhils@gmail.com>
3 siblings, 0 replies; 59+ messages in thread
From: Bruce Momjian @ 2007-04-26 23:08 UTC (permalink / raw)
To: NikhilS <nikkhils@gmail.com>; +Cc: pgsql-patches
This has been saved for the 8.4 release:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold
---------------------------------------------------------------------------
NikhilS wrote:
> Hi,
>
> Please find attached the WIP version 1 of the auto partitioning patch. There
> was discussion on this a while back on -hackers at:
> http://archives.postgresql.org/pgsql-hackers/2007-03/msg00375.php
>
> Please note that this patch tries to automate the activities that currently
> are carried out manually. It does nothing fancy beyond that for now. There
> were a lot of good suggestions, I have noted them down but for now I have
> tried to stick to the initial goal of automating existing steps for
> providing partitioning.
>
> Things that this patch does:
>
> i) Handle new syntax to provide partitioning:
>
> CREATE TABLE tabname (
> ...
> ) PARTITION BY
> RANGE(ColId)
> | LIST(ColId)
> (
> PARTITION partition_name CHECK(...),
> PARTITION partition_name CHECK(...)
> ...
> );
>
> ii) Create master table.
> iii) Create children tables based on the number of partitions specified and
> make them inherit from the master table.
>
> The following things are TODOs:
>
> iv) Auto generate rules using the checks mentioned for the partitions, to
> handle INSERTs/DELETEs/UPDATEs to navigate them to the appropriate child.
> Note that checks specified directly on the master table will get inherited
> automatically.
> v) Based on the PRIMARY, UNIQUE information specified, pass it on to the
> children tables.
> vi) [stretch goal] Support HASH partitions
>
> Will try to complete the above mentioned TODOs as soon as is possible.
>
> Comments, feedback appreciated.
>
> Thanks and Regards,
> Nikhils
> --
>
> EnterpriseDB http://www.enterprisedb.com
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://www.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-21 14:00 Simon Riggs <simon@2ndquadrant.com>
parent: NikhilS <nikkhils@gmail.com>
3 siblings, 1 reply; 59+ messages in thread
From: Simon Riggs @ 2008-03-21 14:00 UTC (permalink / raw)
To: NikhilS <nikkhils@gmail.com>; +Cc: pgsql-patches@postgresql.org, Bruce Momjian <bruce@momjian.us>
On Fri, 2007-03-30 at 12:28 +0530, NikhilS wrote:
> Please find attached the WIP version 1 of the auto partitioning patch.
> There was discussion on this a while back on -hackers at:
> http://archives.postgresql.org/pgsql-hackers/2007-03/msg00375.php
>
> Please note that this patch tries to automate the activities that
> currently are carried out manually. It does nothing fancy beyond that
> for now. There were a lot of good suggestions, I have noted them down
> but for now I have tried to stick to the initial goal of automating
> existing steps for providing partitioning.
>
> Things that this patch does:
I think this patch is a reasonable first step and clearly written, but
not yet ready for application to Postgres in this commit fest.
I would say we need:
* Clear explanation of the new syntax, with examples of each permutation
so we can see how that would work. In light of recent discussions on
-hackers we need to take a view on whether we should go with Gavin's
suggested syntax or this syntax.
* There are some additional syntax items I don't understand the need
for. So these need to be explained.
* I would be against using the term PARTITION BY since it is already a
phrase that is part of the SQL Standard. Perhaps PARTITIONED BY?
* We need regression tests for any new command syntax
* No docs - that might be the same thing as the first item
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-21 14:45 NikhilS <nikkhils@gmail.com>
parent: Simon Riggs <simon@2ndquadrant.com>
0 siblings, 2 replies; 59+ messages in thread
From: NikhilS @ 2008-03-21 14:45 UTC (permalink / raw)
To: Simon Riggs <simon@2ndquadrant.com>; +Cc: pgsql-patches@postgresql.org, "Bruce Momjian" <bruce@momjian.us>
Hi Simon,
On Fri, Mar 21, 2008 at 7:30 PM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On Fri, 2007-03-30 at 12:28 +0530, NikhilS wrote:
>
> > Please find attached the WIP version 1 of the auto partitioning patch.
> > There was discussion on this a while back on -hackers at:
> > http://archives.postgresql.org/pgsql-hackers/2007-03/msg00375.php
> >
> > Please note that this patch tries to automate the activities that
> > currently are carried out manually. It does nothing fancy beyond that
> > for now. There were a lot of good suggestions, I have noted them down
> > but for now I have tried to stick to the initial goal of automating
> > existing steps for providing partitioning.
> >
> > Things that this patch does:
>
> I think this patch is a reasonable first step and clearly written, but
> not yet ready for application to Postgres in this commit fest.
>
> I would say we need:
>
> * Clear explanation of the new syntax, with examples of each permutation
> so we can see how that would work. In light of recent discussions on
> -hackers we need to take a view on whether we should go with Gavin's
> suggested syntax or this syntax.
>
> * There are some additional syntax items I don't understand the need
> for. So these need to be explained.
>
> * I would be against using the term PARTITION BY since it is already a
> phrase that is part of the SQL Standard. Perhaps PARTITIONED BY?
>
> * We need regression tests for any new command syntax
>
> * No docs - that might be the same thing as the first item
>
> --
Thanks for taking a look. But if I am not mistaken Gavin and co. are working
on a much exhaustive proposal. In light of that maybe this patch might not
be needed in the first place?
I will wait for discussion and a subsequent collective consensus here,
before deciding the further course of actions.
Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-21 15:18 Bruce Momjian <bruce@momjian.us>
parent: NikhilS <nikkhils@gmail.com>
1 sibling, 3 replies; 59+ messages in thread
From: Bruce Momjian @ 2008-03-21 15:18 UTC (permalink / raw)
To: NikhilS <nikkhils@gmail.com>; +Cc: Simon Riggs <simon@2ndquadrant.com>; pgsql-patches
NikhilS wrote:
> > * Clear explanation of the new syntax, with examples of each permutation
> > so we can see how that would work. In light of recent discussions on
> > -hackers we need to take a view on whether we should go with Gavin's
> > suggested syntax or this syntax.
> >
> > * There are some additional syntax items I don't understand the need
> > for. So these need to be explained.
> >
> > * I would be against using the term PARTITION BY since it is already a
> > phrase that is part of the SQL Standard. Perhaps PARTITIONED BY?
> >
> > * We need regression tests for any new command syntax
> >
> > * No docs - that might be the same thing as the first item
>
> Thanks for taking a look. But if I am not mistaken Gavin and co. are working
> on a much exhaustive proposal. In light of that maybe this patch might not
> be needed in the first place?
>
> I will wait for discussion and a subsequent collective consensus here,
> before deciding the further course of actions.
I think it is unwise to wait on Gavin for a more complex implemention
--- we might end up with nothing for 8.4. As long as your syntax is
compatible with whatever Gavin proposed Gavin can add on to your patch
once it is applied.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-21 15:53 Tom Lane <tgl@sss.pgh.pa.us>
parent: Bruce Momjian <bruce@momjian.us>
2 siblings, 1 reply; 59+ messages in thread
From: Tom Lane @ 2008-03-21 15:53 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: NikhilS <nikkhils@gmail.com>; Simon Riggs <simon@2ndquadrant.com>; pgsql-patches
Bruce Momjian <bruce@momjian.us> writes:
> NikhilS wrote:
>> Thanks for taking a look. But if I am not mistaken Gavin and co. are working
>> on a much exhaustive proposal. In light of that maybe this patch might not
>> be needed in the first place?
>>
>> I will wait for discussion and a subsequent collective consensus here,
>> before deciding the further course of actions.
> I think it is unwise to wait on Gavin for a more complex implemention
> --- we might end up with nothing for 8.4. As long as your syntax is
> compatible with whatever Gavin proposed Gavin can add on to your patch
> once it is applied.
It would be equally unwise to apply a stopgap patch if we're not certain
it will be upward compatible with what we want to do later.
I haven't been through the partitioning threads at all yet, but I think
what we probably want to have when we emerge from commit fest is some
consensus on what the road map is for partitioning.
regards, tom lane
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-21 16:53 Simon Riggs <simon@2ndquadrant.com>
parent: NikhilS <nikkhils@gmail.com>
1 sibling, 0 replies; 59+ messages in thread
From: Simon Riggs @ 2008-03-21 16:53 UTC (permalink / raw)
To: NikhilS <nikkhils@gmail.com>; +Cc: pgsql-patches@postgresql.org, Bruce Momjian <bruce@momjian.us>
On Fri, 2008-03-21 at 20:15 +0530, NikhilS wrote:
> Thanks for taking a look. But if I am not mistaken Gavin and co. are
> working on a much exhaustive proposal. In light of that maybe this
> patch might not be needed in the first place?
We should wait to apply, but not wait to discuss. Somebody must take the
initiative.
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-21 17:01 Simon Riggs <simon@2ndquadrant.com>
parent: Bruce Momjian <bruce@momjian.us>
2 siblings, 0 replies; 59+ messages in thread
From: Simon Riggs @ 2008-03-21 17:01 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: NikhilS <nikkhils@gmail.com>; pgsql-patches
On Fri, 2008-03-21 at 11:18 -0400, Bruce Momjian wrote:
> I think it is unwise to wait on Gavin for a more complex implemention
> --- we might end up with nothing for 8.4. As long as your syntax is
> compatible with whatever Gavin proposed Gavin can add on to your patch
> once it is applied.
The patch as stands does little apart from bring together many DDL
statements into one. Partitioning is much much more than this so there
seems little reason to think we should rush to commit this, especially
before we get some good docs that explain what it does, and why.
--
Simon Riggs
2ndQuadrant http://www.2ndQuadrant.com
PostgreSQL UK 2008 Conference: http://www.postgresql.org.uk
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-22 05:19 NikhilS <nikkhils@gmail.com>
parent: Tom Lane <tgl@sss.pgh.pa.us>
0 siblings, 0 replies; 59+ messages in thread
From: NikhilS @ 2008-03-22 05:19 UTC (permalink / raw)
To: Tom Lane <tgl@sss.pgh.pa.us>; +Cc: Bruce Momjian <bruce@momjian.us>; Simon Riggs <simon@2ndquadrant.com>; pgsql-patches
Hi,
On Fri, Mar 21, 2008 at 9:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Bruce Momjian <bruce@momjian.us> writes:
> > NikhilS wrote:
> >> Thanks for taking a look. But if I am not mistaken Gavin and co. are
> working
> >> on a much exhaustive proposal. In light of that maybe this patch might
> not
> >> be needed in the first place?
> >>
> >> I will wait for discussion and a subsequent collective consensus here,
> >> before deciding the further course of actions.
>
> > I think it is unwise to wait on Gavin for a more complex implemention
> > --- we might end up with nothing for 8.4. As long as your syntax is
> > compatible with whatever Gavin proposed Gavin can add on to your patch
> > once it is applied.
>
> It would be equally unwise to apply a stopgap patch if we're not certain
> it will be upward compatible with what we want to do later.
>
> I haven't been through the partitioning threads at all yet, but I think
> what we probably want to have when we emerge from commit fest is some
> consensus on what the road map is for partitioning.
>
+2
Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-22 05:35 Luke Lonergan <LLonergan@greenplum.com>
0 siblings, 0 replies; 59+ messages in thread
From: Luke Lonergan @ 2008-03-22 05:35 UTC (permalink / raw)
To: nikkhils@gmail.com; tgl@sss.pgh.pa.us; Jeffrey.Cohen <JCohen@greenplum.com>; swm@linuxworld.com.au; +Cc: bruce@momjian.us; simon@2ndquadrant.com; pgsql-patches
Hi all,
I think the intent of the syntax / parser patch from Gavin and Jeff was to get consensus from PG on the syntax prior to proceeding with the next chunk of work.
The next chunk of work is now well underway - with support for "ALTER TABLE" and partitioning, along with fast inserts into the "parent" table. This involves changes to the catalog, so we'll also need to discuss this as part of a submission.
GP is in the middle of merging 8.3 into our product, so it will be a few weeks at least before we can push any more info to the list.
Was there consensus on the syntax? IIRC, there was a cessation of contrary comments on the matter. If so, the parser patch was provided earlier - we could posibly refresh it. The way it works in our dev branch now is that the partition syntax is turned off by default using a GUC, but is fully functional wrt creating rules, etc. This allows for experimentation.
----- Original Message -----
From: pgsql-patches-owner@postgresql.org <pgsql-patches-owner@postgresql.org>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: Bruce Momjian <bruce@momjian.us>; Simon Riggs <simon@2ndquadrant.com>; pgsql-patches@postgresql.org <pgsql-patches@postgresql.org>
Sent: Sat Mar 22 01:19:01 2008
Subject: Re: [PATCHES] Auto Partitioning Patch - WIP version 1
Hi,
On Fri, Mar 21, 2008 at 9:23 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Bruce Momjian <bruce@momjian.us> writes:
> NikhilS wrote:
>> Thanks for taking a look. But if I am not mistaken Gavin and co. are working
>> on a much exhaustive proposal. In light of that maybe this patch might not
>> be needed in the first place?
>>
>> I will wait for discussion and a subsequent collective consensus here,
>> before deciding the further course of actions.
> I think it is unwise to wait on Gavin for a more complex implemention
> --- we might end up with nothing for 8.4. As long as your syntax is
> compatible with whatever Gavin proposed Gavin can add on to your patch
> once it is applied.
It would be equally unwise to apply a stopgap patch if we're not certain
it will be upward compatible with what we want to do later.
I haven't been through the partitioning threads at all yet, but I think
what we probably want to have when we emerge from commit fest is some
consensus on what the road map is for partitioning.
+2
Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-26 00:55 Bruce Momjian <bruce@momjian.us>
parent: NikhilS <nikkhils@gmail.com>
3 siblings, 1 reply; 59+ messages in thread
From: Bruce Momjian @ 2008-03-26 00:55 UTC (permalink / raw)
To: NikhilS <nikkhils@gmail.com>; +Cc: pgsql-patches
This has been saved for the next commit-fest:
http://momjian.postgresql.org/cgi-bin/pgpatches_hold
---------------------------------------------------------------------------
NikhilS wrote:
> Hi,
>
> Please find attached the WIP version 1 of the auto partitioning patch. There
> was discussion on this a while back on -hackers at:
> http://archives.postgresql.org/pgsql-hackers/2007-03/msg00375.php
>
> Please note that this patch tries to automate the activities that currently
> are carried out manually. It does nothing fancy beyond that for now. There
> were a lot of good suggestions, I have noted them down but for now I have
> tried to stick to the initial goal of automating existing steps for
> providing partitioning.
>
> Things that this patch does:
>
> i) Handle new syntax to provide partitioning:
>
> CREATE TABLE tabname (
> ...
> ) PARTITION BY
> RANGE(ColId)
> | LIST(ColId)
> (
> PARTITION partition_name CHECK(...),
> PARTITION partition_name CHECK(...)
> ...
> );
>
> ii) Create master table.
> iii) Create children tables based on the number of partitions specified and
> make them inherit from the master table.
>
> The following things are TODOs:
>
> iv) Auto generate rules using the checks mentioned for the partitions, to
> handle INSERTs/DELETEs/UPDATEs to navigate them to the appropriate child.
> Note that checks specified directly on the master table will get inherited
> automatically.
> v) Based on the PRIMARY, UNIQUE information specified, pass it on to the
> children tables.
> vi) [stretch goal] Support HASH partitions
>
> Will try to complete the above mentioned TODOs as soon as is possible.
>
> Comments, feedback appreciated.
>
> Thanks and Regards,
> Nikhils
> --
>
> EnterpriseDB http://www.enterprisedb.com
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: explain analyze is your friend
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-26 12:31 Alvaro Herrera <alvherre@commandprompt.com>
parent: Bruce Momjian <bruce@momjian.us>
0 siblings, 1 reply; 59+ messages in thread
From: Alvaro Herrera @ 2008-03-26 12:31 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: NikhilS <nikkhils@gmail.com>; pgsql-patches
Bruce Momjian escribió:
>
> This has been saved for the next commit-fest:
I noticed you broke an URL that previously worked: what was
http://momjian.us/mhonarc/patches/d3c4af540703292358s8ed731el7771ab14083aa610@mail.gmail.com.html
is now
http://momjian.us/mhonarc/patches_hold/d3c4af540703292358s8ed731el7771ab14083aa610@mail.gmail.com.ht...
May I suggest that the URLs with Message-Ids are stored outside the
particular patch queue directory? The script I showed you yesterday
could be used to do that.
Also I noticed that by moving it to the hold queue, the comments that
may have existed on the patch queue are now gone :-( I'm not sure if
there were any in this case, but it's better if we're aware of that
fact. I think this could be solved if the "namespace" of the comment
does not contain the patch queue name.
I moved it to the May commitfest on the wiki too.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-26 13:34 Bruce Momjian <bruce@momjian.us>
parent: Alvaro Herrera <alvherre@commandprompt.com>
0 siblings, 1 reply; 59+ messages in thread
From: Bruce Momjian @ 2008-03-26 13:34 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@commandprompt.com>; +Cc: NikhilS <nikkhils@gmail.com>; pgsql-patches
Alvaro Herrera wrote:
> Bruce Momjian escribi?:
> >
> > This has been saved for the next commit-fest:
>
> I noticed you broke an URL that previously worked: what was
> http://momjian.us/mhonarc/patches/d3c4af540703292358s8ed731el7771ab14083aa610@mail.gmail.com.html
> is now
> http://momjian.us/mhonarc/patches_hold/d3c4af540703292358s8ed731el7771ab14083aa610@mail.gmail.com.ht...
>
> May I suggest that the URLs with Message-Ids are stored outside the
> particular patch queue directory? The script I showed you yesterday
> could be used to do that.
My email are added/removed so I am unsure how to do that easily becuase
the lists would share the same directory. We don't have that problem
with the archives.
> Also I noticed that by moving it to the hold queue, the comments that
> may have existed on the patch queue are now gone :-( I'm not sure if
> there were any in this case, but it's better if we're aware of that
> fact. I think this could be solved if the "namespace" of the comment
> does not contain the patch queue name.
I specifically set things up so the comments should move with the email.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-26 13:42 Alvaro Herrera <alvherre@commandprompt.com>
parent: Bruce Momjian <bruce@momjian.us>
0 siblings, 1 reply; 59+ messages in thread
From: Alvaro Herrera @ 2008-03-26 13:42 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: NikhilS <nikkhils@gmail.com>; pgsql-patches
Bruce Momjian escribió:
> Alvaro Herrera wrote:
> > I noticed you broke an URL that previously worked: what was
> > http://momjian.us/mhonarc/patches/d3c4af540703292358s8ed731el7771ab14083aa610@mail.gmail.com.html
> > is now
> > http://momjian.us/mhonarc/patches_hold/d3c4af540703292358s8ed731el7771ab14083aa610@mail.gmail.com.ht...
> >
> > May I suggest that the URLs with Message-Ids are stored outside the
> > particular patch queue directory? The script I showed you yesterday
> > could be used to do that.
>
> My email are added/removed so I am unsure how to do that easily becuase
> the lists would share the same directory. We don't have that problem
> with the archives.
My point is that you should only _add_ Message-Ids, not remove them.
You can move the messages from one queue to the other to your heart's
content, but the Message-Id URL should continue to work with no changes.
> > Also I noticed that by moving it to the hold queue, the comments that
> > may have existed on the patch queue are now gone :-( I'm not sure if
> > there were any in this case, but it's better if we're aware of that
> > fact. I think this could be solved if the "namespace" of the comment
> > does not contain the patch queue name.
>
> I specifically set things up so the comments should move with the email.
Yeah, I noticed that after sending the email -- the js-kit name seems to
be only "msgid-<foo>". I thought the "permalink=" attribute was part of
that, but perhaps not? In case you added that permalink attribute
because of my request the other day, let me clarify that what I was
actually thinking was having something like
<a href="http://momjian.us/msgid/123foobar@mylaptop">permalink</a;
after the name of the poster, so that it would be visible on the index
page and the user didn't have to open the page to get it.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-26 13:48 Bruce Momjian <bruce@momjian.us>
parent: Alvaro Herrera <alvherre@commandprompt.com>
0 siblings, 1 reply; 59+ messages in thread
From: Bruce Momjian @ 2008-03-26 13:48 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@commandprompt.com>; +Cc: NikhilS <nikkhils@gmail.com>; pgsql-patches
Alvaro Herrera wrote:
> Bruce Momjian escribi?:
> > Alvaro Herrera wrote:
>
> > > I noticed you broke an URL that previously worked: what was
> > > http://momjian.us/mhonarc/patches/d3c4af540703292358s8ed731el7771ab14083aa610@mail.gmail.com.html
> > > is now
> > > http://momjian.us/mhonarc/patches_hold/d3c4af540703292358s8ed731el7771ab14083aa610@mail.gmail.com.ht...
> > >
> > > May I suggest that the URLs with Message-Ids are stored outside the
> > > particular patch queue directory? The script I showed you yesterday
> > > could be used to do that.
> >
> > My email are added/removed so I am unsure how to do that easily becuase
> > the lists would share the same directory. We don't have that problem
> > with the archives.
>
> My point is that you should only _add_ Message-Ids, not remove them.
> You can move the messages from one queue to the other to your heart's
> content, but the Message-Id URL should continue to work with no changes.
Well, when an email is applied, it is deleted. How do I update the
message-id for that in an automated manner. Right now a rebuild deleted
all the links and recreates them.
> > > Also I noticed that by moving it to the hold queue, the comments that
> > > may have existed on the patch queue are now gone :-( I'm not sure if
> > > there were any in this case, but it's better if we're aware of that
> > > fact. I think this could be solved if the "namespace" of the comment
> > > does not contain the patch queue name.
> >
> > I specifically set things up so the comments should move with the email.
>
> Yeah, I noticed that after sending the email -- the js-kit name seems to
> be only "msgid-<foo>". I thought the "permalink=" attribute was part of
> that, but perhaps not? In case you added that permalink attribute
> because of my request the other day, let me clarify that what I was
> actually thinking was having something like
>
> <a href="http://momjian.us/msgid/123foobar@mylaptop">permalink</a;
The permalink is for people who get email --- it tells them which
message got the comment. (People were complaining before I fixed that.)
> after the name of the poster, so that it would be visible on the index
> page and the user didn't have to open the page to get it.
We can do that if people want. I used to show the message id on the
thread page but several felt it was too cluttered-looking.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-26 13:54 Alvaro Herrera <alvherre@commandprompt.com>
parent: Bruce Momjian <bruce@momjian.us>
0 siblings, 1 reply; 59+ messages in thread
From: Alvaro Herrera @ 2008-03-26 13:54 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: NikhilS <nikkhils@gmail.com>; pgsql-patches
Bruce Momjian escribió:
> Alvaro Herrera wrote:
> > My point is that you should only _add_ Message-Ids, not remove them.
> > You can move the messages from one queue to the other to your heart's
> > content, but the Message-Id URL should continue to work with no changes.
>
> Well, when an email is applied, it is deleted. How do I update the
> message-id for that in an automated manner. Right now a rebuild deleted
> all the links and recreates them.
Well, remove the part that deletes links, and keep the part that
creates links. That way, links that used to work continue working.
I am assuming you use hard links -- obviously this doesn't work with
symlinks. (My script creates hard links.)
> > after the name of the poster, so that it would be visible on the index
> > page and the user didn't have to open the page to get it.
>
> We can do that if people want. I used to show the message id on the
> thread page but several felt it was too cluttered-looking.
Well, it would have helped me. I suggested "permalink" as text because
it is less clutter than the full Message-Id. Besides, the Message-Id by
itself is useless, whereas a link is useful.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-26 14:05 Bruce Momjian <bruce@momjian.us>
parent: Alvaro Herrera <alvherre@commandprompt.com>
0 siblings, 0 replies; 59+ messages in thread
From: Bruce Momjian @ 2008-03-26 14:05 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@commandprompt.com>; +Cc: NikhilS <nikkhils@gmail.com>; pgsql-patches
Alvaro Herrera wrote:
> Bruce Momjian escribi?:
> > Alvaro Herrera wrote:
>
> > > My point is that you should only _add_ Message-Ids, not remove them.
> > > You can move the messages from one queue to the other to your heart's
> > > content, but the Message-Id URL should continue to work with no changes.
> >
> > Well, when an email is applied, it is deleted. How do I update the
> > message-id for that in an automated manner. Right now a rebuild deleted
> > all the links and recreates them.
>
> Well, remove the part that deletes links, and keep the part that
> creates links. That way, links that used to work continue working.
>
> I am assuming you use hard links -- obviously this doesn't work with
> symlinks. (My script creates hard links.)
I was using symlinks. I try to avoid hardlinks that cross directories
--- not sure why, probably because I can't easily manage them to find
out what something is linked to without playing with inodes. You are
right that if I used hard-links I put things in a separate directory,
and the hard links would still contain the email, though if the email
has been removed, should we still be returning it from a query? I
suppose I could delete items with only one link.
Well, if I move the permanent links to another directory, as you
suggest, I am going to invalidate every link. Maybe we should wait for
this commit fest to end and then I can create a permanent link directory
and use hard links.
Or is everyone OK with invalidating all the permanent links now.
> > > after the name of the poster, so that it would be visible on the index
> > > page and the user didn't have to open the page to get it.
> >
> > We can do that if people want. I used to show the message id on the
> > thread page but several felt it was too cluttered-looking.
>
> Well, it would have helped me. I suggested "permalink" as text because
> it is less clutter than the full Message-Id. Besides, the Message-Id by
> itself is useless, whereas a link is useful.
Are you talking about the permalink in the js-kit comment or the
permalink at the top of each message? What text do you want to be the
permalink? How is that created in an automated manner?
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-26 17:50 Bruce Momjian <bruce@momjian.us>
0 siblings, 1 reply; 59+ messages in thread
From: Bruce Momjian @ 2008-03-26 17:50 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@commandprompt.com>; NikhilS <nikkhils@gmail.com>; pgsql-patches
bruce wrote:
> > I am assuming you use hard links -- obviously this doesn't work with
> > symlinks. (My script creates hard links.)
>
> I was using symlinks. I try to avoid hardlinks that cross directories
> --- not sure why, probably because I can't easily manage them to find
> out what something is linked to without playing with inodes. You are
> right that if I used hard-links I put things in a separate directory,
> and the hard links would still contain the email, though if the email
> has been removed, should we still be returning it from a query? I
> suppose I could delete items with only one link.
>
> Well, if I move the permanent links to another directory, as you
> suggest, I am going to invalidate every link. Maybe we should wait for
> this commit fest to end and then I can create a permanent link directory
> and use hard links.
>
> Or is everyone OK with invalidating all the permanent links now.
OK, I remember now. The problem wasn't symlinks but that mhonarc
generates URLs relative to the current directory. If we have message-id
files outside the directory, links like "Thread Next" will not work. We
could try to change those to absolute with a script. The community
archives are going to have the same problem.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-26 17:57 Alvaro Herrera <alvherre@commandprompt.com>
parent: Bruce Momjian <bruce@momjian.us>
0 siblings, 1 reply; 59+ messages in thread
From: Alvaro Herrera @ 2008-03-26 17:57 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: pgsql-patches
Bruce Momjian escribió:
> OK, I remember now. The problem wasn't symlinks but that mhonarc
> generates URLs relative to the current directory. If we have message-id
> files outside the directory, links like "Thread Next" will not work. We
> could try to change those to absolute with a script. The community
> archives are going to have the same problem.
Ahh, thanks for pointing it out. I think I will use an HTTP redirect.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-26 18:14 Bruce Momjian <bruce@momjian.us>
parent: Alvaro Herrera <alvherre@commandprompt.com>
0 siblings, 2 replies; 59+ messages in thread
From: Bruce Momjian @ 2008-03-26 18:14 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@commandprompt.com>; +Cc: pgsql-patches
Alvaro Herrera wrote:
> Bruce Momjian escribi?:
>
> > OK, I remember now. The problem wasn't symlinks but that mhonarc
> > generates URLs relative to the current directory. If we have message-id
> > files outside the directory, links like "Thread Next" will not work. We
> > could try to change those to absolute with a script. The community
> > archives are going to have the same problem.
>
> Ahh, thanks for pointing it out. I think I will use an HTTP redirect.
How do you know which directory to redirect to?
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-26 18:23 Bruce Momjian <bruce@momjian.us>
parent: Bruce Momjian <bruce@momjian.us>
1 sibling, 0 replies; 59+ messages in thread
From: Bruce Momjian @ 2008-03-26 18:23 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: Alvaro Herrera <alvherre@commandprompt.com>; pgsql-patches
Bruce Momjian wrote:
> Alvaro Herrera wrote:
> > Bruce Momjian escribi?:
> >
> > > OK, I remember now. The problem wasn't symlinks but that mhonarc
> > > generates URLs relative to the current directory. If we have message-id
> > > files outside the directory, links like "Thread Next" will not work. We
> > > could try to change those to absolute with a script. The community
> > > archives are going to have the same problem.
> >
> > Ahh, thanks for pointing it out. I think I will use an HTTP redirect.
>
> How do you know which directory to redirect to?
I can't see how Apache redirects would work, but I suppose you could use
HTML redirect because you know at the time you are creating the message
file which directory to point to, but again the title bar will change
to relative once you do that.
--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://postgres.enterprisedb.com
+ If your life is a hard drive, Christ can be your backup. +
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Auto Partitioning Patch - WIP version 1
@ 2008-03-26 18:30 Alvaro Herrera <alvherre@commandprompt.com>
parent: Bruce Momjian <bruce@momjian.us>
1 sibling, 0 replies; 59+ messages in thread
From: Alvaro Herrera @ 2008-03-26 18:30 UTC (permalink / raw)
To: Bruce Momjian <bruce@momjian.us>; +Cc: pgsql-patches
Bruce Momjian wrote:
> Alvaro Herrera wrote:
> > Ahh, thanks for pointing it out. I think I will use an HTTP redirect.
>
> How do you know which directory to redirect to?
The script gets the directory as a parameter.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
^ permalink raw reply [nested|flat] 59+ messages in thread
* Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-10-22 14:44 Jaime Casanova <jcasanov@systemguards.com.ec>
parent: Bruce Momjian <bruce@momjian.us>
2 siblings, 3 replies; 59+ messages in thread
From: Jaime Casanova @ 2008-10-22 14:44 UTC (permalink / raw)
To: PostgreSQL-development <pgsql-hackers@postgresql.org>
just remembering that -patches is a dead list, so i'm sending this to
-hackers where it will have more visibility...
---------- Forwarded message ----------
From: Jaime Casanova <jcasanov@systemguards.com.ec>
Date: Oct 22, 2008 9:43 AM
Subject: Re: [PATCHES] Auto Partitioning Patch - WIP version 1
To: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
Cc: Bruce Momjian <bruce@momjian.us>, NikhilS <nikkhils@gmail.com>,
Simon Riggs <simon@2ndquadrant.com>, pgsql-patches@postgresql.org
On 10/22/08, Nikhil Sontakke <nikhil.sontakke@enterprisedb.com> wrote:
> Hi,
>
> > >
> > >> Thanks for taking a look. But if I am not mistaken Gavin and co. are
> working
> > >> on a much exhaustive proposal. In light of that maybe this patch might
> not
> > >> be needed in the first place?
> > >>
> > >> I will wait for discussion and a subsequent collective consensus here,
> > >> before deciding the further course of actions.
> > >
> > > I think it is unwise to wait on Gavin for a more complex implemention
> > > --- we might end up with nothing for 8.4. As long as your syntax is
> > > compatible with whatever Gavin proposed Gavin can add on to your patch
> > > once it is applied.
> > >
> >
> > seems like you're a prophet... or i miss something?
> >
>
> :)
>
> Maybe I will try to summarize the functionality of this patch, rebase it
> against latest CVS head and try to get it on the commitfest queue atleast
> for further feedback to keep the ball rolling on auto-partitioning...
>
yeah! i was thinking on doing that but still have no time... and
frankly you're the best man for the job ;)
one thing i was thinking of is to use triggers instead of rules just
as our current docs recommends
http://www.postgresql.org/docs/8.3/static/ddl-partitioning.html
with the benefit that a trigger can check if the child table exists
for the range being inserted and if not it can create it first...
haven't looked at the code in the detail but seems that your patch is
still missing the "create rule" part so we are in time to change
that... no?
--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-10-22 21:49 Emmanuel Cecchet <manu@frogthinker.org>
parent: Jaime Casanova <jcasanov@systemguards.com.ec>
2 siblings, 0 replies; 59+ messages in thread
From: Emmanuel Cecchet @ 2008-10-22 21:49 UTC (permalink / raw)
To: Jaime Casanova <jcasanov@systemguards.com.ec>; +Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Another advantage of triggers over rules is that it would work with COPY
which is probably a desired feature.
Emmanuel
Jaime Casanova wrote:
> just remembering that -patches is a dead list, so i'm sending this to
> -hackers where it will have more visibility...
>
> ---------- Forwarded message ----------
> From: Jaime Casanova <jcasanov@systemguards.com.ec>
> Date: Oct 22, 2008 9:43 AM
> Subject: Re: [PATCHES] Auto Partitioning Patch - WIP version 1
> To: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
> Cc: Bruce Momjian <bruce@momjian.us>, NikhilS <nikkhils@gmail.com>,
> Simon Riggs <simon@2ndquadrant.com>, pgsql-patches@postgresql.org
>
>
> On 10/22/08, Nikhil Sontakke <nikhil.sontakke@enterprisedb.com> wrote:
>
>> Hi,
>>
>>
>>>>> Thanks for taking a look. But if I am not mistaken Gavin and co. are
>>>>>
>> working
>>
>>>>> on a much exhaustive proposal. In light of that maybe this patch might
>>>>>
>> not
>>
>>>>> be needed in the first place?
>>>>>
>>>>> I will wait for discussion and a subsequent collective consensus here,
>>>>> before deciding the further course of actions.
>>>>>
>>>> I think it is unwise to wait on Gavin for a more complex implemention
>>>> --- we might end up with nothing for 8.4. As long as your syntax is
>>>> compatible with whatever Gavin proposed Gavin can add on to your patch
>>>> once it is applied.
>>>>
>>>>
>>> seems like you're a prophet... or i miss something?
>>>
>>>
>> :)
>>
>> Maybe I will try to summarize the functionality of this patch, rebase it
>> against latest CVS head and try to get it on the commitfest queue atleast
>> for further feedback to keep the ball rolling on auto-partitioning...
>>
>>
>
> yeah! i was thinking on doing that but still have no time... and
> frankly you're the best man for the job ;)
>
> one thing i was thinking of is to use triggers instead of rules just
> as our current docs recommends
> http://www.postgresql.org/docs/8.3/static/ddl-partitioning.html
>
> with the benefit that a trigger can check if the child table exists
> for the range being inserted and if not it can create it first...
> haven't looked at the code in the detail but seems that your patch is
> still missing the "create rule" part so we are in time to change
> that... no?
>
--
Emmanuel Cecchet
FTO @ Frog Thinker
Open Source Development & Consulting
--
Web: http://www.frogthinker.org
email: manu@frogthinker.org
Skype: emmanuel_cecchet
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-10-23 12:41 Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
parent: Jaime Casanova <jcasanov@systemguards.com.ec>
2 siblings, 1 reply; 59+ messages in thread
From: Nikhil Sontakke @ 2008-10-23 12:41 UTC (permalink / raw)
To: Jaime Casanova <jcasanov@systemguards.com.ec>; +Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Hi,
On Wed, Oct 22, 2008 at 8:14 PM, Jaime Casanova <
jcasanov@systemguards.com.ec> wrote:
> just remembering that -patches is a dead list, so i'm sending this to
> -hackers where it will have more visibility...
>
> ---------- Forwarded message ----------
> On 10/22/08, Nikhil Sontakke <nikhil.sontakke@enterprisedb.com> wrote:
> > Hi,
> >
> > > >
> > > >> Thanks for taking a look. But if I am not mistaken Gavin and co. are
> > working
> > > >> on a much exhaustive proposal. In light of that maybe this patch
> might
> > not
> > > >> be needed in the first place?
> > > >>
> > > >> I will wait for discussion and a subsequent collective consensus
> here,
> > > >> before deciding the further course of actions.
> > > >
> > > > I think it is unwise to wait on Gavin for a more complex implemention
> > > > --- we might end up with nothing for 8.4. As long as your syntax is
> > > > compatible with whatever Gavin proposed Gavin can add on to your
> patch
> > > > once it is applied.
> > > >
> > >
> > > seems like you're a prophet... or i miss something?
> > >
> >
> > :)
> >
> > Maybe I will try to summarize the functionality of this patch, rebase it
> > against latest CVS head and try to get it on the commitfest queue atleast
> > for further feedback to keep the ball rolling on auto-partitioning...
> >
>
> yeah! i was thinking on doing that but still have no time... and
> frankly you're the best man for the job ;)
>
> one thing i was thinking of is to use triggers instead of rules just
> as our current docs recommends
> http://www.postgresql.org/docs/8.3/static/ddl-partitioning.html
>
> with the benefit that a trigger can check if the child table exists
> for the range being inserted and if not it can create it first...
> haven't looked at the code in the detail but seems that your patch is
> still missing the "create rule" part so we are in time to change
> that... no?
>
Yes triggers should be used instead of rules. Automatic generation of
rules/triggers would be kind of hard and needs some looking into. Also there
are issues like checking mutual exclusivity of the partition clauses
specified too (I have been maintaining that the onus of ensuring sane
partition ranges/clauses should rest with the users atleast initially..).
I will take a stab at this again whenever I get some free cycles.
Regards,
Nikhils
--
http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-10-31 14:50 Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
parent: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
0 siblings, 1 reply; 59+ messages in thread
From: Nikhil Sontakke @ 2008-10-31 14:50 UTC (permalink / raw)
To: Jaime Casanova <jcasanov@systemguards.com.ec>; +Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Hi,
> > > >
>> > > >> Thanks for taking a look. But if I am not mistaken Gavin and co.
>> are
>> > working
>> > > >> on a much exhaustive proposal. In light of that maybe this patch
>> might
>> > not
>> > > >> be needed in the first place?
>> > > >>
>> > > >> I will wait for discussion and a subsequent collective consensus
>> here,
>> > > >> before deciding the further course of actions.
>> > > >
>> > > > I think it is unwise to wait on Gavin for a more complex
>> implemention
>> > > > --- we might end up with nothing for 8.4. As long as your syntax
>> is
>> > > > compatible with whatever Gavin proposed Gavin can add on to your
>> patch
>> > > > once it is applied.
>> > > >
>> > >
>> > > seems like you're a prophet... or i miss something?
>> > >
>> >
>> > :)
>> >
>> > Maybe I will try to summarize the functionality of this patch, rebase it
>> > against latest CVS head and try to get it on the commitfest queue
>> atleast
>> > for further feedback to keep the ball rolling on auto-partitioning...
>> >
>>
>> yeah! i was thinking on doing that but still have no time... and
>> frankly you're the best man for the job ;)
>>
>> one thing i was thinking of is to use triggers instead of rules just
>> as our current docs recommends
>> http://www.postgresql.org/docs/8.3/static/ddl-partitioning.html
>>
>> with the benefit that a trigger can check if the child table exists
>> for the range being inserted and if not it can create it first...
>> haven't looked at the code in the detail but seems that your patch is
>> still missing the "create rule" part so we are in time to change
>> that... no?
>>
>
> Yes triggers should be used instead of rules. Automatic generation of
> rules/triggers would be kind of hard and needs some looking into. Also there
> are issues like checking mutual exclusivity of the partition clauses
> specified too (I have been maintaining that the onus of ensuring sane
> partition ranges/clauses should rest with the users atleast initially..).
>
> I will take a stab at this again whenever I get some free cycles.
>
I have synced up and modified the patch against latest CVS sources. Am
attaching the latest WIP patch here.
Am restating that its a WIP patch, more so because we really need feedback
on this before trying to expend any energy trying to come up with a
commit-able patch.
As per me, the syntax introduced by this patch should be similar to what was
proposed by Gavin quite a while back and this patch essentially tries to
bring together a bunch of ddl that would otherwise have been performed
step-by-step in a manual fashion earlier. To summarize this patch provides a
one-shot mechanism to:
-- * create master table
-- * create several child tables that inherit from this master table
-- * add appropriate constraints to each of the child tables
-- * create a trigger function to redirect insert, updates, deletes to
-- appropriate child tables (plpgsql language)
-- * create the trigger using the trigger function
I have created a new file (src/test/regress/sql/partition.sql) to show a
couple of examples of the grammar and the working functionality:
There are TODOs like:
-- logic to ensure unique trigger function and trigger names
-- The trigger function body could raise an exception if the
insert/update/delete operation does not fit into any single partition
-- logic to check mutual exclusivity of ranges/lists
-- misc. issues to convert it from wip to commit-ready
If we think this is ok as a first step towards auto-partitioning then we can
do something more with this patch.
Regards,
Nikhils
--
http://www.enterprisedb.com
Attachments:
partitioning-nov-commitfest-wip-v1.0.patch.tar.gz (application/x-gzip, 8.6K) download
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-10-31 18:42 Emmanuel Cecchet <manu@frogthinker.org>
parent: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
0 siblings, 1 reply; 59+ messages in thread
From: Emmanuel Cecchet @ 2008-10-31 18:42 UTC (permalink / raw)
To: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; +Cc: Jaime Casanova <jcasanov@systemguards.com.ec>; PostgreSQL-development <pgsql-hackers@postgresql.org>
Hi Nikhil,
Here are a couple of questions:
- How do you ALTER the table to repartition it?
- The trigger function for inserts could be improved by using ELSE
instead of independent IFs. This would ensure that the row is inserted
in at most 1 partition. The last ELSE should raise an exception if there
was no match (that would solve point 2 of your TODO list).
- Another option is to have a separate trigger per child table and chain
them to the master table. For example something like:
CREATE OR REPLACE FUNCTION child_insert_trigger()
RETURNS TRIGGER AS $$
BEGIN
IF (NEW.date >= DATE(TG_ARGV[1]) AND NEW.date < DATE(TG_ARGV[2]) ) THEN
INSERT INTO TG_ARGV[0] VALUES (NEW.*);
RETURN NULL;
END IF;
RETURN NEW;
END;
$$
LANGUAGE plpgsql;
DROP TRIGGER IF EXISTS insert_child_trigger ON master;
CREATE TRIGGER insert_child_trigger_y2008m01
BEFORE INSERT ON master
FOR EACH ROW EXECUTE PROCEDURE
child_insert_trigger_date(child_y2008m01, '2008-01-01', '2008-02-01');
CREATE TRIGGER insert_child_trigger_y2008m02
BEFORE INSERT ON master
FOR EACH ROW EXECUTE PROCEDURE
child_insert_trigger_date(child_y2008m02, '2008-02-01', '2008-03-01');
CREATE TRIGGER insert_child_trigger_y2008m03
BEFORE INSERT ON master
FOR EACH ROW EXECUTE PROCEDURE
child_insert_trigger_date(child_y2008m03, '2008-03-01', '2008-04-01');
This might make it easier when you want to alter a specific partition
rather than rewriting the whole trigger. Performance-wise, I am not sure
how chained triggers will compare to the big if/then/else trigger.
- In the case of an insert, could it be possible to avoid the cost of a
new INSERT statement (parser, planner, executor, etc...) by moving
directly the tuple in the right table like the COPY code does? If we had
an INSERT trigger code in C, given a HeapTuple and a target Relation we
should be able to call heap_insert_tuple directly, with no parsing,
planning, etc. required.
Thanks for your time,
Emmanuel
> Hi,
>
>
> > > >
> > > >> Thanks for taking a look. But if I am not mistaken
> Gavin and co. are
> > working
> > > >> on a much exhaustive proposal. In light of that maybe
> this patch might
> > not
> > > >> be needed in the first place?
> > > >>
> > > >> I will wait for discussion and a subsequent collective
> consensus here,
> > > >> before deciding the further course of actions.
> > > >
> > > > I think it is unwise to wait on Gavin for a more complex
> implemention
> > > > --- we might end up with nothing for 8.4. As long as
> your syntax is
> > > > compatible with whatever Gavin proposed Gavin can add on
> to your patch
> > > > once it is applied.
> > > >
> > >
> > > seems like you're a prophet... or i miss something?
> > >
> >
> > :)
> >
> > Maybe I will try to summarize the functionality of this
> patch, rebase it
> > against latest CVS head and try to get it on the commitfest
> queue atleast
> > for further feedback to keep the ball rolling on
> auto-partitioning...
> >
>
> yeah! i was thinking on doing that but still have no time... and
> frankly you're the best man for the job ;)
>
> one thing i was thinking of is to use triggers instead of
> rules just
> as our current docs recommends
> http://www.postgresql.org/docs/8.3/static/ddl-partitioning.html
>
> with the benefit that a trigger can check if the child table
> exists
> for the range being inserted and if not it can create it first...
> haven't looked at the code in the detail but seems that your
> patch is
> still missing the "create rule" part so we are in time to change
> that... no?
>
>
> Yes triggers should be used instead of rules. Automatic generation
> of rules/triggers would be kind of hard and needs some looking
> into. Also there are issues like checking mutual exclusivity of
> the partition clauses specified too (I have been maintaining that
> the onus of ensuring sane partition ranges/clauses should rest
> with the users atleast initially..).
>
> I will take a stab at this again whenever I get some free cycles.
>
>
> I have synced up and modified the patch against latest CVS sources. Am
> attaching the latest WIP patch here.
>
> Am restating that its a WIP patch, more so because we really need
> feedback on this before trying to expend any energy trying to come up
> with a commit-able patch.
>
> As per me, the syntax introduced by this patch should be similar to
> what was proposed by Gavin quite a while back and this patch
> essentially tries to bring together a bunch of ddl that would
> otherwise have been performed step-by-step in a manual fashion
> earlier. To summarize this patch provides a one-shot mechanism to:
>
> -- * create master table
> -- * create several child tables that inherit from this master table
> -- * add appropriate constraints to each of the child tables
> -- * create a trigger function to redirect insert, updates, deletes to
> -- appropriate child tables (plpgsql language)
> -- * create the trigger using the trigger function
>
> I have created a new file (src/test/regress/sql/partition.sql) to show
> a couple of examples of the grammar and the working functionality:
>
> There are TODOs like:
> -- logic to ensure unique trigger function and trigger names
> -- The trigger function body could raise an exception if the
> insert/update/delete operation does not fit into any single partition
> -- logic to check mutual exclusivity of ranges/lists
> -- misc. issues to convert it from wip to commit-ready
>
> If we think this is ok as a first step towards auto-partitioning then
> we can do something more with this patch.
>
> Regards,
> Nikhils
> --
> http://www.enterprisedb.com
> ------------------------------------------------------------------------
>
>
>
--
Emmanuel Cecchet
FTO @ Frog Thinker
Open Source Development & Consulting
--
Web: http://www.frogthinker.org
email: manu@frogthinker.org
Skype: emmanuel_cecchet
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-06 04:47 Jaime Casanova <jcasanov@systemguards.com.ec>
parent: Emmanuel Cecchet <manu@frogthinker.org>
0 siblings, 1 reply; 59+ messages in thread
From: Jaime Casanova @ 2008-11-06 04:47 UTC (permalink / raw)
To: Emmanuel Cecchet <manu@frogthinker.org>; +Cc: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>
On Fri, Oct 31, 2008 at 7:42 PM, Emmanuel Cecchet <manu@frogthinker.org> wrote:
> Hi Nikhil,
>
i'm looking at this one:
http://archives.postgresql.org/message-id/a301bfd90810310750pf108c69x36499546f406650f@mail.gmail.com
> Here are a couple of questions:
> - How do you ALTER the table to repartition it?
fair question. but the patch was advertized to only automate some
tasks that we do manually... so keeping the same limitations seems
reasonably to me...
> - Another option is to have a separate trigger per child table and chain
> them to the master table. For example something like:
that sounds like a lot of overhead...
---------------
Now, about the patch...
- seems strange the need to create plpgsql language before we can
create any partitioned table but given that the trigger is a plpgsql
function (and a c function can't be used because we could need to add
new partitions) it seems necesary... ideas?
- the update part of the trigger looks very simplistic... if the new
values isn't in the range accepted by the partition it errors out
because of the check constraint... can't we be a little smarter,
delete from the actual partition and insert in the new one...
for the rest, the patch passes all regression tests and seems to work
as advertized....
--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-26 21:35 Jaime Casanova <jcasanov@systemguards.com.ec>
parent: Jaime Casanova <jcasanov@systemguards.com.ec>
0 siblings, 1 reply; 59+ messages in thread
From: Jaime Casanova @ 2008-11-26 21:35 UTC (permalink / raw)
To: Emmanuel Cecchet <manu@frogthinker.org>; +Cc: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>
On Wed, Nov 5, 2008 at 11:47 PM, Jaime Casanova
<jcasanov@systemguards.com.ec> wrote:
> On Fri, Oct 31, 2008 at 7:42 PM, Emmanuel Cecchet <manu@frogthinker.org> wrote:
>> Hi Nikhil,
>>
>
> i'm looking at this one:
> http://archives.postgresql.org/message-id/a301bfd90810310750pf108c69x36499546f406650f@mail.gmail.com
>
'cause the great interest this one has (i'm being ironic, just in case
;) can we safely say this was returned with feedback and remove it
from the list of pending patches?
--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-26 21:52 Robert Haas <robertmhaas@gmail.com>
parent: Jaime Casanova <jcasanov@systemguards.com.ec>
0 siblings, 1 reply; 59+ messages in thread
From: Robert Haas @ 2008-11-26 21:52 UTC (permalink / raw)
To: Jaime Casanova <jcasanov@systemguards.com.ec>; +Cc: Emmanuel Cecchet <manu@frogthinker.org>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>
> 'cause the great interest this one has (i'm being ironic, just in case
> ;) can we safely say this was returned with feedback and remove it
> from the list of pending patches?
Um... are you referring to lack of interest from the patch author, or
from the community?
If the patch author is no longer interested in the patch, of course it
should be withdrawn. But as for the community, the patch is on the
commitfest wiki[1] and you are listed as the reviewer, so I wouldn't
necessarily expect anyone else to comment at this point - although, in
fact, Emmanuel Cecchet wrote in as well, so I would say you have
exactly the opposite of a lack of interest.
If you think the patch needs further review from another reviewer, say
so. I'm sure someone else can be assigned to do an additional review.
If you think the patch is ready to commit, say so, and update the wiki
accordingly.
...Robert
[1] http://wiki.postgresql.org/wiki/CommitFest_2008-11
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-27 00:13 Jaime Casanova <jcasanov@systemguards.com.ec>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 59+ messages in thread
From: Jaime Casanova @ 2008-11-27 00:13 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Emmanuel Cecchet <manu@frogthinker.org>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>
On Wed, Nov 26, 2008 at 10:52 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> 'cause the great interest this one has (i'm being ironic, just in case
>> ;) can we safely say this was returned with feedback and remove it
>> from the list of pending patches?
>
> the patch is on the
> commitfest wiki[1] and you are listed as the reviewer, so I wouldn't
> necessarily expect anyone else to comment at this point - although, in
> fact, Emmanuel Cecchet wrote in as well, so I would say you have
> exactly the opposite of a lack of interest.
>
i review it on nov 6, and there were open questions by me and by
Emmanuel none of those has been answered:
http://archives.postgresql.org/pgsql-hackers/2008-11/msg00362.php
--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-27 04:09 Robert Haas <robertmhaas@gmail.com>
parent: Jaime Casanova <jcasanov@systemguards.com.ec>
0 siblings, 1 reply; 59+ messages in thread
From: Robert Haas @ 2008-11-27 04:09 UTC (permalink / raw)
To: Jaime Casanova <jcasanov@systemguards.com.ec>; +Cc: Emmanuel Cecchet <manu@frogthinker.org>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>
> i review it on nov 6, and there were open questions by me and by
> Emmanuel none of those has been answered:
> http://archives.postgresql.org/pgsql-hackers/2008-11/msg00362.php
Hmm, there's only one actual question in that email, which is a
request for ideas about PL/pgsql vs. C. I suspect you didn't get any
responses because the rest of the email seems to indicate that the
patch is not very mature at this point: for example, being able to
handle updates that move rows between partitions would seem to me to
be an essential feature for a project of this type, even though there
are many practical scenarios were it's unimportant. Likewise, being
able to repartition sounds important.
With respect to the specific question about PL/pgsql vs C, I suspect
it's very unlikely that any patch of this type that relies on PL/pgsql
being loaded would be accepted into core. However, it's possible that
a useful contrib module or pgfoundry project could be spawned on that
basis, and that might be a good place to start.
I think having a useful toolkit, or a core language feature, that
supports table partitioning would be awesome and would find very broad
application... but it sounds like there is quite a bit of work left
to be done to get there.
...Robert
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-27 09:06 Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 59+ messages in thread
From: Nikhil Sontakke @ 2008-11-27 09:06 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Jaime Casanova <jcasanov@systemguards.com.ec>; Emmanuel Cecchet <manu@frogthinker.org>; PostgreSQL-development <pgsql-hackers@postgresql.org>
Hi,
> i review it on nov 6, and there were open questions by me and by
> > Emmanuel none of those has been answered:
> > http://archives.postgresql.org/pgsql-hackers/2008-11/msg00362.php
>
> Hmm, there's only one actual question in that email, which is a
> request for ideas about PL/pgsql vs. C. I suspect you didn't get any
> responses because the rest of the email seems to indicate that the
> patch is not very mature at this point: for example, being able to
> handle updates that move rows between partitions would seem to me to
> be an essential feature for a project of this type, even though there
> are many practical scenarios were it's unimportant. Likewise, being
> able to repartition sounds important.
>
> With respect to the specific question about PL/pgsql vs C, I suspect
> it's very unlikely that any patch of this type that relies on PL/pgsql
> being loaded would be accepted into core. However, it's possible that
> a useful contrib module or pgfoundry project could be spawned on that
> basis, and that might be a good place to start.
>
> I think having a useful toolkit, or a core language feature, that
> supports table partitioning would be awesome and would find very broad
> application... but it sounds like there is quite a bit of work left
> to be done to get there.
>
This patch does introduce some basic syntax to help create partitions.
The status has always being WIP, because what has not happened is that we
have not had consensus on whether this is a logical first baby step ahead
with partitioning. I haven't seen core members commenting on whether trying
to aggregate the current set of manual operations together via this approach
is worth spending further efforts, to get it into commitable shape.
To summarize, the community should decide if this is indeed the first step
ahead.
Regards,
Nikhils
--
http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-27 12:04 Alvaro Herrera <alvherre@commandprompt.com>
parent: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
0 siblings, 2 replies; 59+ messages in thread
From: Alvaro Herrera @ 2008-11-27 12:04 UTC (permalink / raw)
To: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; +Cc: Robert Haas <robertmhaas@gmail.com>; Jaime Casanova <jcasanov@systemguards.com.ec>; Emmanuel Cecchet <manu@frogthinker.org>; PostgreSQL-development <pgsql-hackers@postgresql.org>
Nikhil Sontakke escribió:
> The status has always being WIP, because what has not happened is that we
> have not had consensus on whether this is a logical first baby step ahead
> with partitioning. I haven't seen core members commenting on whether trying
> to aggregate the current set of manual operations together via this approach
> is worth spending further efforts, to get it into commitable shape.
There was a lenghty, interesting discussion about this topic in the
developer meeting in Ottawa.
http://wiki.postgresql.org/wiki/PgCon_2008_Developer_Meeting#Partitioning_Roadmap
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-27 13:07 Robert Haas <robertmhaas@gmail.com>
parent: Alvaro Herrera <alvherre@commandprompt.com>
1 sibling, 2 replies; 59+ messages in thread
From: Robert Haas @ 2008-11-27 13:07 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@commandprompt.com>; +Cc: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; Jaime Casanova <jcasanov@systemguards.com.ec>; Emmanuel Cecchet <manu@frogthinker.org>; PostgreSQL-development <pgsql-hackers@postgresql.org>
On Thu, Nov 27, 2008 at 7:04 AM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:
> Nikhil Sontakke escribió:
>
>> The status has always being WIP, because what has not happened is that we
>> have not had consensus on whether this is a logical first baby step ahead
>> with partitioning. I haven't seen core members commenting on whether trying
>> to aggregate the current set of manual operations together via this approach
>> is worth spending further efforts, to get it into commitable shape.
>
> There was a lenghty, interesting discussion about this topic in the
> developer meeting in Ottawa.
> http://wiki.postgresql.org/wiki/PgCon_2008_Developer_Meeting#Partitioning_Roadmap
Interesting - too bad there aren't some mode detailed notes.
The semantics of PARTITION ON (<expr>) are unclear to me. I was
thinking maybe it would make sense to do something like:
CREATE PARTITION <name> ON <table> WHERE <expr>
Then you could:
CREATE PARTITION transaction_2008_11 ON transaction WHERE record_date
BETWEEN '2008-11-01' AND '2008-11-30';
I like the idea of using table inheritance as a foundation for this
feature, but I think it's not going to be very useful for real-world
applications without cross-table indexes. Suppose for example that I
have five years worth of data (thus, 60 partitions) and each
transaction has a unique identifier of some sort that is unrelated to
the date. It's bad enough that a query like this has to check every
partition:
SELECT * FROM transaction WHERE uuid = ?
What's even worse (at least IMHO) is that there's no way to use
transaction (uuid) as a reference for a foreign key.
...Robert
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-27 13:31 Gregory Stark <stark@enterprisedb.com>
parent: Robert Haas <robertmhaas@gmail.com>
1 sibling, 1 reply; 59+ messages in thread
From: Gregory Stark @ 2008-11-27 13:31 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Alvaro Herrera <alvherre@commandprompt.com>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; Jaime Casanova <jcasanov@systemguards.com.ec>; Emmanuel Cecchet <manu@frogthinker.org>; PostgreSQL-development <pgsql-hackers@postgresql.org>
"Robert Haas" <robertmhaas@gmail.com> writes:
> CREATE PARTITION transaction_2008_11 ON transaction WHERE record_date
> BETWEEN '2008-11-01' AND '2008-11-30';
I think the main advantage to a better partitioning method would be teaching
Postgres about the partition key. Instead of a collection of different
constraints Postgres would know that "record_date" is *always* the partition
key. So it wouldn't have to be specified every time you declare a partition.
> I like the idea of using table inheritance as a foundation for this
> feature, but I think it's not going to be very useful for real-world
> applications without cross-table indexes.
Well we could add support for cross-table indexes. It's not hard from the
point of low level implementation -- just include the table oid in the index
pointers. Figuring out how to represent such a thing at the index description
point of view would be quite tricky though.
*But*... in practice I would suggest that cross-table indexes are actually
very rarely useful. Having them defeats much of the advantage of partitioning
in the first place. Suddenly you would not be able to instantly drop and load
whole partitions. They're a big check-list item that people want to have
before they partition in case they need them but then they find out that the
down-sides of actually using them makes them quite useless.
Postgres's current architecture actually has a big advantage over more
methodical partitioning methods in this case. You can always add additional
constraints on other columns even if they aren't the "real" partitioning key.
So for example if you partition the invoice table by month once you close the
books for a previous month you can add a constraint WHERE invoice_id < 'xxx'.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training!
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-27 14:01 Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
parent: Alvaro Herrera <alvherre@commandprompt.com>
1 sibling, 0 replies; 59+ messages in thread
From: Nikhil Sontakke @ 2008-11-27 14:01 UTC (permalink / raw)
To: Alvaro Herrera <alvherre@commandprompt.com>; +Cc: Robert Haas <robertmhaas@gmail.com>; Jaime Casanova <jcasanov@systemguards.com.ec>; Emmanuel Cecchet <manu@frogthinker.org>; PostgreSQL-development <pgsql-hackers@postgresql.org>
Hi,
>
> > The status has always being WIP, because what has not happened is that we
> > have not had consensus on whether this is a logical first baby step ahead
> > with partitioning. I haven't seen core members commenting on whether
> trying
> > to aggregate the current set of manual operations together via this
> approach
> > is worth spending further efforts, to get it into commitable shape.
>
> There was a lenghty, interesting discussion about this topic in the
> developer meeting in Ottawa.
>
> http://wiki.postgresql.org/wiki/PgCon_2008_Developer_Meeting#Partitioning_Roadmap
>
The discussion is indeed interesting. But again the notes do not indicate
any broad consensus on the roadmap :).
The current inheritance based mechanism has its pros-cons and there seem to
be a multitude of requests/expectations around partitioning from different
quarters. Even basic consensus about the syntax is missing. What we need is
a step-by-step approach (starting with fixing up the syntax - if it can be
done like that) and working our way downwards towards the underlying
representation/planning for partitions...
Regards,
Nikhils
--
http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-27 14:01 Jaime Casanova <jcasanov@systemguards.com.ec>
parent: Robert Haas <robertmhaas@gmail.com>
1 sibling, 1 reply; 59+ messages in thread
From: Jaime Casanova @ 2008-11-27 14:01 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Alvaro Herrera <alvherre@commandprompt.com>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; Emmanuel Cecchet <manu@frogthinker.org>; PostgreSQL-development <pgsql-hackers@postgresql.org>
On Thu, Nov 27, 2008 at 8:07 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> The semantics of PARTITION ON (<expr>) are unclear to me. I was
> thinking maybe it would make sense to do something like:
>
> CREATE PARTITION <name> ON <table> WHERE <expr>
>
At first look seems nice but s Gregory said the ideal would be to
identify the key partition.
>
> I like the idea of using table inheritance as a foundation for this
> feature, but I think it's not going to be very useful for real-world
> applications without cross-table indexes. Suppose for example that I
> have five years worth of data (thus, 60 partitions) and each
> transaction has a unique identifier of some sort that is unrelated to
> the date. It's bad enough that a query like this has to check every
> partition:
>
you haven't. the WHERE clause in your hipotetical CREATE PARTITION
should create a check constraint on the child (inherited) table and if
you have constraint_exclusion to on you will check just the
partition(s) that match with the check constraint.
>
> What's even worse (at least IMHO) is that there's no way to use
> transaction (uuid) as a reference for a foreign key.
>
not directly, but you always can create a trigger instead of the
foreign key constraint...
mmm...the docs says that there is no good workaround, what about
mention a trigger?
--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-27 14:41 Robert Haas <robertmhaas@gmail.com>
parent: Gregory Stark <stark@enterprisedb.com>
0 siblings, 1 reply; 59+ messages in thread
From: Robert Haas @ 2008-11-27 14:41 UTC (permalink / raw)
To: Gregory Stark <stark@enterprisedb.com>; +Cc: Alvaro Herrera <alvherre@commandprompt.com>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; Jaime Casanova <jcasanov@systemguards.com.ec>; Emmanuel Cecchet <manu@frogthinker.org>; PostgreSQL-development <pgsql-hackers@postgresql.org>
On Thu, Nov 27, 2008 at 8:31 AM, Gregory Stark <stark@enterprisedb.com> wrote:
>> CREATE PARTITION transaction_2008_11 ON transaction WHERE record_date
>> BETWEEN '2008-11-01' AND '2008-11-30';
>
> I think the main advantage to a better partitioning method would be teaching
> Postgres about the partition key. Instead of a collection of different
> constraints Postgres would know that "record_date" is *always* the partition
> key. So it wouldn't have to be specified every time you declare a partition.
Hmm... I thought the main advantage would be that you wouldn't have
to manually add constraints to all of the child tables, and you
wouldn't have to manually add rules/triggers to the parent table to
redirect DML operations.
What do you see as the advantage of pre-declaring record_date as the
partition key? The major advantage I can think of is that it should
simplify constraint exclusion calculations considerably. Also, you
can easily enforce that partitions are non-overlapping. The
disadvantage is that you can't support more complex partitioning
schemes that can't be expressed in terms of ranges on a single key (an
obvious case is when you want to partition by date AND transaction
type, though that could probably be made to work if you allow
specifying multiple partition keys; less tractable cases are
imaginable).
I guess we could decide we don't care about the more complex
scenarios. Or we could offer:
CREATE TABLE (...) WITH PARTITIONING; -- ad-hoc partitioning
CREATE TABLE (...) WITH PARTITIONING ON (...); -- partition keys must
be non-overlapping slices based only on the given columns
> *But*... in practice I would suggest that cross-table indexes are actually
> very rarely useful. Having them defeats much of the advantage of partitioning
> in the first place. Suddenly you would not be able to instantly drop and load
> whole partitions. They're a big check-list item that people want to have
> before they partition in case they need them but then they find out that the
> down-sides of actually using them makes them quite useless.
That's possible. My every attempt to use inheritance has been stymied
by lack of this feature, but my attempts may not be representative.
In any case, the projects are severable.
> Postgres's current architecture actually has a big advantage over more
> methodical partitioning methods in this case. You can always add additional
> constraints on other columns even if they aren't the "real" partitioning key.
> So for example if you partition the invoice table by month once you close the
> books for a previous month you can add a constraint WHERE invoice_id < 'xxx'.
That's cool.
...Robert
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-27 14:47 Robert Haas <robertmhaas@gmail.com>
parent: Jaime Casanova <jcasanov@systemguards.com.ec>
0 siblings, 0 replies; 59+ messages in thread
From: Robert Haas @ 2008-11-27 14:47 UTC (permalink / raw)
To: Jaime Casanova <jcasanov@systemguards.com.ec>; +Cc: Alvaro Herrera <alvherre@commandprompt.com>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; Emmanuel Cecchet <manu@frogthinker.org>; PostgreSQL-development <pgsql-hackers@postgresql.org>
>> I like the idea of using table inheritance as a foundation for this
>> feature, but I think it's not going to be very useful for real-world
>> applications without cross-table indexes. Suppose for example that I
>> have five years worth of data (thus, 60 partitions) and each
>> transaction has a unique identifier of some sort that is unrelated to
>> the date. It's bad enough that a query like this has to check every
>> partition:
> you haven't. the WHERE clause in your hipotetical CREATE PARTITION
> should create a check constraint on the child (inherited) table and if
> you have constraint_exclusion to on you will check just the
> partition(s) that match with the check constraint.
The problem is that constraint exclusion will not be able to exclude
anything for queries unrelated to the partition key. If my
transactions are identified by UUIDs or similar, there's no way to
predict which table will contain any particular value. You end up
having to scan them all, and even if they all have individual indices
on the column in question, that's still 60 index scans instead of 1.
>> What's even worse (at least IMHO) is that there's no way to use
>> transaction (uuid) as a reference for a foreign key.
> not directly, but you always can create a trigger instead of the
> foreign key constraint...
> mmm...the docs says that there is no good workaround, what about
> mention a trigger?
I think it's pretty hard to make this bulletproof. I think the
triggers that enforce ordinary foreign key constraints contain some
magical cross-checks on transaction commit that can't easily be
emulated by user-written triggers. In any case, it's a long way from
"Oh, yeah, that just works."
...Robert
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-27 15:10 Jaime Casanova <jcasanov@systemguards.com.ec>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 2 replies; 59+ messages in thread
From: Jaime Casanova @ 2008-11-27 15:10 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Gregory Stark <stark@enterprisedb.com>; Alvaro Herrera <alvherre@commandprompt.com>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; Emmanuel Cecchet <manu@frogthinker.org>; PostgreSQL-development <pgsql-hackers@postgresql.org>
On Thu, Nov 27, 2008 at 9:41 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Nov 27, 2008 at 8:31 AM, Gregory Stark <stark@enterprisedb.com> wrote:
>>> CREATE PARTITION transaction_2008_11 ON transaction WHERE record_date
>>> BETWEEN '2008-11-01' AND '2008-11-30';
>>
>> I think the main advantage to a better partitioning method would be teaching
>> Postgres about the partition key. Instead of a collection of different
>> constraints Postgres would know that "record_date" is *always* the partition
>> key. So it wouldn't have to be specified every time you declare a partition.
>
> Hmm... I thought the main advantage would be that you wouldn't have
> to manually add constraints to all of the child tables, and you
> wouldn't have to manually add rules/triggers to the parent table to
> redirect DML operations.
>
ok. what about let CREATE TABLE WITH PARTITIONING to create an entry
in a catalog indicating the key of the partition and install the
triggers and let the trigger decide if it has the partition to insert
the new row (making UPDATE working almost as DELETE+INSERT if it needs
to change of partitions) or create the new partition maybe with an
apropiate CREATE PARTITION...
that way i don't need to create triggers nor inherit tables
manually... and because of that maybe we can make possible to add
<expr> as partition key...
PS: i'm against using CREATE TABLE because we are inventing new syntax
but it seems like using ALTER TABLE is a *lot* of work altough ISTM
more usefull
--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-27 15:12 Jaime Casanova <jcasanov@systemguards.com.ec>
parent: Jaime Casanova <jcasanov@systemguards.com.ec>
1 sibling, 1 reply; 59+ messages in thread
From: Jaime Casanova @ 2008-11-27 15:12 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Gregory Stark <stark@enterprisedb.com>; Alvaro Herrera <alvherre@commandprompt.com>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; Emmanuel Cecchet <manu@frogthinker.org>; PostgreSQL-development <pgsql-hackers@postgresql.org>
On Thu, Nov 27, 2008 at 10:10 AM, Jaime Casanova
<jcasanov@systemguards.com.ec> wrote:
> On Thu, Nov 27, 2008 at 9:41 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Thu, Nov 27, 2008 at 8:31 AM, Gregory Stark <stark@enterprisedb.com> wrote:
>>>> CREATE PARTITION transaction_2008_11 ON transaction WHERE record_date
>>>> BETWEEN '2008-11-01' AND '2008-11-30';
>>>
>>> I think the main advantage to a better partitioning method would be teaching
>>> Postgres about the partition key. Instead of a collection of different
>>> constraints Postgres would know that "record_date" is *always* the partition
>>> key. So it wouldn't have to be specified every time you declare a partition.
>>
>> Hmm... I thought the main advantage would be that you wouldn't have
>> to manually add constraints to all of the child tables, and you
>> wouldn't have to manually add rules/triggers to the parent table to
>> redirect DML operations.
>>
>
> ok. what about let CREATE TABLE WITH PARTITIONING to create an entry
> in a catalog indicating the key of the partition and install the
> triggers and let the trigger decide if it has the partition to insert
> the new row (making UPDATE working almost as DELETE+INSERT if it needs
> to change of partitions) or create the new partition maybe with an
> apropiate CREATE PARTITION...
>
i thik i have to clarify this...
i intend to say that, the trigger will insert or create the partition
and insert...
--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-27 16:09 Emmanuel Cecchet <manu@frogthinker.org>
parent: Jaime Casanova <jcasanov@systemguards.com.ec>
0 siblings, 1 reply; 59+ messages in thread
From: Emmanuel Cecchet @ 2008-11-27 16:09 UTC (permalink / raw)
To: Jaime Casanova <jcasanov@systemguards.com.ec>; +Cc: Robert Haas <robertmhaas@gmail.com>; Gregory Stark <stark@enterprisedb.com>; Alvaro Herrera <alvherre@commandprompt.com>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>
Hi all,
I have been following that discussion very closely but it seems that we
are debating solutions without a good specification of the
problem/requirements.
I would suggest that we collect all the partitioning requirements on a
dedicated Wiki page. There might not be a one size fits it all solution
for all requirements. We can also look at what other databases are
proposing to address these issues.
If we can prioritize features, that should also allow us to stage the
partitioning implementation.
I have a prototype insert trigger in C that directly move inserts in a
master table to the appropriate child table (directly moving the tuple).
Let me know if anyone is interested.
Emmanuel
Jaime Casanova wrote:
> On Thu, Nov 27, 2008 at 10:10 AM, Jaime Casanova
> <jcasanov@systemguards.com.ec> wrote:
>
>> On Thu, Nov 27, 2008 at 9:41 AM, Robert Haas <robertmhaas@gmail.com> wrote:
>>
>>> On Thu, Nov 27, 2008 at 8:31 AM, Gregory Stark <stark@enterprisedb.com> wrote:
>>>
>>>>> CREATE PARTITION transaction_2008_11 ON transaction WHERE record_date
>>>>> BETWEEN '2008-11-01' AND '2008-11-30';
>>>>>
>>>> I think the main advantage to a better partitioning method would be teaching
>>>> Postgres about the partition key. Instead of a collection of different
>>>> constraints Postgres would know that "record_date" is *always* the partition
>>>> key. So it wouldn't have to be specified every time you declare a partition.
>>>>
>>> Hmm... I thought the main advantage would be that you wouldn't have
>>> to manually add constraints to all of the child tables, and you
>>> wouldn't have to manually add rules/triggers to the parent table to
>>> redirect DML operations.
>>>
>>>
>> ok. what about let CREATE TABLE WITH PARTITIONING to create an entry
>> in a catalog indicating the key of the partition and install the
>> triggers and let the trigger decide if it has the partition to insert
>> the new row (making UPDATE working almost as DELETE+INSERT if it needs
>> to change of partitions) or create the new partition maybe with an
>> apropiate CREATE PARTITION...
>>
>>
>
> i thik i have to clarify this...
>
> i intend to say that, the trigger will insert or create the partition
> and insert...
>
>
--
Emmanuel Cecchet
FTO @ Frog Thinker
Open Source Development & Consulting
--
Web: http://www.frogthinker.org
email: manu@frogthinker.org
Skype: emmanuel_cecchet
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-11-27 16:38 Robert Haas <robertmhaas@gmail.com>
parent: Jaime Casanova <jcasanov@systemguards.com.ec>
1 sibling, 0 replies; 59+ messages in thread
From: Robert Haas @ 2008-11-27 16:38 UTC (permalink / raw)
To: Jaime Casanova <jcasanov@systemguards.com.ec>; +Cc: Gregory Stark <stark@enterprisedb.com>; Alvaro Herrera <alvherre@commandprompt.com>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; Emmanuel Cecchet <manu@frogthinker.org>; PostgreSQL-development <pgsql-hackers@postgresql.org>
> ok. what about let CREATE TABLE WITH PARTITIONING to create an entry
> in a catalog indicating the key of the partition and install the
> triggers and let the trigger decide if it has the partition to insert
> the new row (making UPDATE working almost as DELETE+INSERT if it needs
> to change of partitions) or create the new partition maybe with an
> apropiate CREATE PARTITION...
>
> that way i don't need to create triggers nor inherit tables
> manually... and because of that maybe we can make possible to add
> <expr> as partition key...
>
>
> PS: i'm against using CREATE TABLE because we are inventing new syntax
> but it seems like using ALTER TABLE is a *lot* of work altough ISTM
> more usefull
I think that's one of the useful things that could be done in this
area (not the only one, certainly), but I don't think we've defined
the semantics well enough to start talking about exactly which
commands to use. As to CREATE TABLE and ALTER TABLE, I suspect you'll
need both. We have to come to some consensus on whether predefining a
partition key is necessary, optional, or not supported. And we need
to define ways both to set things up and to change them later.
If there is no predefined partition key, there's probably nothing
terribly special that needs to be done to prepare a table for
partitioning. You could decide that all the data will live in the
parent table except for the partitions that are explicitly created.
When the user creates a partition, you create the new child table, set
it to inherit from the parent, add the necessary constraint,
create/update an automatically generated rule/trigger on the parent
that redirects DML to the appropriate partition, and move any EXISTING
tuples that belong in that partition into it. You'd also need
operations to merge a partition back into the parent table (moving the
data back), drop a partition (lose the data), and change the
definition of a partition (move data around).
A significant problem with this design is that you don't know that the
partition constraints are mutually exclusive. What do you do with
data that matches multiple partition constraints? You'll have to
devise some rule, like maybe picking the first partition
alphabetically, which will complicate the rearrangement of data when
partitions are added or removed.
If there IS a predefined partition key, then you'll need a way to tell
the parent table what it is (and a way to remove it later if you
change your mind). Then it should be possible to validate that child
partitions are defined only in terms of that key and that they are
mutually exclusive. You'll still need basically all the same
operations: create partition, modify partition, merge partition back
into parent, drop partition.
...Robert
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-12-02 22:52 Robert Haas <robertmhaas@gmail.com>
parent: Emmanuel Cecchet <manu@frogthinker.org>
0 siblings, 1 reply; 59+ messages in thread
From: Robert Haas @ 2008-12-02 22:52 UTC (permalink / raw)
To: Emmanuel Cecchet <manu@frogthinker.org>; +Cc: Jaime Casanova <jcasanov@systemguards.com.ec>; Gregory Stark <stark@enterprisedb.com>; Alvaro Herrera <alvherre@commandprompt.com>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>
On Thu, Nov 27, 2008 at 11:09 AM, Emmanuel Cecchet <manu@frogthinker.org> wrote:
> I have been following that discussion very closely but it seems that we are
> debating solutions without a good specification of the problem/requirements.
> I would suggest that we collect all the partitioning requirements on a
> dedicated Wiki page. There might not be a one size fits it all solution for
> all requirements. We can also look at what other databases are proposing to
> address these issues.
> If we can prioritize features, that should also allow us to stage the
> partitioning implementation.
This might be a good idea. Want to take a crack at it?
> I have a prototype insert trigger in C that directly move inserts in a
> master table to the appropriate child table (directly moving the tuple). Let
> me know if anyone is interested.
Can't hurt to post it.
...Robert
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-12-10 00:51 Emmanuel Cecchet <manu@frogthinker.org>
parent: Robert Haas <robertmhaas@gmail.com>
0 siblings, 1 reply; 59+ messages in thread
From: Emmanuel Cecchet @ 2008-12-10 00:51 UTC (permalink / raw)
To: Robert Haas <robertmhaas@gmail.com>; +Cc: Jaime Casanova <jcasanov@systemguards.com.ec>; Gregory Stark <stark@enterprisedb.com>; Alvaro Herrera <alvherre@commandprompt.com>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>
Hi all,
While I was trying to find the right place to add a new page on the
wiki, I found the document of Simon on partitioning requirements
(http://wiki.postgresql.org/wiki/Image:Partitioning_Requirements.pdf)
referenced from http://wiki.postgresql.org/wiki/Development_projects
I think this is a good base to start from. Should we convert the doc
into a wiki page or get the source for the doc and go from there?
I attach what I have come up with so far for the C trigger I was talking
about for efficient automatic auto-partitioning of inserts in child tables.
Emmanuel
Robert Haas wrote:
> On Thu, Nov 27, 2008 at 11:09 AM, Emmanuel Cecchet <manu@frogthinker.org> wrote:
>
>> I have been following that discussion very closely but it seems that we are
>> debating solutions without a good specification of the problem/requirements.
>> I would suggest that we collect all the partitioning requirements on a
>> dedicated Wiki page. There might not be a one size fits it all solution for
>> all requirements. We can also look at what other databases are proposing to
>> address these issues.
>> If we can prioritize features, that should also allow us to stage the
>> partitioning implementation.
>>
>
> This might be a good idea. Want to take a crack at it?
>
>
>> I have a prototype insert trigger in C that directly move inserts in a
>> master table to the appropriate child table (directly moving the tuple). Let
>> me know if anyone is interested.
>>
>
> Can't hurt to post it.
>
> ...Robert
>
>
--
Emmanuel Cecchet
FTO @ Frog Thinker
Open Source Development & Consulting
--
Web: http://www.frogthinker.org
email: manu@frogthinker.org
Skype: emmanuel_cecchet
Attachments:
patch-trigger-autopartition-C.txt (text/plain, 10.2K) download
### Eclipse Workspace Patch 1.0
#P Postgres-HEAD
Index: src/test/regress/regress.c
===================================================================
RCS file: /root/cvsrepo/pgsql/src/test/regress/regress.c,v
retrieving revision 1.71
diff -u -r1.71 regress.c
--- src/test/regress/regress.c 25 Mar 2008 22:42:46 -0000 1.71
+++ src/test/regress/regress.c 13 Nov 2008 06:11:08 -0000
@@ -10,6 +10,9 @@
#include "utils/geo_decls.h" /* includes <math.h> */
#include "executor/executor.h" /* For GetAttributeByName */
#include "commands/sequence.h" /* for nextval() */
+#include "catalog/namespace.h"
+#include "executor/executor.h"
+#include "executor/tuptable.h"
#define P_MAXDIG 12
#define LDELIM '('
@@ -732,3 +735,90 @@
*--walk = '\0';
PG_RETURN_CSTRING(result);
}
+
+
+/*
+ * Partition trigger test
+ *
+ * The trigger should be used this way:
+ * CREATE TRIGGER child_table_name
+ BEFORE INSERT ON master_table
+ FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger();
+ */
+
+extern Datum partition_insert_trigger(PG_FUNCTION_ARGS);
+
+PG_FUNCTION_INFO_V1(partition_insert_trigger);
+
+Datum
+partition_insert_trigger(PG_FUNCTION_ARGS)
+{
+ TriggerData *trigdata = (TriggerData *) fcinfo->context;
+ HeapTuple trigtuple= trigdata->tg_trigtuple;
+ char *child_table_name;
+ Relation child_table_relation;
+ Oid relation_id;
+
+ /* make sure it's called as a trigger at all */
+ if (!CALLED_AS_TRIGGER(fcinfo))
+ elog(ERROR, "partition_insert_trigger: not called by trigger manager");
+
+ /* Sanity checks */
+ if (!TRIGGER_FIRED_BY_INSERT(trigdata->tg_event) || !TRIGGER_FIRED_BEFORE(trigdata->tg_event))
+ elog(ERROR, "partition_insert_trigger: not called on insert before");
+
+ // Child table name is either given as the unique parameter or it is the name of the trigger
+ if (trigdata->tg_trigger->tgnargs == 1)
+ child_table_name = trigdata->tg_trigger->tgargs[0];
+ else
+ child_table_name = trigdata->tg_trigger->tgname;
+
+ // Lookup the child relation
+ relation_id = RelnameGetRelid(child_table_name);
+ if (relation_id == InvalidOid)
+ elog(ERROR, "partition_insert_trigger: Invalid child table %s", child_table_name);
+ child_table_relation = RelationIdGetRelation(relation_id);
+ if (child_table_relation == NULL)
+ elog(ERROR, "partition_insert_trigger: Failed to locate relation for child table %s", child_table_name);
+
+ { // Check the constraints
+ TupleConstr *constr = child_table_relation->rd_att->constr;
+
+ if (constr->num_check > 0)
+ {
+ ResultRelInfo *resultRelInfo;
+ TupleTableSlot *slot;
+ EState *estate= CreateExecutorState();
+
+ resultRelInfo = makeNode(ResultRelInfo);
+ resultRelInfo->ri_RangeTableIndex = 1; /* dummy */
+ resultRelInfo->ri_RelationDesc = child_table_relation;
+
+ estate->es_result_relations = resultRelInfo;
+ estate->es_num_result_relations = 1;
+ estate->es_result_relation_info = resultRelInfo;
+
+ /* Set up a tuple slot too */
+ slot = MakeSingleTupleTableSlot(trigdata->tg_relation->rd_att);
+ ExecStoreTuple(trigtuple, slot, InvalidBuffer, false);
+
+ if (ExecRelCheck(resultRelInfo, slot, estate) == NULL)
+ { // Constraints satisfied, insert the row in the child table
+ bool use_wal = true;
+ bool use_fsm=true;
+
+ heap_insert(child_table_relation, trigtuple, GetCurrentCommandId(true), use_wal, use_fsm);
+ RelationClose(child_table_relation);
+ ExecDropSingleTupleTableSlot(slot);
+ return PointerGetDatum(NULL);
+ }
+ ExecDropSingleTupleTableSlot(slot);
+ }
+ else
+ elog(ERROR, "partition_insert_trigger: No constraint found for child table %s", child_table_name);
+ }
+ RelationClose(child_table_relation);
+
+ return PointerGetDatum(trigdata->tg_trigtuple);
+}
+
Index: src/backend/executor/execMain.c
===================================================================
RCS file: /root/cvsrepo/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.314
diff -u -r1.314 execMain.c
--- src/backend/executor/execMain.c 31 Oct 2008 21:07:54 -0000 1.314
+++ src/backend/executor/execMain.c 13 Nov 2008 06:11:08 -0000
@@ -1947,7 +1947,7 @@
/*
* ExecRelCheck --- check that tuple meets constraints for result relation
*/
-static const char *
+const char *
ExecRelCheck(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate)
{
Index: src/test/regress/output/create_function_1.source
===================================================================
RCS file: /root/cvsrepo/pgsql/src/test/regress/output/create_function_1.source,v
retrieving revision 1.34
diff -u -r1.34 create_function_1.source
--- src/test/regress/output/create_function_1.source 31 Oct 2008 19:37:56 -0000 1.34
+++ src/test/regress/output/create_function_1.source 13 Nov 2008 06:11:08 -0000
@@ -47,6 +47,10 @@
RETURNS int4
AS '@libdir@/regress@DLSUFFIX@'
LANGUAGE C STRICT;
+CREATE FUNCTION partition_insert_trigger ()
+ RETURNS trigger
+ AS '@libdir@/regress@DLSUFFIX@'
+ LANGUAGE C STRICT;
-- Things that shouldn't work:
CREATE FUNCTION test1 (int) RETURNS int LANGUAGE SQL
AS 'SELECT ''not an integer'';';
@@ -80,3 +84,70 @@
CREATE FUNCTION test1 (int) RETURNS int LANGUAGE internal
AS 'nosuch';
ERROR: there is no built-in function named "nosuch"
+-- Partitioning trigger test
+CREATE TABLE master (
+ id int not null,
+ date date not null,
+ value int
+);
+CREATE TABLE child_y2008m01 (
+ CHECK ( date >= DATE '2008-01-01' AND date < DATE '2008-02-01' )
+) INHERITS (master);
+CREATE TABLE child_y2008m02 (
+ CHECK ( date >= DATE '2008-02-01' AND date < DATE '2008-03-01' )
+) INHERITS (master);
+CREATE TABLE child_y2008m03 (
+ CHECK ( date >= DATE '2008-03-01' AND date < DATE '2008-04-01' )
+) INHERITS (master);
+CREATE TRIGGER insert_child_y2008m01
+ BEFORE INSERT ON master
+ FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger('child_y2008m01');
+CREATE TRIGGER child_y2008m02
+ BEFORE INSERT ON master
+ FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger();
+CREATE TRIGGER child_y2008m03
+ BEFORE INSERT ON master
+ FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger();
+INSERT INTO master VALUES (1, '2008-01-15', 1);
+INSERT INTO master VALUES (2, '2008-02-15', 2);
+INSERT INTO master VALUES (3, '2008-03-15', 3);
+INSERT INTO master VALUES (4, '2008-04-15', 4);
+COPY master FROM '/root/pg_partitions/copy_input.txt';
+select * from master;
+ id | date | value
+----+------------+-------
+ 4 | 04-15-2008 | 4
+ 1 | 01-15-2008 | 1
+ 11 | 01-10-2008 | 11
+ 2 | 02-15-2008 | 2
+ 12 | 02-15-2008 | 12
+ 3 | 03-15-2008 | 3
+ 13 | 03-15-2008 | 13
+(7 rows)
+
+ select * from child_y2008m01;
+ id | date | value
+----+------------+-------
+ 1 | 01-15-2008 | 1
+ 11 | 01-10-2008 | 11
+(2 rows)
+
+ select * from child_y2008m02;
+ id | date | value
+----+------------+-------
+ 2 | 02-15-2008 | 2
+ 12 | 02-15-2008 | 12
+(2 rows)
+
+ select * from child_y2008m03;
+ id | date | value
+----+------------+-------
+ 3 | 03-15-2008 | 3
+ 13 | 03-15-2008 | 13
+(2 rows)
+
+DROP TABLE master CASCADE;
+NOTICE: drop cascades to 3 other objects
+DETAIL: drop cascades to table child_y2008m01
+drop cascades to table child_y2008m02
+drop cascades to table child_y2008m03
Index: src/include/executor/executor.h
===================================================================
RCS file: /root/cvsrepo/pgsql/src/include/executor/executor.h,v
retrieving revision 1.152
diff -u -r1.152 executor.h
--- src/include/executor/executor.h 31 Oct 2008 21:07:55 -0000 1.152
+++ src/include/executor/executor.h 13 Nov 2008 06:11:08 -0000
@@ -155,6 +155,8 @@
extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
extern void ExecConstraints(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate);
+extern const char *ExecRelCheck(ResultRelInfo *resultRelInfo,
+ TupleTableSlot *slot, EState *estate);
extern TupleTableSlot *EvalPlanQual(EState *estate, Index rti,
ItemPointer tid, TransactionId priorXmax);
extern PlanState *ExecGetActivePlanTree(QueryDesc *queryDesc);
Index: src/test/regress/input/create_function_1.source
===================================================================
RCS file: /root/cvsrepo/pgsql/src/test/regress/input/create_function_1.source,v
retrieving revision 1.19
diff -u -r1.19 create_function_1.source
--- src/test/regress/input/create_function_1.source 1 Oct 2008 22:38:57 -0000 1.19
+++ src/test/regress/input/create_function_1.source 13 Nov 2008 06:11:08 -0000
@@ -52,6 +52,12 @@
AS '@libdir@/regress@DLSUFFIX@'
LANGUAGE C STRICT;
+CREATE FUNCTION partition_insert_trigger ()
+ RETURNS trigger
+ AS '@libdir@/regress@DLSUFFIX@'
+ LANGUAGE C STRICT;
+
+
-- Things that shouldn't work:
CREATE FUNCTION test1 (int) RETURNS int LANGUAGE SQL
@@ -77,3 +83,48 @@
CREATE FUNCTION test1 (int) RETURNS int LANGUAGE internal
AS 'nosuch';
+
+-- Partitioning trigger test
+
+CREATE TABLE master (
+ id int not null,
+ date date not null,
+ value int
+);
+
+CREATE TABLE child_y2008m01 (
+ CHECK ( date >= DATE '2008-01-01' AND date < DATE '2008-02-01' )
+) INHERITS (master);
+
+CREATE TABLE child_y2008m02 (
+ CHECK ( date >= DATE '2008-02-01' AND date < DATE '2008-03-01' )
+) INHERITS (master);
+
+CREATE TABLE child_y2008m03 (
+ CHECK ( date >= DATE '2008-03-01' AND date < DATE '2008-04-01' )
+) INHERITS (master);
+
+CREATE TRIGGER insert_child_y2008m01
+ BEFORE INSERT ON master
+ FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger('child_y2008m01');
+
+CREATE TRIGGER child_y2008m02
+ BEFORE INSERT ON master
+ FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger();
+
+CREATE TRIGGER child_y2008m03
+ BEFORE INSERT ON master
+ FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger();
+
+INSERT INTO master VALUES (1, '2008-01-15', 1);
+INSERT INTO master VALUES (2, '2008-02-15', 2);
+INSERT INTO master VALUES (3, '2008-03-15', 3);
+INSERT INTO master VALUES (4, '2008-04-15', 4);
+COPY master FROM '/root/pg_partitions/copy_input.txt';
+
+select * from master;
+select * from child_y2008m01;
+select * from child_y2008m02;
+select * from child_y2008m03;
+
+DROP TABLE master CASCADE;
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-12-15 17:49 Josh Berkus <josh@agliodbs.com>
parent: Emmanuel Cecchet <manu@frogthinker.org>
0 siblings, 1 reply; 59+ messages in thread
From: Josh Berkus @ 2008-12-15 17:49 UTC (permalink / raw)
To: Emmanuel Cecchet <manu@frogthinker.org>; +Cc: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>
Hackers,
We don't yet seem to have a clear specification for this feature, and
the Other Open Source DB has shown us how problematic it is to get
auto-partitioning wrong.
Should we defer auto-partitioning to 8.5?
--Josh
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-12-15 18:46 Gregory Stark <stark@enterprisedb.com>
parent: Josh Berkus <josh@agliodbs.com>
0 siblings, 1 reply; 59+ messages in thread
From: Gregory Stark @ 2008-12-15 18:46 UTC (permalink / raw)
To: Josh Berkus <josh@agliodbs.com>; +Cc: Emmanuel Cecchet <manu@frogthinker.org>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>
Josh Berkus <josh@agliodbs.com> writes:
> Hackers,
>
> We don't yet seem to have a clear specification for this feature, and the Other
> Open Source DB has shown us how problematic it is to get auto-partitioning
> wrong.
>
> Should we defer auto-partitioning to 8.5?
If we're serious about having a "next generation" partitioning with a concept
of partition keys then it seems to me to make more sense to do that first and
then add on a feature like this.
This is still very useful. I haven't looked at the actual patch, does it
require core changes or can it be stashed in a pgfoundry or contrib module?
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's 24x7 Postgres support!
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-12-15 19:01 Jaime Casanova <jcasanov@systemguards.com.ec>
parent: Gregory Stark <stark@enterprisedb.com>
0 siblings, 1 reply; 59+ messages in thread
From: Jaime Casanova @ 2008-12-15 19:01 UTC (permalink / raw)
To: Gregory Stark <stark@enterprisedb.com>; +Cc: Josh Berkus <josh@agliodbs.com>; Emmanuel Cecchet <manu@frogthinker.org>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>
On Mon, Dec 15, 2008 at 1:46 PM, Gregory Stark <stark@enterprisedb.com> wrote:
> Josh Berkus <josh@agliodbs.com> writes:
>
>> Hackers,
>>
>> We don't yet seem to have a clear specification for this feature, and the Other
>> Open Source DB has shown us how problematic it is to get auto-partitioning
>> wrong.
>>
>> Should we defer auto-partitioning to 8.5?
>
> If we're serious about having a "next generation" partitioning with a concept
> of partition keys then it seems to me to make more sense to do that first and
> then add on a feature like this.
>
+1
> This is still very useful. I haven't looked at the actual patch, does it
> require core changes or can it be stashed in a pgfoundry or contrib module?
>
what i don't like about this one is that it creates partitions at
create table time and to manually add all new partitions (inherit
tables and modify the trigger)... and what i want to see is an
automatic creation when it's needed...
--
Atentamente,
Jaime Casanova
Soporte y capacitación de PostgreSQL
Asesoría y desarrollo de sistemas
Guayaquil - Ecuador
Cel. +59387171157
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-12-16 02:05 Emmanuel Cecchet <manu@frogthinker.org>
parent: Jaime Casanova <jcasanov@systemguards.com.ec>
0 siblings, 2 replies; 59+ messages in thread
From: Emmanuel Cecchet @ 2008-12-16 02:05 UTC (permalink / raw)
To: Jaime Casanova <jcasanov@systemguards.com.ec>; +Cc: Gregory Stark <stark@enterprisedb.com>; Josh Berkus <josh@agliodbs.com>; Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>
Hi all,
I will be working on a roadmap for the partitioning features. I think
that there are different needs and that we will not be able to address
them all in 8.5 or even 8.6.
The goal will be to get things done step by step but possibly with a
design that will not require major refactoring to support new features.
I will try to setup the new wiki page tomorrow.
In the meantime, I have made some more tests with the trigger in C (see
attached patch). To prevent duplicating too much code, it requires the
ExecRelCheck method to be exported (that would be nice to have this
function exported in 8.4 so that we can start experimenting in 8.4 and
don't have to wait another year for 8.5). If there is locality in the
inserts (which might be the case if you COPY sorted data), the
performance remains constant regardless the number of child tables.
My initial tests to insert 140k rows are as follows:
- direct inserts in a child table: 2 seconds
- pgplsql trigger (IF ... ELSE IF ... blocks) : 14.5 seconds.
- C trigger: 4 seconds (actually the overhead is in the constraint check)
Right now if the row cannot be inserted in any child table, it is
inserted in the parent. If you want to fail, we can add a 'fail trigger'
(after all other triggers) that generates an error if previous triggers
did not capture the row. If you want to create a new partition, you can
have another trigger to handle that.
So I think that this trigger approach is pretty flexible like people
used AOP in J2EE servers to process requests. It has also the advantage
of allowing fast prototyping. It should also be easy to push that
functionality down in the core as needed.
Is it ok if I move Simon's requirement document under a more generic
'Table partitioning' page on the Wiki?
Thanks for your feedback,
manu
Jaime Casanova wrote:
> On Mon, Dec 15, 2008 at 1:46 PM, Gregory Stark <stark@enterprisedb.com> wrote:
>
>> Josh Berkus <josh@agliodbs.com> writes:
>>
>>
>>> Hackers,
>>>
>>> We don't yet seem to have a clear specification for this feature, and the Other
>>> Open Source DB has shown us how problematic it is to get auto-partitioning
>>> wrong.
>>>
>>> Should we defer auto-partitioning to 8.5?
>>>
>> If we're serious about having a "next generation" partitioning with a concept
>> of partition keys then it seems to me to make more sense to do that first and
>> then add on a feature like this.
>>
>>
>
> +1
>
>
>> This is still very useful. I haven't looked at the actual patch, does it
>> require core changes or can it be stashed in a pgfoundry or contrib module?
>>
>>
>
> what i don't like about this one is that it creates partitions at
> create table time and to manually add all new partitions (inherit
> tables and modify the trigger)... and what i want to see is an
> automatic creation when it's needed...
>
>
>
--
Emmanuel Cecchet
FTO @ Frog Thinker
Open Source Development & Consulting
--
Web: http://www.frogthinker.org
email: manu@frogthinker.org
Skype: emmanuel_cecchet
Attachments:
patch-trigger-autopartition-C-v2.txt (text/plain, 11.7K) download
### Eclipse Workspace Patch 1.0
#P Postgres-HEAD
Index: src/test/regress/regress.c
===================================================================
RCS file: /root/cvsrepo/pgsql/src/test/regress/regress.c,v
retrieving revision 1.71
diff -u -r1.71 regress.c
--- src/test/regress/regress.c 25 Mar 2008 22:42:46 -0000 1.71
+++ src/test/regress/regress.c 16 Dec 2008 01:42:50 -0000
@@ -10,6 +10,9 @@
#include "utils/geo_decls.h" /* includes <math.h> */
#include "executor/executor.h" /* For GetAttributeByName */
#include "commands/sequence.h" /* for nextval() */
+#include "catalog/namespace.h"
+#include "executor/executor.h"
+#include "executor/tuptable.h"
#define P_MAXDIG 12
#define LDELIM '('
@@ -732,3 +735,141 @@
*--walk = '\0';
PG_RETURN_CSTRING(result);
}
+
+
+/*
+ * Partition trigger test
+ *
+ * The trigger should be used this way:
+ * CREATE TRIGGER trigger_name
+ BEFORE INSERT ON master_table
+ FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger(child_table_name, column_number, min_val, max_val);
+ */
+
+extern Datum partition_insert_trigger(PG_FUNCTION_ARGS);
+
+static Datum
+check_constraints_and_insert_tuple(Relation child_table_relation, TriggerData *trigdata ,HeapTuple trigtuple)
+{ // Check the constraints
+ ResultRelInfo *resultRelInfo;
+ TupleTableSlot *slot;
+ EState *estate = CreateExecutorState();
+ Datum result;
+
+ result = PointerGetDatum(trigdata->tg_trigtuple);
+
+ resultRelInfo = makeNode(ResultRelInfo);
+ resultRelInfo->ri_RangeTableIndex = 1; /* dummy */
+ resultRelInfo->ri_RelationDesc = child_table_relation;
+
+ estate->es_result_relations = resultRelInfo;
+ estate->es_num_result_relations = 1;
+ estate->es_result_relation_info = resultRelInfo;
+
+ /* Set up a tuple slot too */
+ slot = MakeSingleTupleTableSlot(trigdata->tg_relation->rd_att);
+ ExecStoreTuple(trigtuple, slot, InvalidBuffer, false);
+
+ if (ExecRelCheck(resultRelInfo, slot, estate) == NULL)
+ { // Constraints satisfied, insert the row in the child table
+ bool use_wal = true;
+ bool use_fsm=true;
+
+ /* BEFORE ROW INSERT Triggers */
+ if (resultRelInfo->ri_TrigDesc &&
+ resultRelInfo->ri_TrigDesc->n_before_row[TRIGGER_EVENT_INSERT] > 0)
+ {
+ HeapTuple newtuple;
+ newtuple = ExecBRInsertTriggers(estate, resultRelInfo, trigtuple);
+
+ if (newtuple != trigtuple) /* modified by Trigger(s) */
+ {
+ heap_freetuple(trigtuple);
+ trigtuple = newtuple;
+ }
+ }
+
+ /* Perform the insert
+ * TODO: Check that we detect constraint violation if before row insert does something bad */
+ heap_insert(child_table_relation, trigtuple, GetCurrentCommandId(true), use_wal, use_fsm);
+
+ /* Update indices */
+ ExecOpenIndices(resultRelInfo);
+ if (resultRelInfo->ri_NumIndices > 0)
+ ExecInsertIndexTuples(slot, &(trigtuple->t_self), estate, false);
+
+ /* AFTER ROW INSERT Triggers */
+ ExecARInsertTriggers(estate, resultRelInfo, trigtuple);
+
+ result = PointerGetDatum(NULL);
+ }
+ // Free resources
+ FreeExecutorState(estate);
+ ExecDropSingleTupleTableSlot(slot);
+
+ return result;
+}
+
+
+PG_FUNCTION_INFO_V1(partition_insert_trigger);
+
+static Relation last_inserted_relation;
+
+Datum
+partition_insert_trigger(PG_FUNCTION_ARGS)
+{
+ TriggerData *trigdata = (TriggerData *) fcinfo->context;
+ HeapTuple trigtuple= trigdata->tg_trigtuple;
+ TupleConstr *constr;
+ char *child_table_name;
+ Relation child_table_relation;
+ Oid relation_id;
+ Datum result;
+
+ // Try to exploit locality for bulk inserts
+ // We expect consecutive insert to go to the same child table
+ if (last_inserted_relation != NULL)
+ { // Try the last table we used
+ result = check_constraints_and_insert_tuple(last_inserted_relation, trigdata, trigtuple);
+ if (result == PointerGetDatum(NULL))
+ return result;
+ // We got a miss
+ last_inserted_relation = NULL;
+ }
+
+ /* make sure it's called as a trigger at all */
+ if (!CALLED_AS_TRIGGER(fcinfo))
+ elog(ERROR, "partition_insert_trigger: not called by trigger manager");
+
+ /* Sanity checks */
+ if (!TRIGGER_FIRED_BY_INSERT(trigdata->tg_event) || !TRIGGER_FIRED_BEFORE(trigdata->tg_event))
+ elog(ERROR, "partition_insert_trigger: not called on insert before");
+
+ // Child table name is either given as the unique parameter or it is the name of the trigger
+ if (trigdata->tg_trigger->tgnargs == 1)
+ child_table_name = trigdata->tg_trigger->tgargs[0];
+ else
+ child_table_name = trigdata->tg_trigger->tgname;
+
+ // Lookup the child relation
+ relation_id = RelnameGetRelid(child_table_name);
+ if (relation_id == InvalidOid)
+ elog(ERROR, "partition_insert_trigger: Invalid child table %s", child_table_name);
+ child_table_relation = RelationIdGetRelation(relation_id);
+ if (child_table_relation == NULL)
+ elog(ERROR, "partition_insert_trigger: Failed to locate relation for child table %s", child_table_name);
+
+ constr = child_table_relation->rd_att->constr;
+ if (constr->num_check == 0)
+ elog(ERROR, "partition_insert_trigger: No constraint found for child table %s", child_table_name);
+
+ result = check_constraints_and_insert_tuple(child_table_relation, trigdata, trigtuple);
+
+ if (result == PointerGetDatum(NULL))
+ last_inserted_relation = child_table_relation;
+
+ RelationClose(child_table_relation);
+
+ return result;
+}
+
Index: src/backend/executor/execMain.c
===================================================================
RCS file: /root/cvsrepo/pgsql/src/backend/executor/execMain.c,v
retrieving revision 1.314
diff -u -r1.314 execMain.c
--- src/backend/executor/execMain.c 31 Oct 2008 21:07:54 -0000 1.314
+++ src/backend/executor/execMain.c 16 Dec 2008 01:42:50 -0000
@@ -1947,7 +1947,7 @@
/*
* ExecRelCheck --- check that tuple meets constraints for result relation
*/
-static const char *
+const char *
ExecRelCheck(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate)
{
Index: src/test/regress/output/create_function_1.source
===================================================================
RCS file: /root/cvsrepo/pgsql/src/test/regress/output/create_function_1.source,v
retrieving revision 1.34
diff -u -r1.34 create_function_1.source
--- src/test/regress/output/create_function_1.source 31 Oct 2008 19:37:56 -0000 1.34
+++ src/test/regress/output/create_function_1.source 16 Dec 2008 01:42:50 -0000
@@ -47,6 +47,10 @@
RETURNS int4
AS '@libdir@/regress@DLSUFFIX@'
LANGUAGE C STRICT;
+CREATE FUNCTION partition_insert_trigger ()
+ RETURNS trigger
+ AS '@libdir@/regress@DLSUFFIX@'
+ LANGUAGE C STRICT;
-- Things that shouldn't work:
CREATE FUNCTION test1 (int) RETURNS int LANGUAGE SQL
AS 'SELECT ''not an integer'';';
@@ -80,3 +84,70 @@
CREATE FUNCTION test1 (int) RETURNS int LANGUAGE internal
AS 'nosuch';
ERROR: there is no built-in function named "nosuch"
+-- Partitioning trigger test
+CREATE TABLE master (
+ id int not null,
+ date date not null,
+ value int
+);
+CREATE TABLE child_y2008m01 (
+ CHECK ( date >= DATE '2008-01-01' AND date < DATE '2008-02-01' )
+) INHERITS (master);
+CREATE TABLE child_y2008m02 (
+ CHECK ( date >= DATE '2008-02-01' AND date < DATE '2008-03-01' )
+) INHERITS (master);
+CREATE TABLE child_y2008m03 (
+ CHECK ( date >= DATE '2008-03-01' AND date < DATE '2008-04-01' )
+) INHERITS (master);
+CREATE TRIGGER insert_child_y2008m01
+ BEFORE INSERT ON master
+ FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger('child_y2008m01');
+CREATE TRIGGER child_y2008m02
+ BEFORE INSERT ON master
+ FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger();
+CREATE TRIGGER child_y2008m03
+ BEFORE INSERT ON master
+ FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger();
+INSERT INTO master VALUES (1, '2008-01-15', 1);
+INSERT INTO master VALUES (2, '2008-02-15', 2);
+INSERT INTO master VALUES (3, '2008-03-15', 3);
+INSERT INTO master VALUES (4, '2008-04-15', 4);
+COPY master FROM '/root/pg_partitions/copy_input.txt';
+select * from master;
+ id | date | value
+----+------------+-------
+ 4 | 04-15-2008 | 4
+ 1 | 01-15-2008 | 1
+ 11 | 01-10-2008 | 11
+ 2 | 02-15-2008 | 2
+ 12 | 02-15-2008 | 12
+ 3 | 03-15-2008 | 3
+ 13 | 03-15-2008 | 13
+(7 rows)
+
+ select * from child_y2008m01;
+ id | date | value
+----+------------+-------
+ 1 | 01-15-2008 | 1
+ 11 | 01-10-2008 | 11
+(2 rows)
+
+ select * from child_y2008m02;
+ id | date | value
+----+------------+-------
+ 2 | 02-15-2008 | 2
+ 12 | 02-15-2008 | 12
+(2 rows)
+
+ select * from child_y2008m03;
+ id | date | value
+----+------------+-------
+ 3 | 03-15-2008 | 3
+ 13 | 03-15-2008 | 13
+(2 rows)
+
+DROP TABLE master CASCADE;
+NOTICE: drop cascades to 3 other objects
+DETAIL: drop cascades to table child_y2008m01
+drop cascades to table child_y2008m02
+drop cascades to table child_y2008m03
Index: src/include/executor/executor.h
===================================================================
RCS file: /root/cvsrepo/pgsql/src/include/executor/executor.h,v
retrieving revision 1.152
diff -u -r1.152 executor.h
--- src/include/executor/executor.h 31 Oct 2008 21:07:55 -0000 1.152
+++ src/include/executor/executor.h 16 Dec 2008 01:42:50 -0000
@@ -155,6 +155,8 @@
extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
extern void ExecConstraints(ResultRelInfo *resultRelInfo,
TupleTableSlot *slot, EState *estate);
+extern const char *ExecRelCheck(ResultRelInfo *resultRelInfo,
+ TupleTableSlot *slot, EState *estate);
extern TupleTableSlot *EvalPlanQual(EState *estate, Index rti,
ItemPointer tid, TransactionId priorXmax);
extern PlanState *ExecGetActivePlanTree(QueryDesc *queryDesc);
Index: src/test/regress/input/create_function_1.source
===================================================================
RCS file: /root/cvsrepo/pgsql/src/test/regress/input/create_function_1.source,v
retrieving revision 1.19
diff -u -r1.19 create_function_1.source
--- src/test/regress/input/create_function_1.source 1 Oct 2008 22:38:57 -0000 1.19
+++ src/test/regress/input/create_function_1.source 16 Dec 2008 01:42:50 -0000
@@ -52,6 +52,12 @@
AS '@libdir@/regress@DLSUFFIX@'
LANGUAGE C STRICT;
+CREATE FUNCTION partition_insert_trigger ()
+ RETURNS trigger
+ AS '@libdir@/regress@DLSUFFIX@'
+ LANGUAGE C STRICT;
+
+
-- Things that shouldn't work:
CREATE FUNCTION test1 (int) RETURNS int LANGUAGE SQL
@@ -77,3 +83,48 @@
CREATE FUNCTION test1 (int) RETURNS int LANGUAGE internal
AS 'nosuch';
+
+-- Partitioning trigger test
+
+CREATE TABLE master (
+ id int not null,
+ date date not null,
+ value int
+);
+
+CREATE TABLE child_y2008m01 (
+ CHECK ( date >= DATE '2008-01-01' AND date < DATE '2008-02-01' )
+) INHERITS (master);
+
+CREATE TABLE child_y2008m02 (
+ CHECK ( date >= DATE '2008-02-01' AND date < DATE '2008-03-01' )
+) INHERITS (master);
+
+CREATE TABLE child_y2008m03 (
+ CHECK ( date >= DATE '2008-03-01' AND date < DATE '2008-04-01' )
+) INHERITS (master);
+
+CREATE TRIGGER insert_child_y2008m01
+ BEFORE INSERT ON master
+ FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger('child_y2008m01');
+
+CREATE TRIGGER child_y2008m02
+ BEFORE INSERT ON master
+ FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger();
+
+CREATE TRIGGER child_y2008m03
+ BEFORE INSERT ON master
+ FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger();
+
+INSERT INTO master VALUES (1, '2008-01-15', 1);
+INSERT INTO master VALUES (2, '2008-02-15', 2);
+INSERT INTO master VALUES (3, '2008-03-15', 3);
+INSERT INTO master VALUES (4, '2008-04-15', 4);
+COPY master FROM '/root/pg_partitions/copy_input.txt';
+
+select * from master;
+select * from child_y2008m01;
+select * from child_y2008m02;
+select * from child_y2008m03;
+
+DROP TABLE master CASCADE;
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-12-16 06:35 Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
parent: Emmanuel Cecchet <manu@frogthinker.org>
1 sibling, 1 reply; 59+ messages in thread
From: Nikhil Sontakke @ 2008-12-16 06:35 UTC (permalink / raw)
To: Emmanuel Cecchet <manu@frogthinker.org>; +Cc: Jaime Casanova <jcasanov@systemguards.com.ec>; Gregory Stark <stark@enterprisedb.com>; Josh Berkus <josh@agliodbs.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>
Hi,
> I will be working on a roadmap for the partitioning features. I think that
> there are different needs and that we will not be able to address them all
> in 8.5 or even 8.6.
> The goal will be to get things done step by step but possibly with a design
> that will not require major refactoring to support new features. I will try
> to setup the new wiki page tomorrow.
>
+1.
>
> In the meantime, I have made some more tests with the trigger in C (see
> attached patch). To prevent duplicating too much code, it requires the
> ExecRelCheck method to be exported (that would be nice to have this function
> exported in 8.4 so that we can start experimenting in 8.4 and don't have to
> wait another year for 8.5). If there is locality in the inserts (which might
> be the case if you COPY sorted data), the performance remains constant
> regardless the number of child tables.
A similar DELETE trigger should be pretty easy to write up in C. I think the
main challenge is with UPDATE triggers especially if the new row will fall
into another child table - but we can always throw an error for such a case
initially.
>
> Right now if the row cannot be inserted in any child table, it is inserted
> in the parent. If you want to fail, we can add a 'fail trigger' (after all
> other triggers) that generates an error if previous triggers did not capture
> the row. If you want to create a new partition, you can have another trigger
> to handle that.
>
One of the work items related to partitioning eventually is to avoid having
to APPEND the parent in all queries involving children. Maybe having an
overflow child table might help to catch failed triggers for those cases?
Regards,
Nikhils
>
> So I think that this trigger approach is pretty flexible like people used
> AOP in J2EE servers to process requests. It has also the advantage of
> allowing fast prototyping. It should also be easy to push that functionality
> down in the core as needed.
>
> Is it ok if I move Simon's requirement document under a more generic 'Table
> partitioning' page on the Wiki?
>
> Thanks for your feedback,
> manu
>
> Jaime Casanova wrote:
>
>> On Mon, Dec 15, 2008 at 1:46 PM, Gregory Stark <stark@enterprisedb.com>
>> wrote:
>>
>>
>>> Josh Berkus <josh@agliodbs.com> writes:
>>>
>>>
>>>
>>>> Hackers,
>>>>
>>>> We don't yet seem to have a clear specification for this feature, and
>>>> the Other
>>>> Open Source DB has shown us how problematic it is to get
>>>> auto-partitioning
>>>> wrong.
>>>>
>>>> Should we defer auto-partitioning to 8.5?
>>>>
>>>>
>>> If we're serious about having a "next generation" partitioning with a
>>> concept
>>> of partition keys then it seems to me to make more sense to do that first
>>> and
>>> then add on a feature like this.
>>>
>>>
>>>
>>
>> +1
>>
>>
>>
>>> This is still very useful. I haven't looked at the actual patch, does it
>>> require core changes or can it be stashed in a pgfoundry or contrib
>>> module?
>>>
>>>
>>>
>>
>> what i don't like about this one is that it creates partitions at
>> create table time and to manually add all new partitions (inherit
>> tables and modify the trigger)... and what i want to see is an
>> automatic creation when it's needed...
>>
>>
>>
>>
>
>
> --
> Emmanuel Cecchet
> FTO @ Frog Thinker Open Source Development & Consulting
> --
> Web: http://www.frogthinker.org
> email: manu@frogthinker.org
> Skype: emmanuel_cecchet
>
>
> ### Eclipse Workspace Patch 1.0
> #P Postgres-HEAD
> Index: src/test/regress/regress.c
> ===================================================================
> RCS file: /root/cvsrepo/pgsql/src/test/regress/regress.c,v
> retrieving revision 1.71
> diff -u -r1.71 regress.c
> --- src/test/regress/regress.c 25 Mar 2008 22:42:46 -0000 1.71
> +++ src/test/regress/regress.c 16 Dec 2008 01:42:50 -0000
> @@ -10,6 +10,9 @@
> #include "utils/geo_decls.h" /* includes <math.h> */
> #include "executor/executor.h" /* For GetAttributeByName */
> #include "commands/sequence.h" /* for nextval() */
> +#include "catalog/namespace.h"
> +#include "executor/executor.h"
> +#include "executor/tuptable.h"
>
> #define P_MAXDIG 12
> #define LDELIM '('
> @@ -732,3 +735,141 @@
> *--walk = '\0';
> PG_RETURN_CSTRING(result);
> }
> +
> +
> +/*
> + * Partition trigger test
> + *
> + * The trigger should be used this way:
> + * CREATE TRIGGER trigger_name
> + BEFORE INSERT ON master_table
> + FOR EACH ROW EXECUTE PROCEDURE
> partition_insert_trigger(child_table_name, column_number, min_val, max_val);
> + */
> +
> +extern Datum partition_insert_trigger(PG_FUNCTION_ARGS);
> +
> +static Datum
> +check_constraints_and_insert_tuple(Relation child_table_relation,
> TriggerData *trigdata ,HeapTuple trigtuple)
> +{ // Check the constraints
> + ResultRelInfo *resultRelInfo;
> + TupleTableSlot *slot;
> + EState *estate = CreateExecutorState();
> + Datum result;
> +
> + result = PointerGetDatum(trigdata->tg_trigtuple);
> +
> + resultRelInfo = makeNode(ResultRelInfo);
> + resultRelInfo->ri_RangeTableIndex = 1; /* dummy */
> + resultRelInfo->ri_RelationDesc = child_table_relation;
> +
> + estate->es_result_relations = resultRelInfo;
> + estate->es_num_result_relations = 1;
> + estate->es_result_relation_info = resultRelInfo;
> +
> + /* Set up a tuple slot too */
> + slot = MakeSingleTupleTableSlot(trigdata->tg_relation->rd_att);
> + ExecStoreTuple(trigtuple, slot, InvalidBuffer, false);
> +
> + if (ExecRelCheck(resultRelInfo, slot, estate) == NULL)
> + { // Constraints satisfied, insert the row in the child table
> + bool use_wal = true;
> + bool use_fsm=true;
> +
> + /* BEFORE ROW INSERT Triggers */
> + if (resultRelInfo->ri_TrigDesc &&
> +
> resultRelInfo->ri_TrigDesc->n_before_row[TRIGGER_EVENT_INSERT] > 0)
> + {
> + HeapTuple newtuple;
> + newtuple = ExecBRInsertTriggers(estate,
> resultRelInfo, trigtuple);
> +
> + if (newtuple != trigtuple) /* modified by
> Trigger(s) */
> + {
> + heap_freetuple(trigtuple);
> + trigtuple = newtuple;
> + }
> + }
> +
> + /* Perform the insert
> + * TODO: Check that we detect constraint violation if
> before row insert does something bad */
> + heap_insert(child_table_relation, trigtuple,
> GetCurrentCommandId(true), use_wal, use_fsm);
> +
> + /* Update indices */
> + ExecOpenIndices(resultRelInfo);
> + if (resultRelInfo->ri_NumIndices > 0)
> + ExecInsertIndexTuples(slot, &(trigtuple->t_self),
> estate, false);
> +
> + /* AFTER ROW INSERT Triggers */
> + ExecARInsertTriggers(estate, resultRelInfo, trigtuple);
> +
> + result = PointerGetDatum(NULL);
> + }
> + // Free resources
> + FreeExecutorState(estate);
> + ExecDropSingleTupleTableSlot(slot);
> +
> + return result;
> +}
> +
> +
> +PG_FUNCTION_INFO_V1(partition_insert_trigger);
> +
> +static Relation last_inserted_relation;
> +
> +Datum
> +partition_insert_trigger(PG_FUNCTION_ARGS)
> +{
> + TriggerData *trigdata = (TriggerData *) fcinfo->context;
> + HeapTuple trigtuple= trigdata->tg_trigtuple;
> + TupleConstr *constr;
> + char *child_table_name;
> + Relation child_table_relation;
> + Oid relation_id;
> + Datum result;
> +
> + // Try to exploit locality for bulk inserts
> + // We expect consecutive insert to go to the same child table
> + if (last_inserted_relation != NULL)
> + { // Try the last table we used
> + result =
> check_constraints_and_insert_tuple(last_inserted_relation, trigdata,
> trigtuple);
> + if (result == PointerGetDatum(NULL))
> + return result;
> + // We got a miss
> + last_inserted_relation = NULL;
> + }
> +
> + /* make sure it's called as a trigger at all */
> + if (!CALLED_AS_TRIGGER(fcinfo))
> + elog(ERROR, "partition_insert_trigger: not called by trigger
> manager");
> +
> + /* Sanity checks */
> + if (!TRIGGER_FIRED_BY_INSERT(trigdata->tg_event) ||
> !TRIGGER_FIRED_BEFORE(trigdata->tg_event))
> + elog(ERROR, "partition_insert_trigger: not called on insert
> before");
> +
> + // Child table name is either given as the unique parameter or it is
> the name of the trigger
> + if (trigdata->tg_trigger->tgnargs == 1)
> + child_table_name = trigdata->tg_trigger->tgargs[0];
> + else
> + child_table_name = trigdata->tg_trigger->tgname;
> +
> + // Lookup the child relation
> + relation_id = RelnameGetRelid(child_table_name);
> + if (relation_id == InvalidOid)
> + elog(ERROR, "partition_insert_trigger: Invalid child table %s",
> child_table_name);
> + child_table_relation = RelationIdGetRelation(relation_id);
> + if (child_table_relation == NULL)
> + elog(ERROR, "partition_insert_trigger: Failed to locate relation
> for child table %s", child_table_name);
> +
> + constr = child_table_relation->rd_att->constr;
> + if (constr->num_check == 0)
> + elog(ERROR, "partition_insert_trigger: No constraint found
> for child table %s", child_table_name);
> +
> + result = check_constraints_and_insert_tuple(child_table_relation,
> trigdata, trigtuple);
> +
> + if (result == PointerGetDatum(NULL))
> + last_inserted_relation = child_table_relation;
> +
> + RelationClose(child_table_relation);
> +
> + return result;
> +}
> +
> Index: src/backend/executor/execMain.c
> ===================================================================
> RCS file: /root/cvsrepo/pgsql/src/backend/executor/execMain.c,v
> retrieving revision 1.314
> diff -u -r1.314 execMain.c
> --- src/backend/executor/execMain.c 31 Oct 2008 21:07:54 -0000
> 1.314
> +++ src/backend/executor/execMain.c 16 Dec 2008 01:42:50 -0000
> @@ -1947,7 +1947,7 @@
> /*
> * ExecRelCheck --- check that tuple meets constraints for result relation
> */
> -static const char *
> +const char *
> ExecRelCheck(ResultRelInfo *resultRelInfo,
> TupleTableSlot *slot, EState *estate)
> {
> Index: src/test/regress/output/create_function_1.source
> ===================================================================
> RCS file:
> /root/cvsrepo/pgsql/src/test/regress/output/create_function_1.source,v
> retrieving revision 1.34
> diff -u -r1.34 create_function_1.source
> --- src/test/regress/output/create_function_1.source 31 Oct 2008
> 19:37:56 -0000 1.34
> +++ src/test/regress/output/create_function_1.source 16 Dec 2008
> 01:42:50 -0000
> @@ -47,6 +47,10 @@
> RETURNS int4
> AS '@libdir@/regress@DLSUFFIX@'
> LANGUAGE C STRICT;
> +CREATE FUNCTION partition_insert_trigger ()
> + RETURNS trigger
> + AS '@libdir@/regress@DLSUFFIX@'
> + LANGUAGE C STRICT;
> -- Things that shouldn't work:
> CREATE FUNCTION test1 (int) RETURNS int LANGUAGE SQL
> AS 'SELECT ''not an integer'';';
> @@ -80,3 +84,70 @@
> CREATE FUNCTION test1 (int) RETURNS int LANGUAGE internal
> AS 'nosuch';
> ERROR: there is no built-in function named "nosuch"
> +-- Partitioning trigger test
> +CREATE TABLE master (
> + id int not null,
> + date date not null,
> + value int
> +);
> +CREATE TABLE child_y2008m01 (
> + CHECK ( date >= DATE '2008-01-01' AND date < DATE '2008-02-01' )
> +) INHERITS (master);
> +CREATE TABLE child_y2008m02 (
> + CHECK ( date >= DATE '2008-02-01' AND date < DATE '2008-03-01' )
> +) INHERITS (master);
> +CREATE TABLE child_y2008m03 (
> + CHECK ( date >= DATE '2008-03-01' AND date < DATE '2008-04-01' )
> +) INHERITS (master);
> +CREATE TRIGGER insert_child_y2008m01
> + BEFORE INSERT ON master
> + FOR EACH ROW EXECUTE PROCEDURE
> partition_insert_trigger('child_y2008m01');
> +CREATE TRIGGER child_y2008m02
> + BEFORE INSERT ON master
> + FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger();
> +CREATE TRIGGER child_y2008m03
> + BEFORE INSERT ON master
> + FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger();
> +INSERT INTO master VALUES (1, '2008-01-15', 1);
> +INSERT INTO master VALUES (2, '2008-02-15', 2);
> +INSERT INTO master VALUES (3, '2008-03-15', 3);
> +INSERT INTO master VALUES (4, '2008-04-15', 4);
> +COPY master FROM '/root/pg_partitions/copy_input.txt';
> +select * from master;
> + id | date | value
> +----+------------+-------
> + 4 | 04-15-2008 | 4
> + 1 | 01-15-2008 | 1
> + 11 | 01-10-2008 | 11
> + 2 | 02-15-2008 | 2
> + 12 | 02-15-2008 | 12
> + 3 | 03-15-2008 | 3
> + 13 | 03-15-2008 | 13
> +(7 rows)
> +
> + select * from child_y2008m01;
> + id | date | value
> +----+------------+-------
> + 1 | 01-15-2008 | 1
> + 11 | 01-10-2008 | 11
> +(2 rows)
> +
> + select * from child_y2008m02;
> + id | date | value
> +----+------------+-------
> + 2 | 02-15-2008 | 2
> + 12 | 02-15-2008 | 12
> +(2 rows)
> +
> + select * from child_y2008m03;
> + id | date | value
> +----+------------+-------
> + 3 | 03-15-2008 | 3
> + 13 | 03-15-2008 | 13
> +(2 rows)
> +
> +DROP TABLE master CASCADE;
> +NOTICE: drop cascades to 3 other objects
> +DETAIL: drop cascades to table child_y2008m01
> +drop cascades to table child_y2008m02
> +drop cascades to table child_y2008m03
> Index: src/include/executor/executor.h
> ===================================================================
> RCS file: /root/cvsrepo/pgsql/src/include/executor/executor.h,v
> retrieving revision 1.152
> diff -u -r1.152 executor.h
> --- src/include/executor/executor.h 31 Oct 2008 21:07:55 -0000
> 1.152
> +++ src/include/executor/executor.h 16 Dec 2008 01:42:50 -0000
> @@ -155,6 +155,8 @@
> extern bool ExecContextForcesOids(PlanState *planstate, bool *hasoids);
> extern void ExecConstraints(ResultRelInfo *resultRelInfo,
> TupleTableSlot *slot, EState *estate);
> +extern const char *ExecRelCheck(ResultRelInfo *resultRelInfo,
> + TupleTableSlot *slot, EState *estate);
> extern TupleTableSlot *EvalPlanQual(EState *estate, Index rti,
> ItemPointer tid, TransactionId priorXmax);
> extern PlanState *ExecGetActivePlanTree(QueryDesc *queryDesc);
> Index: src/test/regress/input/create_function_1.source
> ===================================================================
> RCS file:
> /root/cvsrepo/pgsql/src/test/regress/input/create_function_1.source,v
> retrieving revision 1.19
> diff -u -r1.19 create_function_1.source
> --- src/test/regress/input/create_function_1.source 1 Oct 2008 22:38:57
> -0000 1.19
> +++ src/test/regress/input/create_function_1.source 16 Dec 2008
> 01:42:50 -0000
> @@ -52,6 +52,12 @@
> AS '@libdir@/regress@DLSUFFIX@'
> LANGUAGE C STRICT;
>
> +CREATE FUNCTION partition_insert_trigger ()
> + RETURNS trigger
> + AS '@libdir@/regress@DLSUFFIX@'
> + LANGUAGE C STRICT;
> +
> +
> -- Things that shouldn't work:
>
> CREATE FUNCTION test1 (int) RETURNS int LANGUAGE SQL
> @@ -77,3 +83,48 @@
>
> CREATE FUNCTION test1 (int) RETURNS int LANGUAGE internal
> AS 'nosuch';
> +
> +-- Partitioning trigger test
> +
> +CREATE TABLE master (
> + id int not null,
> + date date not null,
> + value int
> +);
> +
> +CREATE TABLE child_y2008m01 (
> + CHECK ( date >= DATE '2008-01-01' AND date < DATE '2008-02-01' )
> +) INHERITS (master);
> +
> +CREATE TABLE child_y2008m02 (
> + CHECK ( date >= DATE '2008-02-01' AND date < DATE '2008-03-01' )
> +) INHERITS (master);
> +
> +CREATE TABLE child_y2008m03 (
> + CHECK ( date >= DATE '2008-03-01' AND date < DATE '2008-04-01' )
> +) INHERITS (master);
> +
> +CREATE TRIGGER insert_child_y2008m01
> + BEFORE INSERT ON master
> + FOR EACH ROW EXECUTE PROCEDURE
> partition_insert_trigger('child_y2008m01');
> +
> +CREATE TRIGGER child_y2008m02
> + BEFORE INSERT ON master
> + FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger();
> +
> +CREATE TRIGGER child_y2008m03
> + BEFORE INSERT ON master
> + FOR EACH ROW EXECUTE PROCEDURE partition_insert_trigger();
> +
> +INSERT INTO master VALUES (1, '2008-01-15', 1);
> +INSERT INTO master VALUES (2, '2008-02-15', 2);
> +INSERT INTO master VALUES (3, '2008-03-15', 3);
> +INSERT INTO master VALUES (4, '2008-04-15', 4);
> +COPY master FROM '/root/pg_partitions/copy_input.txt';
> +
> +select * from master;
> +select * from child_y2008m01;
> +select * from child_y2008m02;
> +select * from child_y2008m03;
> +
> +DROP TABLE master CASCADE;
>
>
--
http://www.enterprisedb.com
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-12-16 07:02 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
parent: Emmanuel Cecchet <manu@frogthinker.org>
1 sibling, 1 reply; 59+ messages in thread
From: ITAGAKI Takahiro @ 2008-12-16 07:02 UTC (permalink / raw)
To: Emmanuel Cecchet <manu@frogthinker.org>; +Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Emmanuel Cecchet <manu@frogthinker.org> wrote:
> In the meantime, I have made some more tests with the trigger in C (see
> attached patch).
Hmm... The inserting partition is passed by trigger arguments.
Users must replace triggers when the target is changed (ex. every month).
Is it possible to expand all of child paritions from pg_inherits and
determine a suitable parition by checking their constraints?
We can also use it when there are multiple inserting paritions,
something like hash or list paritioning. Fixed target is only applicable
to time-based range paritioning.
BTW, there is another issue in trigger approach. If INSERT commands
are interrupted by triggers, server says "INSERT 0 row" though
rows are inserted into child tables. Since using C, we could
use some back doors to modify a variable counting affected rows.
We could use partitioned tables more transparently if we have it.
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-12-16 07:13 Emmanuel Cecchet <manu@frogthinker.org>
parent: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
0 siblings, 0 replies; 59+ messages in thread
From: Emmanuel Cecchet @ 2008-12-16 07:13 UTC (permalink / raw)
To: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>; +Cc: Jaime Casanova <jcasanov@systemguards.com.ec>; Gregory Stark <stark@enterprisedb.com>; Josh Berkus <josh@agliodbs.com>; PostgreSQL-development <pgsql-hackers@postgresql.org>
Nikhil Sontakke wrote:
> A similar DELETE trigger should be pretty easy to write up in C. I
> think the main challenge is with UPDATE triggers especially if the new
> row will fall into another child table - but we can always throw an
> error for such a case initially.
I agree. A first implementation could be restricted to updates within
the same partition.
>
> Right now if the row cannot be inserted in any child table, it is
> inserted in the parent. If you want to fail, we can add a 'fail
> trigger' (after all other triggers) that generates an error if
> previous triggers did not capture the row. If you want to create a
> new partition, you can have another trigger to handle that.
>
>
> One of the work items related to partitioning eventually is to avoid
> having to APPEND the parent in all queries involving children. Maybe
> having an overflow child table might help to catch failed triggers for
> those cases?
This is a good option too.
Emmanuel
>
> Regards,
> Nikhils
>
>
>
> So I think that this trigger approach is pretty flexible like
> people used AOP in J2EE servers to process requests. It has also
> the advantage of allowing fast prototyping. It should also be easy
> to push that functionality down in the core as needed.
>
> Is it ok if I move Simon's requirement document under a more
> generic 'Table partitioning' page on the Wiki?
>
> Thanks for your feedback,
> manu
>
>
> Jaime Casanova wrote:
>
> On Mon, Dec 15, 2008 at 1:46 PM, Gregory Stark
> <stark@enterprisedb.com <mailto:stark@enterprisedb.com>> wrote:
>
>
> Josh Berkus <josh@agliodbs.com <mailto:josh@agliodbs.com>>
> writes:
>
>
>
> Hackers,
>
> We don't yet seem to have a clear specification for
> this feature, and the Other
> Open Source DB has shown us how problematic it is to
> get auto-partitioning
> wrong.
>
> Should we defer auto-partitioning to 8.5?
>
>
> If we're serious about having a "next generation"
> partitioning with a concept
> of partition keys then it seems to me to make more sense
> to do that first and
> then add on a feature like this.
>
>
>
>
> +1
>
>
>
> This is still very useful. I haven't looked at the actual
> patch, does it
> require core changes or can it be stashed in a pgfoundry
> or contrib module?
>
>
>
>
> what i don't like about this one is that it creates partitions at
> create table time and to manually add all new partitions (inherit
> tables and modify the trigger)... and what i want to see is an
> automatic creation when it's needed...
>
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-12-16 07:33 Emmanuel Cecchet <manu@frogthinker.org>
parent: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
0 siblings, 1 reply; 59+ messages in thread
From: Emmanuel Cecchet @ 2008-12-16 07:33 UTC (permalink / raw)
To: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; +Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
ITAGAKI Takahiro wrote:
> Emmanuel Cecchet <manu@frogthinker.org> wrote
>> In the meantime, I have made some more tests with the trigger in C (see
>> attached patch).
>>
>
> Hmm... The inserting partition is passed by trigger arguments.
>
Actually this is just a fallback option. The preferred option is to name
the trigger after the child table name. This way the trigger retrieve
the table name directly from the trigger name and no argument has to be
passed to the trigger.
> Users must replace triggers when the target is changed (ex. every month).
>
I am not sure what you mean. There is one trigger per child table but
the trigger is always the same, it is just the name that is given to it
that changes.
> Is it possible to expand all of child paritions from pg_inherits and
> determine a suitable parition by checking their constraints?
>
Ideally it would be better to do this way. I have not found yet how to
automatically get all the child partitions of a parent table from the
trigger. This would simplify things by having a single trigger.
> We can also use it when there are multiple inserting paritions,
> something like hash or list paritioning. Fixed target is only applicable
> to time-based range paritioning.
>
I think there is a misunderstanding on how the trigger works. You have 1
trigger per child table and they are all chained on the parent table.
When a tuple is inserted on the parent table, the first trigger is
fired, if the constraints of the 1st child table are satisfied, the
tuple is moved in the 1st child table and that's it. If it is a miss,
the tuple is passed to the next trigger that checks the constraints of
the 2nd table. And so on.
This will work with any type of partitioning (hash or even UDF) as long
as the constraints on the child table reflect the partitioning.
> BTW, there is another issue in trigger approach. If INSERT commands
> are interrupted by triggers, server says "INSERT 0 row" though
> rows are inserted into child tables. Since using C, we could
> use some back doors to modify a variable counting affected rows.
> We could use partitioned tables more transparently if we have it.
>
Even if you don't abort the query, the query reports 0 row if it has
been moved to another table (you can COPY 100k lines and the server will
return 0 if they were all successfully moved to child tables).
Technically this is correct since 0 rows were inserted in the parent
table. Right now any number >0 is the number of rows that did not
satisfy any child table constraint and were inserted in the master table
(useful if you don't want the copy command to fail).
Regards,
Emmanuel
> Regards,
> ---
> ITAGAKI Takahiro
> NTT Open Source Software Center
>
--
Emmanuel Cecchet
FTO @ Frog Thinker
Open Source Development & Consulting
--
Web: http://www.frogthinker.org
email: manu@frogthinker.org
Skype: emmanuel_cecchet
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-12-16 07:58 ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
parent: Emmanuel Cecchet <manu@frogthinker.org>
0 siblings, 2 replies; 59+ messages in thread
From: ITAGAKI Takahiro @ 2008-12-16 07:58 UTC (permalink / raw)
To: Emmanuel Cecchet <manu@frogthinker.org>; +Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Emmanuel Cecchet <manu@frogthinker.org> wrote:
> I think there is a misunderstanding on how the trigger works. You have 1
> trigger per child table and they are all chained on the parent table.
Oops, I misunderstand your patch, sorry.
> > Is it possible to expand all of child paritions from pg_inherits and
> > determine a suitable parition by checking their constraints?
> >
> Ideally it would be better to do this way. I have not found yet how to
> automatically get all the child partitions of a parent table from the
> trigger. This would simplify things by having a single trigger.
The chained triggers would have better flexibilty, and the auto expanding
trigger would have better usability. I'm not sure about performance
because expanding child partitions is not always faster than chained
calls of triggers.
I think chained triggers are hard to maintain. If we drop one of partition
tables, we need to reconnect the single-linked-list of the triggers.
> > server says "INSERT 0 row" though rows are inserted into child tables.
> Technically this is correct since 0 rows were inserted in the parent
> table.
Yes, but users expect non-0 result normally. Some O/R mapping tools
also checks the result exactly and raises errors (it could be turned
off, but default is on).
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-12-16 08:19 Emmanuel Cecchet <manu@frogthinker.org>
parent: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
1 sibling, 0 replies; 59+ messages in thread
From: Emmanuel Cecchet @ 2008-12-16 08:19 UTC (permalink / raw)
To: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; +Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
ITAGAKI Takahiro wrote:
> The chained triggers would have better flexibilty, and the auto expanding
> trigger would have better usability. I'm not sure about performance
> because expanding child partitions is not always faster than chained
> calls of triggers.
>
> I think chained triggers are hard to maintain. If we drop one of partition
> tables, we need to reconnect the single-linked-list of the triggers.
>
When you drop one child table, you would also have to drop the trigger
that has the same name on the parent table. This does not seem too hard
but I may be missing something.
>>> server says "INSERT 0 row" though rows are inserted into child tables.
>>>
>> Technically this is correct since 0 rows were inserted in the parent
>> table.
>>
> Yes, but users expect non-0 result normally. Some O/R mapping tools
> also checks the result exactly and raises errors (it could be turned
> off, but default is on).
>
If the O/R mapping tool is also creating the table it should be aware of
the semantics specifics to partition. But your comment is well taken,
this seems counterintuitive and against most API semantics to return 0
when the number of inserted rows is expected. This would certainly
require some additional hooks to return the proper value.
Best regards,
Emmanuel
> Regards,
> ---
> ITAGAKI Takahiro
> NTT Open Source Software Center
>
--
Emmanuel Cecchet
FTO @ Frog Thinker
Open Source Development & Consulting
--
Web: http://www.frogthinker.org
email: manu@frogthinker.org
Skype: emmanuel_cecchet
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2008-12-16 17:17 Grzegorz Jaskiewicz <gj@pointblue.com.pl>
parent: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
1 sibling, 0 replies; 59+ messages in thread
From: Grzegorz Jaskiewicz @ 2008-12-16 17:17 UTC (permalink / raw)
To: ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>; +Cc: Emmanuel Cecchet <manu@frogthinker.org>; PostgreSQL-development <pgsql-hackers@postgresql.org>
On 2008-12-16, at 07:58, ITAGAKI Takahiro wrote:
>
>
>>> server says "INSERT 0 row" though rows are inserted into child
>>> tables.
>> Technically this is correct since 0 rows were inserted in the parent
>> table.
>
> Yes, but users expect non-0 result normally. Some O/R mapping tools
> also checks the result exactly and raises errors (it could be turned
> off, but default is on).
this is a general problem with triggers on inserts/updates/deletes. To
be honest, I would love to see someone fixing it in 8.4, cos it is
quite annoying - that developer is unable to figure out number of rows
affected - just because there's trigger on that table.
^ permalink raw reply [nested|flat] 59+ messages in thread
* Re: Fwd: [PATCHES] Auto Partitioning Patch - WIP version 1
@ 2009-01-06 17:19 Devrim GÜNDÜZ <devrim@gunduz.org>
parent: Jaime Casanova <jcasanov@systemguards.com.ec>
2 siblings, 0 replies; 59+ messages in thread
From: Devrim GÜNDÜZ @ 2009-01-06 17:19 UTC (permalink / raw)
To: PostgreSQL Hackers ML <pgsql-hackers@postgresql.org>; +Cc: Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
<snip>
Is there any progress on this patch? I was asked about this feature last
month, during a PostgreSQL talk. I am willing to spend time for testing
this patch, if needed.
--
Devrim GÜNDÜZ, RHCE
devrim~gunduz.org, devrim~PostgreSQL.org, devrim.gunduz~linux.org.tr
http://www.gunduz.org
Attachments:
signature.asc (application/pgp-signature, 196B) download
^ permalink raw reply [nested|flat] 59+ messages in thread
end of thread, other threads:[~2009-01-06 17:19 UTC | newest]
Thread overview: 59+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2007-03-30 06:58 Auto Partitioning Patch - WIP version 1 NikhilS <nikkhils@gmail.com>
2007-04-03 15:36 ` NikhilS <nikkhils@gmail.com>
2007-04-26 23:08 ` Bruce Momjian <bruce@momjian.us>
2008-03-21 14:00 ` Simon Riggs <simon@2ndquadrant.com>
2008-03-21 14:45 ` NikhilS <nikkhils@gmail.com>
2008-03-21 15:18 ` Bruce Momjian <bruce@momjian.us>
2008-03-21 15:53 ` Tom Lane <tgl@sss.pgh.pa.us>
2008-03-22 05:19 ` NikhilS <nikkhils@gmail.com>
2008-03-21 17:01 ` Simon Riggs <simon@2ndquadrant.com>
2008-10-22 14:44 ` Jaime Casanova <jcasanov@systemguards.com.ec>
2008-10-22 21:49 ` Emmanuel Cecchet <manu@frogthinker.org>
2008-10-23 12:41 ` Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
2008-10-31 14:50 ` Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
2008-10-31 18:42 ` Emmanuel Cecchet <manu@frogthinker.org>
2008-11-06 04:47 ` Jaime Casanova <jcasanov@systemguards.com.ec>
2008-11-26 21:35 ` Jaime Casanova <jcasanov@systemguards.com.ec>
2008-11-26 21:52 ` Robert Haas <robertmhaas@gmail.com>
2008-11-27 00:13 ` Jaime Casanova <jcasanov@systemguards.com.ec>
2008-11-27 04:09 ` Robert Haas <robertmhaas@gmail.com>
2008-11-27 09:06 ` Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
2008-11-27 12:04 ` Alvaro Herrera <alvherre@commandprompt.com>
2008-11-27 13:07 ` Robert Haas <robertmhaas@gmail.com>
2008-11-27 13:31 ` Gregory Stark <stark@enterprisedb.com>
2008-11-27 14:41 ` Robert Haas <robertmhaas@gmail.com>
2008-11-27 15:10 ` Jaime Casanova <jcasanov@systemguards.com.ec>
2008-11-27 15:12 ` Jaime Casanova <jcasanov@systemguards.com.ec>
2008-11-27 16:09 ` Emmanuel Cecchet <manu@frogthinker.org>
2008-12-02 22:52 ` Robert Haas <robertmhaas@gmail.com>
2008-12-10 00:51 ` Emmanuel Cecchet <manu@frogthinker.org>
2008-12-15 17:49 ` Josh Berkus <josh@agliodbs.com>
2008-12-15 18:46 ` Gregory Stark <stark@enterprisedb.com>
2008-12-15 19:01 ` Jaime Casanova <jcasanov@systemguards.com.ec>
2008-12-16 02:05 ` Emmanuel Cecchet <manu@frogthinker.org>
2008-12-16 06:35 ` Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
2008-12-16 07:13 ` Emmanuel Cecchet <manu@frogthinker.org>
2008-12-16 07:02 ` ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2008-12-16 07:33 ` Emmanuel Cecchet <manu@frogthinker.org>
2008-12-16 07:58 ` ITAGAKI Takahiro <itagaki.takahiro@oss.ntt.co.jp>
2008-12-16 08:19 ` Emmanuel Cecchet <manu@frogthinker.org>
2008-12-16 17:17 ` Grzegorz Jaskiewicz <gj@pointblue.com.pl>
2008-11-27 16:38 ` Robert Haas <robertmhaas@gmail.com>
2008-11-27 14:01 ` Jaime Casanova <jcasanov@systemguards.com.ec>
2008-11-27 14:47 ` Robert Haas <robertmhaas@gmail.com>
2008-11-27 14:01 ` Nikhil Sontakke <nikhil.sontakke@enterprisedb.com>
2009-01-06 17:19 ` Devrim GÜNDÜZ <devrim@gunduz.org>
2008-03-21 16:53 ` Simon Riggs <simon@2ndquadrant.com>
2008-03-26 00:55 ` Bruce Momjian <bruce@momjian.us>
2008-03-26 12:31 ` Alvaro Herrera <alvherre@commandprompt.com>
2008-03-26 13:34 ` Bruce Momjian <bruce@momjian.us>
2008-03-26 13:42 ` Alvaro Herrera <alvherre@commandprompt.com>
2008-03-26 13:48 ` Bruce Momjian <bruce@momjian.us>
2008-03-26 13:54 ` Alvaro Herrera <alvherre@commandprompt.com>
2008-03-26 14:05 ` Bruce Momjian <bruce@momjian.us>
2008-03-22 05:35 Re: Auto Partitioning Patch - WIP version 1 Luke Lonergan <LLonergan@greenplum.com>
2008-03-26 17:50 Re: Auto Partitioning Patch - WIP version 1 Bruce Momjian <bruce@momjian.us>
2008-03-26 17:57 ` Alvaro Herrera <alvherre@commandprompt.com>
2008-03-26 18:14 ` Bruce Momjian <bruce@momjian.us>
2008-03-26 18:23 ` Bruce Momjian <bruce@momjian.us>
2008-03-26 18:30 ` Alvaro Herrera <alvherre@commandprompt.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox