Re: [PATCHES] Exposing keywords to clients
Hi,
Attached is an updated patch, giving the following output.
Oh, one other thing: dropping externs into random modules unrelated to
their source module is completely awful programming style, because there
is nothing preventing incompatible declarations. Put those externs in
keywords.h instead.
OK.
I suspect you have ignored a compiler warning
about not declaring pg_get_keywords itself, too --- it should be
extern'd in builtins.h.
No, no warning (I'm using VC++ today) - but fixed anyway.
Update attached, including corrected docs. Note to self - proof read
docs *after* putting the kids to bed in future.
Here are some comments from me:
* doc/src/sgml/func.sgml
a) Changed localised to localized to be consistent with the references
elsewhere in the same file.
* src/backend/utils/adt/misc.c
b) I wonder if we need the default case in the switch statement at all,
since we are scanning the statically populated ScanKeywords array with
proper category values for each entry.
c) There was a warning during compilation since we were assigning a const
pointer to a char pointer
values[0] = ScanKeywords[funcctx-call_cntr].name;
* src/include/catalog/pg_proc.h
d) oid 2700 has been claimed by another function in the meanwhile. Modified
it to 2701.
DATA(insert OID = 2701 ( pg_get_keywordsPGNSP PGUID 12 10 400 f f t t s
0 2249
e) I was wondering why pronargs is set to 0 above. But I see other functions
doing the same, so its ok I guess for such kinds of usages.
PFA, version 4 of this patch with a,c and d taken care of.
Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com
Index: doc/src/sgml/func.sgml
===
RCS file: /repositories/postgreshome/cvs/pgsql/doc/src/sgml/func.sgml,v
retrieving revision 1.437
diff -c -w -r1.437 func.sgml
*** doc/src/sgml/func.sgml 19 May 2008 18:08:15 - 1.437
--- doc/src/sgml/func.sgml 3 Jul 2008 07:01:47 -
***
*** 10903,10908
--- 10903,10914
/row
row
+entryliteralfunctionpg_get_keywords/function()/literal/entry
+entrytypesetof record/type/entry
+entrylist of keywords and their categories/entry
+ /row
+
+ row
entryliteralfunctionpg_my_temp_schema/function()/literal/entry
entrytypeoid/type/entry
entryOID of session's temporary schema, or 0 if none/entry
***
*** 11044,11049
--- 11050,11068
/para
indexterm
+ primarypg_get_keywords/primary
+/indexterm
+
+para
+ functionpg_get_keywords/function returns a set of records describing
+ the keywords recognized by the server. The structfieldword/ column
+ contains the keyword and the structfieldcatcode/ column contains a
+ category code of 'U' for unreserved, 'C' for column name, 'T' for type
+ or function name or 'R' for reserved. The structfieldcatdesc/
+ column contains a localized string describing the category.
+/para
+
+indexterm
primarypg_my_temp_schema/primary
/indexterm
Index: src/backend/parser/keywords.c
===
RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/parser/keywords.c,v
retrieving revision 1.197
diff -c -w -r1.197 keywords.c
*** src/backend/parser/keywords.c 21 May 2008 19:51:01 - 1.197
--- src/backend/parser/keywords.c 3 Jul 2008 07:01:47 -
***
*** 41,47
* !!WARNING!!: This list must be sorted by ASCII name, because binary
* search is used to locate entries.
*/
! static const ScanKeyword ScanKeywords[] = {
/* name, value, category */
{abort, ABORT_P, UNRESERVED_KEYWORD},
{absolute, ABSOLUTE_P, UNRESERVED_KEYWORD},
--- 41,47
* !!WARNING!!: This list must be sorted by ASCII name, because binary
* search is used to locate entries.
*/
! const ScanKeyword ScanKeywords[] = {
/* name, value, category */
{abort, ABORT_P, UNRESERVED_KEYWORD},
{absolute, ABSOLUTE_P, UNRESERVED_KEYWORD},
***
*** 428,433
--- 428,436
{zone, ZONE, UNRESERVED_KEYWORD},
};
+ /* End of ScanKeywords, for use elsewhere */
+ const ScanKeyword *LastScanKeyword = endof(ScanKeywords);
+
/*
* ScanKeywordLookup - see if a given word is a keyword
*
Index: src/backend/utils/adt/misc.c
===
RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/utils/adt/misc.c,v
retrieving revision 1.62
diff -c -w -r1.62 misc.c
*** src/backend/utils/adt/misc.c 17 Apr 2008 20:56:41 - 1.62
--- src/backend/utils/adt/misc.c 3 Jul 2008 07:01:47 -
***
*** 20,29
--- 20,31
#include math.h
#include access/xact.h
+ #include catalog/pg_type.h
#include catalog/pg_tablespace.h
#include commands/dbcommands.h
#include funcapi.h
#include miscadmin.h
+ #include parser/keywords.h
#include postmaster
Re: [PATCHES] [EMAIL PROTECTED]: Re: [BUGS] Problem identifying constraints which should not be inherited]
Hi, On Sat, May 10, 2008 at 6:11 AM, Alex Hunsaker [EMAIL PROTECTED] wrote: On Fri, May 9, 2008 at 5:37 PM, Tom Lane [EMAIL PROTECTED] wrote: Alex Hunsaker [EMAIL PROTECTED] writes: [ patch to change inherited-check-constraint behavior ] Applied after rather heavy editorializations. You didn't do very well on getting it to work in multiple-inheritance scenarios, such as create table p (f1 int check (f10)); create table c1 (f2 int) inherits (p); create table c2 (f3 int) inherits (p); create table cc () inherits (c1,c2); Here the same constraint is multiply inherited. The base case as above worked okay, but adding the constraint to an existing inheritance tree via ALTER TABLE, not so much. Ouch. Ok Ill (obviously) review what you committed so I can do a lot better next time. Thanks for muddling through it! Ouchie indeed! I'm not sure if we ought to try to back-patch that --- it'd be a behavioral change with non-obvious implications. In the back branches, ADD CHECK followed by DROP CONSTRAINT will end up not deleting the child-table constraints, which is probably a bug but I wouldn't be surprised if applications were depending on the behavior. Given the lack complaints it does not seem worth a back patch IMHO. Yeah, same IMHO. I do hope we have covered things properly for inherited check constraints by now. One minor thing that myself and Alex discussed was the usage of child tables in tablecmds.c, especially in error messages. Again English is not my native language, but shouldn't that be worded as children tables? Admittedly even this does not sound any better than child tables though :). It is nit-picking really, but I can submit a cleanup patch to reword this if the list thinks so.. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] [EMAIL PROTECTED]: Re: [BUGS] Problem identifying constraints which should not be inherited]
Hi Alex,
On Sun, Mar 30, 2008 at 7:10 AM, Alex Hunsaker [EMAIL PROTECTED] wrote:
(trimmed cc's)
Find attached inherited_constraint_v2.patch
Changes since v1:
-rebased against latest HEAD
-changed enum { Anum_pg_constraint_... } back into #define
Anum_pg_constraint_...
-remove whitespace damage I added
-fixed regression tests I added to be more robust
-fixed
create table ac (a int constraint check_a check (a 0));
create table bc (a int constraint check_a check (a 0)) inherits (ac);
so it properly works (removed crud I put into
AddRelationRawConstraints and created a proper fix in DefineRelation)
I was taking a look at this patch to add the pg_dump related changes. Just
wanted to give you a heads up as this patch crashes if we run make
installcheck. Seems there is an issue introduced in the CREATE TABLE
REFERENCES code path due to your patch (this is without my pg_dump changes
just to be sure). Looks like some memory overwrite issue. The trace is as
follows:
Core was generated by `postgres: nikhils regression [local] CREATE
TABLE '.
Program terminated with signal 11, Segmentation fault.
#0 0x08378024 in AllocSetCheck (context=0xa060368) at aset.c:1112
1112if (dsize 0 dsize chsize
*chdata_end != 0x7E)
(gdb) bt
#0 0x08378024 in AllocSetCheck (context=0xa060368) at aset.c:1112
#1 0x0837704f in AllocSetDelete (context=0xa060368) at aset.c:487
#2 0x083783c2 in MemoryContextDelete (context=0xa060368) at mcxt.c:196
#3 0x083797fb in PortalDrop (portal=0xa0845bc, isTopCommit=0 '\0') at
portalmem.c:448
#4 0x08281939 in exec_simple_query (
query_string=0xa07e564 CREATE TABLE enumtest_child (parent rainbow
REFERENCES enumtest_parent);) at postgres.c:992
#5 0x082857d4 in PostgresMain (argc=4, argv=0x9ffbe28, username=0x9ffbcc4
nikhils) at postgres.c:3550
#6 0x0824917b in BackendRun (port=0xa003180) at postmaster.c:3204
#7 0x082486a2 in BackendStartup (port=0xa003180) at postmaster.c:2827
#8 0x08245e9c in ServerLoop () at postmaster.c:1271
#9 0x082457fd in PostmasterMain (argc=3, argv=0x9ff9c60) at postmaster.c
:1019
#10 0x081e1c03 in main (argc=3, argv=0x9ff9c60) at main.c:188
Regards,
Nikhils
--
EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] Auto Partitioning Patch - WIP version 1
Hi Simon, On Fri, Mar 21, 2008 at 7:30 PM, Simon Riggs [EMAIL PROTECTED] 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
Re: [PATCHES] Auto Partitioning Patch - WIP version 1
Hi, On Fri, Mar 21, 2008 at 9:23 PM, Tom Lane [EMAIL PROTECTED] wrote: Bruce Momjian [EMAIL PROTECTED] 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
Re: [PATCHES] UPDATE using sub selects
Hi Tom, ... eg backend/nodes/, optimizer/util/clauses.c, ruleutils.c...) Actually, on further thought ruleutils.c represents one of the biggest stumbling blocks here. We have to be able to reverse-compile the parse tree into something that's at least semantically equivalent to the original query. The existing kluge for UPDATE SET (columns) = ... can ignore this because it rearranges the query into a sematically equivalent update with independent SET targets, but the whole problem with sub-select updates is that there *is* no semantically equivalent command with a flat targetlist. So one way or another there will have to be more information in the parse tree than there is now. If we use Params that can be traced back to specific SubLink list entries, it might be okay to finesse this by having ruleutils.c check for successive targetlist entries that reference outputs of the same SubLink. That's pretty ugly but it might be better than changing the representation of targetlists, which is something that's understood by one heck of a lot of code. Thanks a lot for taking a look at this patch. As mentioned and detailed out by you, this patch *does* need a lot more amount of work and is certainly not a simpler effort that I had envisioned it to be earlier. Another issue with the patch as it stands today is that it does not work with correlated subqueries. This will require some more work too. So for example, something of this sort does not work yet: UPDATE update_tbl set (a, b) = (select a, b from other_tbl where c = update_tbl.c) where a = 10; I will try to have another shot at it if I can, before the next commit fest. Thanks and Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] [DOCS] Partition: use triggers instead of rules
The argument I made for keeping the example around is not dependent on the assumption that using a rule is a good idea. It's dependent on the established fact that we have recommended that in prior releases, and therefore people are going to be seeing that construct in real databases. And they could refer back to the older version of the documentation for it. In fact, we should mention that in the patch: noteparaIf you have a partitioning setup that uses rules please refer to the 8.2 documentation on partitioning/para/note +1 Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] [DOCS] Partition: use triggers instead of rules
Hi, On Nov 30, 2007 1:52 AM, Gregory Stark [EMAIL PROTECTED] wrote: Joshua D. Drake [EMAIL PROTECTED] writes: If you have lots of data it doesn't mean you are modifying lots of data. It sure can. How do you modify lots of data if you *don't* have lots data in the first place? Certainly it doesn't mean you necessarily are, some databases are OLTP which do no large updates. But data warehouses with oodles of data also often have to do large updates or deletions. I don't think anyone here (good lord I hope not) would say that firing a trigger over 500k rows is fast. Instead you should likely just work the data outside the partition and then move it directly into the target partition. Well you don't even have to do that. You can issue the updates directly against the partitions. In fact, that's precisely what the rules effectively do... Rules rewrite the query to be a query directly against the partitions. Come to think of it I think there actually is a correct way to use rules which wouldn't suffer from the problems that have come up. Instead of putting a WHERE clause on the rule just expand deletes and updates to expand to deletes and updates against *all* partitions. Then let constraint_exclusion kick in to narrow down which partitions should actually receive the updates and deletes. I think triggers are the only solution for insert though. Another reason to go along with triggers is that COPY honors triggers, but does not honor rules. While trying to do bulk inserts into a parent of partitioned tables where rules are being employed, the COPY operation will not be so straightforward. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, Attached is a revised patch that does that. Barring any objections, I'll apply this to HEAD tomorrow. A minor correction below in your patch: --- src/include/commands/defrem.h 16 Jul 2007 05:19:43 - *** extern void DefineIndex(RangeVar *heapRe *** 26,31 --- 26,32 List *options, + char *inhreloptions, Guess you want to use src_options here to be uniform with usages elsewhere that you have replaced. You suppose src_options is much more readable than inhreloptions or inh_idxoptions? Your call :). Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, You suppose src_options is much more readable than inhreloptions or inh_idxoptions? Your call :). Yeah, I'm not necessarily sure that it is. inhreloptions made me think of table inheritance, whereas LIKE in general is more of a source = target copying operation. But I'm not convinced about src_options, either ... suggestions/comments welcome. I agree, since its a LIKE operation and not inheritance as such, how about src_idxoptions, just to make the reference to the source index clearer? To reiterate its a minor nitpick really from my side and can be ignored too. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, This version updates the patch to CVS HEAD and has various fixes and refactoring, including proper docs. I refactored get_opclass_name() into lsyscache.c, but then realized that this means that lsyscache.c will depend on commands/indexcmds.c (for GetDefaultOpClass()), which is arguably improper, so I'm tempted to revert and just duplicate the syscache lookups in both ruleutils.c and parse_utilcmd.c Yes, I too would vote for duplicating the lookups for the sake of simplicity. Nikhil: why are both options and inhreloptions necessary in IndexStmt? Won't at least one be NULL? Yes, in the CREATE..LIKE case, options will be NULL and in the normal CREATE..INDEX case inhreloptions will be NULL. Note that options is a List of DefElem entries, whereas inhreloptions is a char pointer. The challenge was with converting the stored reloptions belonging to the parent index into some form which could be consumed by transformRelOptions(). It was difficult to convert it into a list of DefElem entries and hence I had to introduce inhreloptions. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, Anyways, this patch and the functionality introduced herein will be useful in the CREATE .. INCLUDING INDEXES case too. No doubt. But those are different features and we shouldn't confuse 'em; in particular not put behavior into the INCLUDING CONSTRAINTS case that shouldn't be there. PFA, a patch which provides the CREATE .. INCLUDING INDEXES functionality. This patch uses the same functionality introduced by the earlier patches in this thread albeit for the INCLUDING INDEXES case. For ease of review, this patch is based on the latest version which was posted by Neil. Please let me know if there are any issues. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com Index: doc/src/sgml/ref/create_table.sgml === RCS file: /repositories/postgreshome/cvs/pgsql/doc/src/sgml/ref/create_table.sgml,v retrieving revision 1.108 diff -c -r1.108 create_table.sgml *** doc/src/sgml/ref/create_table.sgml 3 Jun 2007 17:06:03 - 1.108 --- doc/src/sgml/ref/create_table.sgml 5 Jun 2007 09:20:14 - *** *** 263,275 literalINCLUDING CONSTRAINTS/literal is specified; other types of constraints will never be copied. Also, no distinction is made between column constraints and table constraints mdash; when constraints are ! requested, all check constraints are copied. /para para Note also that unlike literalINHERITS/literal, copied columns and constraints are not merged with similarly named columns and constraints. If the same name is specified explicitly or in another ! literalLIKE/literal clause an error is signalled. /para /listitem /varlistentry --- 263,278 literalINCLUDING CONSTRAINTS/literal is specified; other types of constraints will never be copied. Also, no distinction is made between column constraints and table constraints mdash; when constraints are ! requested, all check constraints are copied. literalUNIQUE/literal, ! and literalPRIMARY KEY/literal constraints along with other index ! entries will be copied if literalINCLUDING INDEXES/literal is ! specified. /para para Note also that unlike literalINHERITS/literal, copied columns and constraints are not merged with similarly named columns and constraints. If the same name is specified explicitly or in another ! literalLIKE/literal clause, an error is signalled. /para /listitem /varlistentry Index: src/backend/bootstrap/bootparse.y === RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/bootstrap/bootparse.y,v retrieving revision 1.88 diff -c -r1.88 bootparse.y *** src/backend/bootstrap/bootparse.y 13 Mar 2007 00:33:39 - 1.88 --- src/backend/bootstrap/bootparse.y 5 Jun 2007 09:20:14 - *** *** 252,258 LexIDStr($8), NULL, $10, ! NULL, NIL, false, false, false, false, false, true, false, false); do_end(); --- 252,258 LexIDStr($8), NULL, $10, ! NULL, NIL, NULL, false, false, false, false, false, true, false, false); do_end(); *** *** 270,276 LexIDStr($9), NULL, $11, ! NULL, NIL, true, false, false, false, false, true, false, false); do_end(); --- 270,276 LexIDStr($9), NULL, $11, ! NULL, NIL, NULL, true, false, false, false, false, true, false, false); do_end(); Index: src/backend/commands/indexcmds.c === RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/commands/indexcmds.c,v retrieving revision 1.159 diff -c -r1.159 indexcmds.c *** src/backend/commands/indexcmds.c 3 Jun 2007 17:06:16 - 1.159 --- src/backend/commands/indexcmds.c 5 Jun 2007 09:20:14 - *** *** 100,105 --- 100,106 List *attributeList, Expr *predicate, List *options, + char *inhreloptions, bool unique, bool primary, bool isconstraint, *** *** 393,400 /* * Parse AM-specific options, convert to text array form, validate. */ ! reloptions = transformRelOptions((Datum) 0, options, false, false); (void) index_reloptions(amoptions, reloptions, true); --- 394,408 /* * Parse AM-specific options, convert to text array form, validate. + * The inh reloptions introduced due to using indexes via + * the CREATE LIKE INCLUDING INDEXES statement also need to be merged here */ ! if (inhreloptions) ! reloptions = deflatten_reloptions(inhreloptions); ! else ! reloptions = (Datum) 0; ! ! reloptions = transformRelOptions(reloptions
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, On 6/3/07, Neil Conway [EMAIL PROTECTED] wrote: On Mon, 2007-21-05 at 12:23 +0530, NikhilS wrote: I had spent some time on this earlier so decided to complete and send the patch to you for review. This patch supports copying of expressions, predicates, opclass, amorder, reloptions etc. The test case also contains some more additions with this patch. Please let me know if there are any issues. Attached is a revised version of this patch. Note that this pattern is always unsafe: ht_am = SearchSysCache(AMOID, ...); if (!HeapTupleIsValid(ht_am)) elog(ERROR, ...); amrec = (Form_pg_am) GETSTRUCT(ht_am); index-accessMethod = NameStr(amrec-amname); /* ... */ ReleaseSysCache(ht_am); return index; Before calling ReleaseSysCache(), all the data you need from the syscache entry needs to be deep-copied to allow subsequent access, but NameStr() doesn't do a deep-copy. Adding -DFORCE_CATCACHE_RELEASE is a useful way to catch these kinds of problems (I wonder if this is worth adding to the default CFLAGS when assertions are enabled?) I should have delved deep into the NameStr functionality myself for this. I too agree that adding the flag makes sense when assertions are enabled. I also made a bunch of editorial changes, including moving the varattnos_map_schema() call out of the loop in transformInhRelation(). BTW, comments like This function is based on code from ruleutils.c would be helpful for reviewers (whether in the patch itself or just in the email containing the patch). Yes I had this in mind but somehow forgot to mention this when I posted the patch. I had done some changes in ruleutils.c specificly to take cognizance of this fact. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, On 6/3/07, Tom Lane [EMAIL PROTECTED] wrote: Neil Conway [EMAIL PROTECTED] writes: Attached is a revised version of this patch. This still seems to fundamentally misunderstand the difference between an index and a constraint. IMHO it should not be examining pg_index (or specifically, the index Relations) at all. But as you had mentioned earlier, if we look at index entries as part of the implementation of unique or primary key pg_constraint entries, then examining pg_index is required, right? Anyways, this patch and the functionality introduced herein will be useful in the CREATE .. INCLUDING INDEXES case too. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, The index creation happens after the new table (which is LIKE the parent) has been created, by appending the cxt.alist information to extras_after. The entry point for the index creation is thus via ProcessUtility which expects an IndexStmt structure. That is why the current patch does all the Oid to name mapping exercise to populate the relevant fields in IndexStmt some of which are char pointers. The internal DefineIndex function also expects most of the fields to be in IndexStmt like format. If we want to follow the above suggestion, as I understand it, we might have to devise a new structure to contain Oids and make ProcessUtility accept a new nodeTag. We will also not be able to use existing Index definition functions and this will lead to more coding IMHO. Do we want to go down this path? Or is there something else that has been suggested above and that I am missing completely? OTOH, we can populate a new structure with the relevant Oids, IndexInfo information from parent relation indexes and call index_create directly from within ProcessUtility. Guess, it should be cleaner than the current approach. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, On 5/23/07, NikhilS [EMAIL PROTECTED] wrote: Hi, The index creation happens after the new table (which is LIKE the parent) has been created, by appending the cxt.alist information to extras_after. The entry point for the index creation is thus via ProcessUtility which expects an IndexStmt structure. That is why the current patch does all the Oid to name mapping exercise to populate the relevant fields in IndexStmt some of which are char pointers. The internal DefineIndex function also expects most of the fields to be in IndexStmt like format. If we want to follow the above suggestion, as I understand it, we might have to devise a new structure to contain Oids and make ProcessUtility accept a new nodeTag. We will also not be able to use existing Index definition functions and this will lead to more coding IMHO. Do we want to go down this path? Or is there something else that has been suggested above and that I am missing completely? OTOH, we can populate a new structure with the relevant Oids, IndexInfo information from parent relation indexes and call index_create directly from within ProcessUtility. Guess, it should be cleaner than the current approach. Sorry for the barrage of emails. But as I looked closely at the current patch there are only 2 fields (accessMethod and tableSpace) in IndexStmt structure that we populate by doing the conversion from OIDs to name. For the other fields, the current transformations will remain. If so, I think we can introduce 2 Oid fields in the IndexStmt structure and store the Oids there. In DefineIndex we can use these Oids if they are not invalid. IMHO, all this is less work and the bulk of the changes remain localized in mostly one or two functions as in the current patch. Comments? Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, On 5/23/07, Tom Lane [EMAIL PROTECTED] wrote: NikhilS [EMAIL PROTECTED] writes: If so, I think we can introduce 2 Oid fields in the IndexStmt structure and store the Oids there. In DefineIndex we can use these Oids if they are not invalid. I think this is just make-work that causes the patch to complicate parts of the system it didn't need to touch. The original suggestion was to actively refactor existing code, which might or might not have been worthwhile. But this isn't an appropriate substitute --- it's just making the API uglier for no particular benefit. I agree this will unnecessary add arguments to the DefineIndex API. If we stick to the patch's earlier way of converting the Oid to names for just these 2 arguments, we can avoid this IMO. Considering that we will be generating this information from existing valid index information, I think converting the Oids to names is safe enough. Alvaro, do you think we should stick to the existing patch mechanism then considering that it avoids polluting the API? Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, I agree this will unnecessary add arguments to the DefineIndex API. If we stick to the patch's earlier way of converting the Oid to names for just these 2 arguments, we can avoid this IMO. Considering that we will be generating this information from existing valid index information, I think converting the Oids to names is safe enough. Alvaro, do you think we should stick to the existing patch mechanism then considering that it avoids polluting the API? Not sure. Is it possible that the schema is renamed while the operation is being executed? If it's not then this not a problem at all so the existing patch is fine. I doubt if accessMethod name will change. The tableSpace name can change, but the possibility is no worse to doing a [CREATE TABLE table_name ... TABLESPACE tablespace]. So this should be reasonably ok. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi Neil, On 5/18/07, Neil Conway [EMAIL PROTECTED] wrote: On Mon, 2007-14-05 at 22:58 -0400, Neil Conway wrote: Has a revised version of this patch been submitted? In the absence of a revised patch, I can finish the feature myself, but I won't get the free cycles until after PGCon. I can commit to getting it done before the end of May, or else we can just push this to 8.4. I had spent some time on this earlier so decided to complete and send the patch to you for review. This patch supports copying of expressions, predicates, opclass, amorder, reloptions etc. The test case also contains some more additions with this patch. Please let me know if there are any issues. Also, if this patch is acceptable, I think the mechanism provided here can be used to support INCLUDING INDEXES case easily too. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com Index: doc/src/sgml/ref/create_table.sgml === RCS file: /repositories/postgreshome/cvs/pgsql/doc/src/sgml/ref/create_table.sgml,v retrieving revision 1.107 diff -c -r1.107 create_table.sgml *** doc/src/sgml/ref/create_table.sgml 1 Feb 2007 00:28:18 - 1.107 --- doc/src/sgml/ref/create_table.sgml 21 May 2007 06:50:34 - *** *** 259,269 /para para Not-null constraints are always copied to the new table. ! literalCHECK/literal constraints will only be copied if ! literalINCLUDING CONSTRAINTS/literal is specified; other types of ! constraints will never be copied. Also, no distinction is made between ! column constraints and table constraints mdash; when constraints are ! requested, all check constraints are copied. /para para Note also that unlike literalINHERITS/literal, copied columns and --- 259,268 /para para Not-null constraints are always copied to the new table. ! literalCHECK, UNIQUE, and PRIMARY KEY/literal constraints will only ! be copied if literalINCLUDING CONSTRAINTS/literal is specified. Also, ! no distinction is made between column constraints and table constraints ! mdash; when constraints are requested, all check constraints are copied. /para para Note also that unlike literalINHERITS/literal, copied columns and Index: src/backend/bootstrap/bootparse.y === RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/bootstrap/bootparse.y,v retrieving revision 1.88 diff -c -r1.88 bootparse.y *** src/backend/bootstrap/bootparse.y 13 Mar 2007 00:33:39 - 1.88 --- src/backend/bootstrap/bootparse.y 21 May 2007 06:50:34 - *** *** 252,258 LexIDStr($8), NULL, $10, ! NULL, NIL, false, false, false, false, false, true, false, false); do_end(); --- 252,258 LexIDStr($8), NULL, $10, ! NULL, NIL, NULL, false, false, false, false, false, true, false, false); do_end(); *** *** 270,276 LexIDStr($9), NULL, $11, ! NULL, NIL, true, false, false, false, false, true, false, false); do_end(); --- 270,276 LexIDStr($9), NULL, $11, ! NULL, NIL, NULL, true, false, false, false, false, true, false, false); do_end(); Index: src/backend/commands/indexcmds.c === RCS file: /repositories/postgreshome/cvs/pgsql/src/backend/commands/indexcmds.c,v retrieving revision 1.158 diff -c -r1.158 indexcmds.c *** src/backend/commands/indexcmds.c 2 May 2007 21:08:45 - 1.158 --- src/backend/commands/indexcmds.c 21 May 2007 06:50:34 - *** *** 100,105 --- 100,106 List *attributeList, Expr *predicate, List *options, + char *inhreloptions, bool unique, bool primary, bool isconstraint, *** *** 393,400 /* * Parse AM-specific options, convert to text array form, validate. */ ! reloptions = transformRelOptions((Datum) 0, options, false, false); (void) index_reloptions(amoptions, reloptions, true); --- 394,408 /* * Parse AM-specific options, convert to text array form, validate. + * The inh reloptions introduced due to using unique/primary indexes via + * the CREATE LIKE INCLUDING CONSTRAINTS statement also need to be merged here */ ! if (inhreloptions) ! reloptions = deflatten_reloptions(inhreloptions); ! else ! reloptions = (Datum) 0; ! ! reloptions = transformRelOptions(reloptions, options, false, false); (void) index_reloptions(amoptions, reloptions, true); Index: src/backend/commands/tablecmds.c === RCS
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, Since this patch is going to consider creating unique/primary indexes assuming them to be constraints, If it does that it will be rejected. There is a difference here and that difference has to be maintained. The correct way to think about this is that a pg_constraint entry of type unique or primary key has an associated index that is part of its implementation (and therefore has an internal dependency on the constraint). But they are far from being the same thing. Thanks Tom, I understand the difference now. I have a working patch and will send it to Neil for review tommorrow. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, [ remembering previous discussions more clearly... ] Actually there is a concrete problem here: unique constraints are supposed to be represented in the information_schema views, and there is no spec-compliant way to do that for a constraint on something other than a column. We'd have to guess at what the SQL committee would do about that, and the odds of guessing exactly right don't seem encouraging. Considering that a unique index is a unique constraint, then isn't allowing expressional unique indexes contradictory to the above? It seems that CREATE UNIQUE INDEX currently does not pass isconstraint as true to DefineIndex, otherwise index_create() would have cribbed with: constraints cannot have index expressions error. Since this patch is going to consider creating unique/primary indexes assuming them to be constraints, IMHO we should be uniform about unique constraints semantics. That might mean that we only support expressionless, non-predicate indexes via INCLUDING CONSTRAINTS? Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, Nope: neilc=# create table t1 (a int, b int); CREATE TABLE neilc=# create unique index t1_a_idx on t1 ((a + b)) where (a 5); CREATE INDEX I just now realized that even though we allow the above. We do not allow: pg=# create table t1 (a int, b int, unique(a+b)); nor the where clause syntax. Any specific reason for this behaviour? If we want to pass such kinds of expr, predicate based constraints via the LIKE .. INCLUDING CONSTRAINTS statements, transformIndexConstraints as well as the Constraint structure might need modifications. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, On 5/3/07, Tom Lane [EMAIL PROTECTED] wrote: NikhilS [EMAIL PROTECTED] writes: Hi Neil, * the patch is broken for expressional indexes, and silently omits copying the predicate that may be associated with an index. Since this patch is only supposed to copy unique/primary indexes, I dont think we will ever have predicates associated to such indexes? Huh? I would expect a clause INCLUDING INDEXES to mean copying *all* indexes. A clause INCLUDING CONSTRAINTS would reasonably act as you suggest, ie copy only indexes derived from SQL constraint clauses. But this patch is not for the INCLUDING INDEXES case. As mentioned by Bruce earlier in this thread, this patch is for the following TODO: o Have WITH CONSTRAINTS also create constraint indexes http://archives.postgresql.org/pgsql-patches/2007-04/msg00149.php Regards, Nikhils regards, tom lane -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi Neil, * the patch is broken for expressional indexes, and silently omits copying the predicate that may be associated with an index. It also doesn't copy the index's amoptions (WITH clause), or the NULLS FIRST/etc. options that may be associated with any of the index's columns. Since this patch is only supposed to copy unique/primary indexes, I dont think we will ever have predicates associated to such indexes? Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi Trevor, + + parent_index_info = BuildIndexInfo(parent_index); The above is not used anywhere else in the code and seems redundant. + + ereport(NOTICE, + (errmsg(Index \%s\ cloned., + RelationGetRelationName(parent_index; DefineIndex will give out a message anyways for unique/primary keys. The above seems additional to it. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
[PATCHES] UPDATE using sub selects
Hi, As per discussion on -hackers, a patch which allows updates to use subselects is attached with this mail. As per discussion with Tom, I have adopted the following approach: * Introduce ROWEXPR_SUBLINK type for subqueries that allows multiple column outputs. * Populate the targetList with PARAM_SUBLINK entries dependent on the subselects. * Modify the targets in-place into PARAM_EXEC entries in the make_subplan phase. The above does not require any kluges in the targetList processing code path at all. UPDATEs seem to work fine using subselects with this patch. I have modified the update.sql regression test to include possible variations . No documentation changes are present in this patch. Feedback, comments appreciated. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com ? GNUmakefile ? config.log ? config.status ? cscope.out ? src/Makefile.global ? src/backend/postgres ? src/backend/catalog/postgres.bki ? src/backend/catalog/postgres.description ? src/backend/catalog/postgres.shdescription ? src/backend/utils/mb/conversion_procs/conversion_create.sql ? src/backend/utils/mb/conversion_procs/ascii_and_mic/libascii_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/cyrillic_and_mic/libcyrillic_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/euc_cn_and_mic/libeuc_cn_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/euc_jis_2004_and_shift_jis_2004/libeuc_jis_2004_and_shift_jis_2004.so.0.0 ? src/backend/utils/mb/conversion_procs/euc_jp_and_sjis/libeuc_jp_and_sjis.so.0.0 ? src/backend/utils/mb/conversion_procs/euc_kr_and_mic/libeuc_kr_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/euc_tw_and_big5/libeuc_tw_and_big5.so.0.0 ? src/backend/utils/mb/conversion_procs/latin2_and_win1250/liblatin2_and_win1250.so.0.0 ? src/backend/utils/mb/conversion_procs/latin_and_mic/liblatin_and_mic.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_ascii/libutf8_and_ascii.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_big5/libutf8_and_big5.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_cyrillic/libutf8_and_cyrillic.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_euc_cn/libutf8_and_euc_cn.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_euc_jis_2004/libutf8_and_euc_jis_2004.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_euc_jp/libutf8_and_euc_jp.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_euc_kr/libutf8_and_euc_kr.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_euc_tw/libutf8_and_euc_tw.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_gb18030/libutf8_and_gb18030.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_gbk/libutf8_and_gbk.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_iso8859/libutf8_and_iso8859.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_iso8859_1/libutf8_and_iso8859_1.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_johab/libutf8_and_johab.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_shift_jis_2004/libutf8_and_shift_jis_2004.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_sjis/libutf8_and_sjis.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_uhc/libutf8_and_uhc.so.0.0 ? src/backend/utils/mb/conversion_procs/utf8_and_win/libutf8_and_win.so.0.0 ? src/bin/initdb/initdb ? src/bin/ipcclean/ipcclean ? src/bin/pg_config/pg_config ? src/bin/pg_controldata/pg_controldata ? src/bin/pg_ctl/pg_ctl ? src/bin/pg_dump/pg_dump ? src/bin/pg_dump/pg_dumpall ? src/bin/pg_dump/pg_restore ? src/bin/pg_resetxlog/pg_resetxlog ? src/bin/psql/psql ? src/bin/scripts/clusterdb ? src/bin/scripts/createdb ? src/bin/scripts/createlang ? src/bin/scripts/createuser ? src/bin/scripts/dropdb ? src/bin/scripts/droplang ? src/bin/scripts/dropuser ? src/bin/scripts/reindexdb ? src/bin/scripts/vacuumdb ? src/include/pg_config.h ? src/include/stamp-h ? src/interfaces/ecpg/compatlib/libecpg_compat.so.2.3 ? src/interfaces/ecpg/ecpglib/libecpg.so.5.3 ? src/interfaces/ecpg/include/ecpg_config.h ? src/interfaces/ecpg/pgtypeslib/libpgtypes.so.2.3 ? src/interfaces/ecpg/preproc/.pgc.c.swp ? src/interfaces/ecpg/preproc/ecpg ? src/interfaces/libpq/exports.list ? src/interfaces/libpq/libpq.so.5.1 ? src/pl/plpgsql/src/libplpgsql.so.1.0 ? src/port/pg_config_paths.h ? src/test/regress/libregress.so.0.0 ? src/test/regress/pg_regress ? src/test/regress/results ? src/test/regress/testtablespace ? src/test/regress/expected/constraints.out ? src/test/regress/expected/copy.out ? src/test/regress/expected/create_function_1.out ? src/test/regress/expected/create_function_2.out ? src/test/regress/expected/largeobject.out ? src/test/regress/expected/largeobject_1.out ? src/test/regress/expected/misc.out ? src/test/regress/expected/tablespace.out ? src/test/regress/sql/constraints.sql ? src/test/regress/sql/copy.sql ? src/test/regress/sql/create_function_1.sql ? src/test/regress/sql/create_function_2.sql ? src/test/regress/sql/largeobject.sql ? src/test/regress/sql/misc.sql ? src/test/regress
Re: [PATCHES] CREATE TABLE LIKE INCLUDING INDEXES support
Hi, On 4/10/07, Bruce Momjian [EMAIL PROTECTED] wrote: Added to TODO: o Have WITH CONSTRAINTS also create constraint indexes http://archives.postgresql.org/pgsql-patches/2007-04/msg00149.php Trevor's patch does add unique/primary indexes. This would mean that we have to remove the syntax support for INCLUDING INDEXES and just add code to the existing WITH CONSTRAINTs code path from his patch. Is there something else and hence we have the above TODO? Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
[PATCHES] Auto Partitioning Patch - WIP version 1
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
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 - 1.218
--- src/backend/commands/tablecmds.c 30 Mar 2007 06:31:37 -
***
*** 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 - 1.372
--- src/backend/nodes/copyfuncs.c 30 Mar 2007 06:31:37 -
***
*** 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
Re: [PATCHES] log_autovacuum
Hi, On 3/9/07, Simon Riggs [EMAIL PROTECTED] wrote: On Thu, 2007-03-08 at 14:53 -0300, Alvaro Herrera wrote: Maybe something like this is better: LOG: index passes: 1 pages: removed 0, 197 remain tuples: removed 7199, 2338 remain CPU usage: whatever CONTEXT: Automatic vacuuming of table database.public.w Yours is better. I've implemented this: LOG: autovac public.w index passes: 1 pages: removed 0, 197 remain tuples: removed 7199, 2338 remain CPU usage: whatever I'm happy if this gets removed later, but I think it will help everybody understand how multi-vacuums are working and what the best way to specify the controls should be. Not sure about the CONTEXT bit. I think its verbose, plus I thought that was for ERRORs only. I will defer on this point, since I know y'all understand that better than I. IMHO, it would be good to have both the messages spit out. The earlier message is much better for parsing and the later makes READABLE sense. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] Recalculating OldestXmin in a long-running vacuum
Hi, I was wondering if we can apply the same logic of recalculating OldestXmin within IndexBuildHeapScan which occurs as part of create index operation? Having to index lesser number of tuples should be a good save in the CREATE INDEX CONCURRENTLY case, if the above is possible? Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] New features for pgbench
Hi, Right now when you run pgbench, the results vary considerably from run to run even if you completely rebuild the database every time. I've found that a lot of that variation comes from two things: The main purpose of pgbench runs is an apples to apples comparison of 2 source bases. One pristine Postgresql source base and another base being the same source patched with supposed enhancements. As long as we use the same postgresql.conf, same hardware environment and exactly same parameter pgbench runs, the difference in the TPS values observed between the 2 sources should be a good enough indicator as to the viability of the new code, dont you think? E.g. autovacuum will trigger on certain tables only if the threshold is over the limit. So that gets tied in to the update rate. The shared_buffers will become a bottleneck only if the code and the run is I/O intensive enough etc. IMHO, as long as the same environment holds true for both the source base runs, we should not see unexplained variations as per the reasons you have mentioned in the observed TPS values. Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
Re: [PATCHES] New features for pgbench
Hi, On 2/12/07, Greg Smith [EMAIL PROTECTED] wrote: The attached adds two new command line switches to pgbench: -x: Generate extended detail in the latency log, including a timestamp for each transaction From your patch I see that it augments the -l flag. IMHO it does not make sense to add another flag. We can save the if check and log the extended contents as part of -l itself. -X: Do extra cleanup after the run (vacuum on all tables, checkpoint) before stopping the clock. This gives substantially more consistancy in results between runs. I am sorry, but I do not understand the above. If I read it correctly, are you suggesting that the same database with a prior pgbench run be used for further pgbench runs? How is it useful? How can one guarantee consistency of observed tps values with this in place? Regards, Nikhils -- EnterpriseDB http://www.enterprisedb.com
