Re: [HACKERS] pg_dump insert with column names speedup
On Sat, Oct 5, 2013 at 6:39 AM, Stephen Frost wrote: > * Robert Haas (robertmh...@gmail.com) wrote: > > On Fri, Oct 4, 2013 at 11:28 AM, Tom Lane wrote: > > > David Rowley writes: > > >> Here's a small patch which greatly increases the speed of > > >> pg_dump --column-inserts. > > > > > > The reason why no one's paid any attention to the speed of that code > path > > > is that if you care about dump/restore speed, you should be using the > COPY > > > code paths instead. Is it really worth adding code and complexity to > > > pg_dump for this? > > > > One possible reason to care about this is if you're trying to move > > data to another database. The INSERT format is more portable. > > > > Also, this isn't really adding any net code or complexity AFAICS. > > Agreed- this looks more like a "gee, that makes a lot of sense" than a > "wow, that's way more complicated". Not a whole lot of point in > building up a known-to-be-constant string on every iteration of the > loop. > > These words made me think that the changes I made were not quite enough to satisfy this what you said. I understand that most people won't use the --column-inserts feature, but that's not really a great reason to have not very clever and wasteful code in there. This fruit was so low hanging it was pretty much touching the ground, so I couldn't resist fixing it when I saw it. The attached revised patch goes a little further and prepares everything that is constant on processing the first row, this now includes the "INSERT INTO tablename " part. I don't think the changes make the code any harder to read, the code which builds the staticStmt fits into my small laptop screen. The timings with my benchmark look something like: Unpatched: 9200 ms Version 0.1: 5700 ms Version 0.2: 5250 ms So it does shave off a bit more, for what it's worth. David > Thanks, > > Stephen > pg_dump_colinsert_v0.2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] ToDo: fast update of arrays with fixed length fields for PL/pgSQL
Hello I am sending little bit cleaned patch there is significant speedup (on example from bug report) - more than 100x on my Dell D830 postgres=# select fill_2d_array(300,300,1); fill_2d_array ─── 9 (1 row) Time: 751.572 ms -- patched postgres=# \q bash-4.1$ psql postgres psql (9.4devel) Type "help" for help. postgres=# select fill_2d_array(300,300,2); fill_2d_array ─── 9 (1 row) Time: 87453.351 ms -- original I checked a result and it is same. Probably there are some issues, because domain tests fails, but these numbers shows so a significantly faster array update is possible. Interesting - I did a profiling and I didn't find anything interesting :( Regards Pavel 2013/10/3 Tom Lane > Pavel Stehule writes: > > If you can do a update of some array in plpgsql now, then you have to > work > > with local copy only. It is a necessary precondition, and I am think it > is > > valid. > > If the proposal only relates to assignments to elements of plpgsql local > variables, it's probably safe, but it's also probably not of much value. > plpgsql has enough overhead that I'm doubting you'd get much real-world > speedup. I'm also not very excited about putting even more low-level > knowledge about array representation into plpgsql. > > regards, tom lane > fast-array-update.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] pg_upgrade: support for btrfs copy-on-write clones
03.10.2013 01:35, Larry Rosenman kirjoitti: > On 2013-10-02 17:32, Josh Berkus wrote: >>> No fundamental reason; I'm hoping ZFS will be supported in addition to >>> btrfs, but I don't have any systems with ZFS filesystems at the moment >>> so I haven't been able to test it or find out the mechanisms ZFS uses >>> for cloning. On btrfs cloning is implemented with a custom >>> btrfs-specific ioctl, ZFS probably has something similar which would be >>> pretty easy to add on top of this patch. >> >> Would you like a VM with ZFS on it? I'm pretty sure I can supply one. >> > I can also supply SSH access to a FreeBSD 10 system that is totally ZFS. Thanks for the offers, but it looks like ZFS doesn't actually implement a similar file level clone operation. See https://github.com/zfsonlinux/zfs/issues/405 for discussion on a feature request for it. ZFS does support cloning entire datasets which seem to be similar to btrfs subvolume snapshots and could be used to set up a new data directory for a new $PGDATA. This would require the original $PGDATA to be a dataset/subvolume of its own and quite a bit different logic (than just another file copy method in pg_upgrade) to initialize the new version's $PGDATA as a snapshot/clone of the original. The way this would work is that the original $PGDATA dataset/subvolume gets cloned to a new location after which we move the files out of the way of the new PG installation and run pg_upgrade in link mode. I'm not sure if there's a good way to integrate this into pg_upgrade or if it's just something that could be documented as a fast way to run pg_upgrade without touching original files. With btrfs tooling the sequence would be something like this: btrfs subvolume snapshot /srv/pg92 /srv/pg93 mv /srv/pg93/data /srv/pg93/data92 initdb /data/pg93/data pg_upgrade --link \ --old-datadir=/data/pg93/data92 \ --new-datadir=/data/pg93/data / Oskari -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] mvcc catalo gsnapshots and TopTransactionContext
On 2013-10-04 15:15:36 -0400, Robert Haas wrote: > Andres, are you (or is anyone) going to try to fix this assertion failure? I think short term replacing it by IsTransactionOrTransactionBlock() is the way to go. Entirely restructuring how cache invalidation in the abort path works is not a small task. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] mvcc catalo gsnapshots and TopTransactionContext
On Fri, Aug 9, 2013 at 11:17 PM, Andres Freund wrote: > On 2013-08-09 14:11:46 -0400, Tom Lane wrote: >> Andres Freund writes: >> > On 2013-08-08 09:27:24 -0400, Robert Haas wrote: >> >> How can it be safe to try to read catalogs if the transaction is aborted? >> >> > Well. It isn't. At least not in general. The specific case triggered >> > here though are cache invalidations being processed which can lead to >> > the catalog being read (pretty crummy, but not easy to get rid >> > of). That's actually safe since before we process the invalidations we >> > have done: >> > 1) CurrentTransactionState->state = TRANS_ABORT >> > 2) RecordTransactionAbort(), marking the transaction as aborted in the >> > clog >> > 3) marked subxacts as aborted >> > 3) ProcArrayEndTransaction() (for toplevel ones) >> >> > Due to these any tqual stuff will treat the current (sub-)xact and it's >> > children as aborted. So the catalog lookups will use the catalog in a >> > sensible state. >> >> I don't have any faith in this argument. You might be right that we'll >> correctly see our own output rows as aborted, but that's barely the tip >> of the iceberg of risk here. Is it safe to take new locks in an aborted >> transaction? (What if we're already past the lock-release point in >> the abort sequence?) > > Don't get me wrong. I find the idea of doing catalog lookup during abort > horrid. But it's been that way for at least 10 years (I checked 7.4), so > it has at least some resemblance of working. > Today we do a good bit less than back then, for one we don't do a full > cache reload during abort anymore, just for the index support > infrastructure. Also, you've reduced the amount of lookups a bit with the > relmapper introduction. > >> For that matter, given that we don't know what >> exactly caused the transaction abort, how safe is it to do anything at >> all --- we might for instance be nearly out of memory. If the catalog >> reading attempt itself fails, won't we be in an infinite loop of >> transaction aborts? > > Looks like that's possible, yes. There seem to be quite some other > opportunities for this to happen if you look at the amount of work done > in AbortSubTransaction(). I guess it rarely happens because we > previously release some memory... > >> I could probably think of ten more risks if I spent a few more minutes >> at it. > > No need to convince me here. I neither could believe we were doing this, > nor figure out why it even "works" for the first hour of looking at it. > >> Cache invalidation during abort should *not* lead to any attempt to >> immediately revalidate the cache. No amount of excuses will make that >> okay. I have not looked to see just what the path of control is in this >> particular case, but we need to fix it, not paper over it. > > I agree, although that's easier said than done in the case of > subtransactions. The problem we have there is that it's perfectly valid > to still have references to a relation from the outer, not aborted, > transaction. Those need to be valid for anybody looking at the relcache > entry after we've processed the ROLLBACK TO/... > > I guess the fix is something like we do in the commit case, where we > transfer invalidations to the parent transaction. If we then process > local invalidations *after* we've cleaned up the subtransaction > completely we should be fine. We already do an implicity > CommandCounterIncrement() in CommitSubTransaction()... Andres, are you (or is anyone) going to try to fix this assertion failure? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reasons to not move pgstattuple to core?
On Fri, Oct 4, 2013 at 6:44 AM, Robert Haas wrote: > I think we were going to try to group the extensions into categories > (debugging tools, demonstration code, data types, etc.) and maybe > encourage packagers to put the debugging tools in the same OS package > as the core server. I was thinking more about different extension tiers, grouping them by quality/usefulness. -- Peter Geoghegan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] insert throw error when year field len > 4 for timestamptz datatype
On Fri, Oct 4, 2013 at 10:19:38AM +, Haribabu kommi wrote: > > On 03 October 2013 19:30 Bruce Momjian wrote: > >On Thu, Oct 3, 2013 at 11:54:14AM +0530, Rushabh Lathia wrote: > >> Thanks Bruce. > >> > >> Yes for me main problem was to make assumption that a 5-digit number > >> is a year, as was bit worried about side effect of that assumption in > >> the date/time module. I did tested patch shared by you with various > >> test and so far it looks good to me. > >> > >> I would like reviewer to review/test the patch and share his comments. > >> > >> Attaching the git patch again with this mail. > >> > >> Assigning to Reviewer. > > >Oh, great. If everyone likes it I can apply it. > > With Year length of 6 digits has some inconsistency problem, > The tests are carried out on a default configuration. The general limitation we have is that while we know 5-digit numbers can't be YMD or HMS, we don't know that for 6-digit values, so we require that the string contain _a_ date and _a_ time specification before we consider a six-digit number as a year. I don't see how we can do any better than that. Your results below show that behavior. Do you have a suggestion for improvement? --- > select timestamptz '199910108 01:01:01 IST'; -- works > select timestamptz '19991 01 08 01:01:01 IST'; -- works > select timestamptz '1999100108 01:01:01 IST'; -- works > select timestamptz '199910 01 08 01:01:01 IST'; -- Not working > > select timestamptz 'January 8, 19991 01:01:01 IST'; -- works > select timestamptz 'January 8, 199910 01:01:01 IST'; -- Not working > > CREATE TABLE TIMESTAMPTZ_TST (a int , b timestamptz); > INSERT INTO TIMESTAMPTZ_TST VALUES(1, '10312 23:58:48 IST'); -- works > INSERT INTO TIMESTAMPTZ_TST VALUES(2, '1 03 12 23:58:48 IST'); -- works > INSERT INTO TIMESTAMPTZ_TST VALUES(3, '100312 23:58:48 IST'); -- works > INSERT INTO TIMESTAMPTZ_TST VALUES(4, '10 03 12 23:58:48 IST'); -- Not > working > > please correct me if anything wrong in the tests. > > Regards, > Hari babu. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + It's impossible for everything to be true. + -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Suggestion: Issue warning when calling SET TRANSACTION outside transaction block
On Fri, Oct 4, 2013 at 09:40:38AM +0530, Amit Kapila wrote:
> On Thu, Oct 3, 2013 at 8:35 PM, Andres Freund wrote:
> > On 2013-09-30 22:19:31 -0400, Bruce Momjian wrote:
> >> On Sun, Sep 29, 2013 at 11:40:51AM +0530, Amit Kapila wrote:
> >> > >> Shouldn't we do it for Set Constraints as well?
> >> > >
> >> > > Oh, very good point. I missed that one. Updated patch attached.
> >>
> >> I am glad you are seeing things I am not. :-)
> >>
> >> > 1. The function set_config also needs similar functionality, else
> >> > there will be inconsistency, the SQL statement will give error but
> >> > equivalent function set_config() will succeed.
> >> >
> >> > SQL Command
> >> > postgres=# set local search_path='public';
> >> > ERROR: SET LOCAL can only be used in transaction blocks
> >> >
> >> > Function
> >> > postgres=# select set_config('search_path', 'public', true);
> >> > set_config
> >> >
> >> > public
> >> > (1 row)
> >>
> >> I looked at this but could not see how to easily pass the value of
> >> 'isTopLevel' down to the SELECT. All the other checks have isTopLevel
> >> passed down from the utility case statement.
> >
> > Doesn't sound like a good idea to prohibit that anyway, it might
> > intentionally be used as part of a more complex statement where it only
> > should take effect during that single statement.
>
>Agreed and I think it is good reason for not changing behaviour of
> set_config().
Applied. Thank you for all your suggestions.
--
Bruce Momjian http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump insert with column names speedup
* Robert Haas (robertmh...@gmail.com) wrote: > On Fri, Oct 4, 2013 at 11:28 AM, Tom Lane wrote: > > David Rowley writes: > >> Here's a small patch which greatly increases the speed of > >> pg_dump --column-inserts. > > > > The reason why no one's paid any attention to the speed of that code path > > is that if you care about dump/restore speed, you should be using the COPY > > code paths instead. Is it really worth adding code and complexity to > > pg_dump for this? > > One possible reason to care about this is if you're trying to move > data to another database. The INSERT format is more portable. > > Also, this isn't really adding any net code or complexity AFAICS. Agreed- this looks more like a "gee, that makes a lot of sense" than a "wow, that's way more complicated". Not a whole lot of point in building up a known-to-be-constant string on every iteration of the loop. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] pg_dump insert with column names speedup
On Fri, Oct 4, 2013 at 11:28 AM, Tom Lane wrote: > David Rowley writes: >> Here's a small patch which greatly increases the speed of >> pg_dump --column-inserts. > > The reason why no one's paid any attention to the speed of that code path > is that if you care about dump/restore speed, you should be using the COPY > code paths instead. Is it really worth adding code and complexity to > pg_dump for this? One possible reason to care about this is if you're trying to move data to another database. The INSERT format is more portable. Also, this isn't really adding any net code or complexity AFAICS. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_dump insert with column names speedup
David Rowley writes: > Here's a small patch which greatly increases the speed of > pg_dump --column-inserts. The reason why no one's paid any attention to the speed of that code path is that if you care about dump/restore speed, you should be using the COPY code paths instead. Is it really worth adding code and complexity to pg_dump for this? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] bgworker doc typo fixes
On Fri, Oct 4, 2013 at 7:48 AM, Maciek Sakrejda wrote: > I noticed a couple of missing words (at least to my reading) in the bgworker > doc. This changes "...is started including the module name..." to "...is > started by including the module name..." and "...to obtain information the > status of the worker." to "...to obtain information regarding the status of > the worker." Thanks, committed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On Fri, Oct 4, 2013 at 10:14 AM, Andres Freund wrote: > But that's not a new problem? It already exists and isn't really > excerbated by this. ... > I agree that we could use some more infrastructure around configuration, > but I fail to understand why it's this patch's duty to deliver it. And I > don't see why this patch would endanger any more groundbreaking > improvements. You keep saying the ship has already sailed, but I think that's inaccurate. IMHO, we haven't committed to anything in this area as a matter of policy; I think the lack of a policy is demonstrated by the inconsistencies you point out. Now, if we are already committed to a policy of supporting the use case you're targeting with this patch, then you're right: this is just a trivial bug fix, and we ought to just take it for what it is and fix whatever other issues come up later. But if we're not committed to such a policy, then "support multi-level GUCs" is a new feature, and it's entirely right to think that, just like any other new feature, it needs to implement that feature completely rather than piecemeal. We know from experience that when certain features (e.g. hash indexes) are implemented incompletely, the resulting warts can remain behind more or less indefinitely. As I read the thread, Amit Kapila is in favor of your proposal. Pavel Stehule, Alvaro Herrera, and I all questioned whether multi-level GUCs were a good idea; at least 2 out of the 3 of us favor not committing the patch out of uncertainty that we wish to be on the hook to support such usages. Andrew Dunstan and Tom Lane agreed that the current behavior was inconsistent but neither clearly endorsed relaxing the checks in guc-file.l; in fact, Tom suggested tightening up SET instead. Not one person argued that multi-level GUCs were already a supported feature and that this patch was just plugging a gap in that feature; the documentation also disagrees with that interpretation. So I just don't think we have consensus that this is already the policy or that it is a policy we should adopt. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] 9.4 HEAD: select() failed in postmaster
From: "Alvaro Herrera" >Can you please send a fixup patch to what's already committed? OK, I'll send a patch against HEAD, which will be a few lines. Am I understanding the meaning of "fixup patch"? Yep, thanks. Please use the attached patch. Thanks. Regards MauMau reliable_immediate_shutdown_fixup.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_dump insert with column names speedup
Here's a small patch which greatly increases the speed of pg_dump --column-inserts. Previous to this patch a string was being build for the names of the columns for each row in the table... I've now changed this to only do this once for the table. I also did some clean ups to remove calling the va_args function to write to the backup file when it was not needed, it instead now uses the puts type function which should be faster. Here are some benchmark results on how it performs: *** patched *** D:\9.4\bin>timer pg_dump --column-inserts -f d:\postgres_backup.sql postgres Ran for 5850 ms D:\9.4\bin>timer pg_dump --column-inserts -f d:\postgres_backup.sql postgres Ran for 5790 ms D:\9.4\bin>timer pg_dump --column-inserts -f d:\postgres_backup.sql postgres Ran for 5700 ms D:\9.4\bin>timer pg_dump --column-inserts -f d:\postgres_backup.sql postgres Ran for 5760 ms D:\9.4\bin>timer pg_dump --column-inserts -f d:\postgres_backup.sql postgres Ran for 5730 ms *** unpatched *** D:\9.4\bin>timer pg_dump --column-inserts -f d:\postgres_backup.sql postgres Ran for 9580 ms D:\9.4\bin>timer pg_dump --column-inserts -f d:\postgres_backup.sql postgres Ran for 9330 ms D:\9.4\bin>timer pg_dump --column-inserts -f d:\postgres_backup.sql postgres Ran for 9250 ms D:\9.4\bin>timer pg_dump --column-inserts -f d:\postgres_backup.sql postgres Ran for 9230 ms D:\9.4\bin>timer pg_dump --column-inserts -f d:\postgres_backup.sql postgres Ran for 9240 ms The database contained a single 1 million record table with 6 columns. Regards David Rowley pg_dump_colinsert_v0.1.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_stat_statements: calls under-estimation propagation
On Thu, Oct 3, 2013 at 5:11 PM, Sameer Thakur wrote: > On Wed, Oct 2, 2013 at 6:40 PM, Sameer Thakur wrote: >>> >>> Looks pretty good. Do you want to package up the patch with your >>> change and do the honors and re-submit it? Thanks for helping out so >>> much! >> Sure, will do. Need to add a bit of documentation explaining >> statistics session as well. >> I did some more basic testing around pg_stat_statements.max, now that >> we have clarity from Peter about its value being legitimate below 100. >> Seems to work fine, with pg_stat_statements =4 the max unique queries >> in the view are 4. On the 5th query the view holds just the latest >> unique query discarding the previous 4. Fujii had reported a >> segmentation fault in this scenario. >> Thank you for the patch > > Please find the patch attached Thanks for the patch! Here are the review comments: +OUT session_start timestamptz, +OUT introduced timestamptz, The patch exposes these columns in pg_stat_statements view. These should be documented. I don't think that session_start should be exposed in every rows in pg_stat_statements because it's updated only when all statistics are reset, i.e., session_start of all entries in pg_stat_statements indicate the same. +OUT query_id int8, query_id or queryid? I like the latter. Also the document uses the latter. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax
On Fri, Oct 4, 2013 at 5:04 AM, Marko Tiikkaja wrote: > I might be completely in the woods here, but I believe something like this > was attempted by Karol earlier, and it failed if two concurrent transactions > did something similar to: > > UPDATE foo SET a = a + 1 RETURNING BEFORE.a; > > Both of them would see BEFORE.a = 0, because that's what the "a" evaluated > to from the tuple we got before EvalPlanQual. > > But maybe what you're suggesting doesn't have this problem? Hmm, it probably does. That explains why there are executor changes here; I guess they need some comments to explain their purpose. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax
On Fri, Oct 4, 2013 at 4:42 AM, Karol Trzcionka wrote: > W dniu 04.10.2013 02:51, Robert Haas pisze: >> Do you have a link to previous discussion on the mailing list? > Sorry, most of discussion was at IRC channel. >> I'm not positive there's enough information available >> at that stage, but if p_target_rangetblentry is populated at that >> point, you should be able to make AFTER.x translate to a Var >> referencing that range table entry. > It's not enough. Even if we know "where we are", there are more issues. > The main question is: how should we pass information about "hello, I'm > specific Var, don't evaluate me like others"? My point is that AFTER.x doesn't appear to need any special marking; it means the same thing as target_table.x. BEFORE.x *does* need some kind of special marking, and I admit I'm not sure what that should look like. Maybe an RTE is OK, but letting that RTE get into the join planning machinery does not seem good; that's going to result in funky special cases all over the place. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On 2013-10-04 09:57:41 -0400, Robert Haas wrote: > On Fri, Oct 4, 2013 at 6:06 AM, Andres Freund wrote: > > It'd be pretty easy to have a function that removes everything inside a > > certain namespace. That'd be enough to make EmitWarningsOnPlaceholders() > > work, right? > > Maybe, but I don't think you're going to paper over the problem that > easily. The GUC mechanism just isn't decided to support settings that > pop into and out of existence like that. It's not a coincidence that > there's no UNSET commands for GUCs. We have RESET but that means "go > back to the default", not "go away". You're trying to bend the > mechanism to do something that it fundamentally wasn't designed to do. > I don't think that's the right way to go, but if we do decide to go > in that direction it's going to take more than a trivial patch to get > there. But that's not a new problem? It already exists and isn't really excerbated by this. > > I don't really understand the resistance to the patch. It's a two line > > change that doesn't allow anything that wasn't already allowed by other > > means (SET, SELECT set_config(), -c). It sure isn't perfect for > > everything but for some scenarios it improves the scenario sufficiently > > that it'd make at least one extension author happy ;) > > That's true, but I think the fact that those things work is an > oversight rather than a deliberate design decision. Yes, but it's already being used, so, while some minor restrictions probably aren't to problematic, forbidding multiple dots outright seems like unnecessarily breaking user applications. > >> Another option is to store the data in an actual table. One could > >> have sneazle.configtable='dbname.schemaname.tablename', for example. > > > > Doesn't really work if your extension needs to do stuff during startup > > tho. > > Granted. There's not a perfect mechanism here. But I think we should > be devoting some thought to what a good mechanism that could be used > by core *and* contrib would look like, rather than thinking that a > quick hack is going to make the pain go away. I agree that we could use some more infrastructure around configuration, but I fail to understand why it's this patch's duty to deliver it. And I don't see why this patch would endanger any more groundbreaking improvements. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpgsql.print_strict_params
On Fri, Oct 4, 2013 at 3:53 AM, Marko Tiikkaja wrote: > I don't think there has. > > I originally had more ideas for options which you could turn on/off, which I > believe might have justified reserving them, but I'm not sure any of those > will ever happen, at least not as a simple on/off option. Did you have a > better idea for the syntax? The only thing I can think of is > print_strict_params and no_print_strict_params, and I'm not very fond of > that. Yeah, that doesn't seem good. How about writing the grammar production as '#' K_PRINT_STRICT_PARAMS option_value where option_value := T_WORD | unreserved_keyword; Then you don't need to make ON and OFF keywords; you can just use strcmp() against the option_value and either throw an error, or set the flag appropriately. > Also, in what contexts are unreserved keywords a problem in modern PL/PgSQL? I am not sure I've found all cases where this can matter, but what I did is flipped through the grammar in less, looking for T_WORD, and checking for productions where T_WORD was allowed but unreserved_keyword was not. I found getdiag_target, for_variable, stmt_execsql, and cursor_variable. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On Fri, Oct 4, 2013 at 6:06 AM, Andres Freund wrote: > It'd be pretty easy to have a function that removes everything inside a > certain namespace. That'd be enough to make EmitWarningsOnPlaceholders() > work, right? Maybe, but I don't think you're going to paper over the problem that easily. The GUC mechanism just isn't decided to support settings that pop into and out of existence like that. It's not a coincidence that there's no UNSET commands for GUCs. We have RESET but that means "go back to the default", not "go away". You're trying to bend the mechanism to do something that it fundamentally wasn't designed to do. I don't think that's the right way to go, but if we do decide to go in that direction it's going to take more than a trivial patch to get there. > The problem is that such configuration formats needs to deal with a host > of already solved things like: > * triggering reload across backends > * presentation layer: SHOW/pg_settings > * The ability to override settings on certain levels: SET/ALTER > DATABASE/ALTER ... > * Correct handling of invalid values on reload and such > * parsed early enough to allocate shared memory > > That's quite the truckload of things that need to be done by any new > format. True. > I don't really understand the resistance to the patch. It's a two line > change that doesn't allow anything that wasn't already allowed by other > means (SET, SELECT set_config(), -c). It sure isn't perfect for > everything but for some scenarios it improves the scenario sufficiently > that it'd make at least one extension author happy ;) That's true, but I think the fact that those things work is an oversight rather than a deliberate design decision. >> Another option is to store the data in an actual table. One could >> have sneazle.configtable='dbname.schemaname.tablename', for example. > > Doesn't really work if your extension needs to do stuff during startup > tho. Granted. There's not a perfect mechanism here. But I think we should be devoting some thought to what a good mechanism that could be used by core *and* contrib would look like, rather than thinking that a quick hack is going to make the pain go away. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Any reasons to not move pgstattuple to core?
On Thu, Oct 3, 2013 at 10:01 PM, Peter Geoghegan wrote: > On Thu, Oct 3, 2013 at 6:36 PM, Alvaro Herrera > wrote: >> Greg Smith made a list some months ago of contrib modules that were >> essential for forensics analysis and such. Weren't we going to do >> something special about those? > > It was more like two years ago. I do still think that that kind of > effort makes a lot of sense. I think we were going to try to group the extensions into categories (debugging tools, demonstration code, data types, etc.) and maybe encourage packagers to put the debugging tools in the same OS package as the core server. But Tom was not supportive, and he was at the time the packager for Red Hat, so it didn't seem like we were going to get to far with it. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax
On 2013-10-03 20:51:08 -0400, Robert Haas wrote: > On Thu, Oct 3, 2013 at 7:54 PM, Karol Trzcionka wrote: > > Compare EXPLAIN ANALYZE VERBOSE on your statement and on "patched" > > workflow. I can see significant difference. And your "after" returns the > > value after whole the work (after trigger fired) as I know (I don't know > > if it is needed or not, I only point at the difference). > > Sure, I'm not saying we should implement it that way. I'm just > pointing out that the ability already exists, at the executor level, > to return either tuple. So I think the executor itself shouldn't need > to be changed; it's just a matter of getting the correct plan tree to > pop out. Note what pullups ExecDelete is doing to return the old tuple though... So, based on precedent special executor support is not an unlikely thing to be required for a proper implemenation. As Marko mentions, any trivial implementation not doing playing dirty like that will refer to the wrong version of the tuple. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] space reserved for WAL record does not match what was written: panic on windows
On Sat, Oct 5, 2013 at 1:34 AM, David Rowley wrote:
> On Sat, Oct 5, 2013 at 1:19 AM, Andres Freund wrote:
>
>> On 2013-10-05 01:05:37 +1300, David Rowley wrote:
>> > In HEAD of 9.4 I'm getting the following:
>> >
>> > D:\9.4\bin>postgres.exe -D d:\9.4\data
>> > LOG: database system was shut down at 2013-10-05 00:43:33 NZDT
>> > LOG: database system is ready to accept connections
>> > LOG: autovacuum launcher started
>> > PANIC: space reserved for WAL record does not match what was written:
>> > CurrPos = 18446744071562067968 EndPos = 2147483648
>>
>> Could it be that MAXALIGN/TYPEALIGN doesn't really work for values
>> bigger than 32bit?
>>
>> #define MAXALIGN(LEN) TYPEALIGN(MAXIMUM_ALIGNOF, (LEN))
>> #define TYPEALIGN(ALIGNVAL,LEN) \
>> (((intptr_t) (LEN) + ((ALIGNVAL) - 1)) & ~((intptr_t) ((ALIGNVAL)
>> - 1)))
>>
>>
> It looks that way.
>
> As a quick test I put some printf's around where the MAXALIGN is used:
>
> /* Align the end position, so that the next record starts aligned */
> printf("CurrPos == %llu (before MAXALIGN)\n", CurrPos);
> CurrPos = MAXALIGN(CurrPos);
> printf("CurrPos == %llu (after MAXALIGN)\n", CurrPos);
>
> I got the following just before the PANIC.
>
> CurrPos == 2147483711 (before MAXALIGN)
> CurrPos == 18446744071562068032 (after MAXALIGN)
>
So I guess this would also break the 32bit linux builds too.
I've attached a proposed patch which seems to fix the problem.
Regards
David
>
> Regards
>
> David
>
>
maxalign64.patch
Description: Binary data
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI freezing bug
Andres Freund wrote: >>> On 2013-10-03 21:14:17 -0700, Dan Ports wrote: > Heikki Linnakangas wrote: >> IMHO it would be better to remove xmin from the lock key, >> and vacuum away the old predicate locks when the >> corresponding tuple is vacuumed. >> The xmin field is only required to handle the case that a >> tuple is vacuumed, and a new unrelated tuple is inserted to >> the same slot. >> Removing the lock when the tuple is removed fixes that. This seems definitely safe: we need the predicate locks to determine if someone is modifying a tuple we read, and certainly if it's eligible for vacuum nobody's going to be modifying that tuple anymore. I totally agree. It would be safe, and generally seems better, to omit xmin from the hash tag for heap tuple locks. > But locks on those still can have meaning, right? From my very > limited understanding of predicate.c/SSI I don't see why we > cannot have meaningful conflicts on tuples that are already > vacuumable. You'd need some curiously interleaved transactions, > sure, but it seems possible? No. We established quite firmly that predicate locks on a tuple do not need to be carried forward to new versions of that tuple. It is only the *version* of the row that was read to create the predicate lock which needs to be monitored for write conflicts. If the row has been vacuumed, or even determined to be eligible for vacuuming, we know it is not a candidate for update or delete -- so it is safe to drop the predicate locks. We do *not* want to do any other cleanup for the transaction which read that tuple based on this; just the individual tuple lock should be cleared. Any conflict with the lock which occurred earlier will be preserved. > ISTM we'd need to peg the xmin horizon for vacuum to the oldest > xmin predicate.c keeps track of. That would be another alternative, but it seems more problematic and less effective. I'm strongly leaning toward the idea that a slightly tweaked version of the proposed patch is appropriate for the back-branches, because the fix Heikki is now suggesting seems too invasive to back-patch. I think it would make sense to apply it to master, too, so that the new isolation tests can be immediately added. We can then work on the new approach for 9.4 and have the tests to help confirm we are not breaking anything. The tweak would be to preserve the signature of heap_freeze_tuple(), because after the more invasive fix in 9.4 the new parameters will not be needed. They are only passed as non-NULL from one of the three callers, so it seems best to leave those call sites alone rather than change them back-and-forth. I will post a new patch today or tomorrow. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] space reserved for WAL record does not match what was written: panic on windows
On Sat, Oct 5, 2013 at 1:19 AM, Andres Freund wrote:
> On 2013-10-05 01:05:37 +1300, David Rowley wrote:
> > In HEAD of 9.4 I'm getting the following:
> >
> > D:\9.4\bin>postgres.exe -D d:\9.4\data
> > LOG: database system was shut down at 2013-10-05 00:43:33 NZDT
> > LOG: database system is ready to accept connections
> > LOG: autovacuum launcher started
> > PANIC: space reserved for WAL record does not match what was written:
> > CurrPos = 18446744071562067968 EndPos = 2147483648
>
> Could it be that MAXALIGN/TYPEALIGN doesn't really work for values
> bigger than 32bit?
>
> #define MAXALIGN(LEN) TYPEALIGN(MAXIMUM_ALIGNOF, (LEN))
> #define TYPEALIGN(ALIGNVAL,LEN) \
> (((intptr_t) (LEN) + ((ALIGNVAL) - 1)) & ~((intptr_t) ((ALIGNVAL)
> - 1)))
>
>
It looks that way.
As a quick test I put some printf's around where the MAXALIGN is used:
/* Align the end position, so that the next record starts aligned */
printf("CurrPos == %llu (before MAXALIGN)\n", CurrPos);
CurrPos = MAXALIGN(CurrPos);
printf("CurrPos == %llu (after MAXALIGN)\n", CurrPos);
I got the following just before the PANIC.
CurrPos == 2147483711 (before MAXALIGN)
CurrPos == 18446744071562068032 (after MAXALIGN)
Regards
David
Re: [HACKERS] space reserved for WAL record does not match what was written: panic on windows
On 2013-10-05 01:05:37 +1300, David Rowley wrote: > In HEAD of 9.4 I'm getting the following: > > D:\9.4\bin>postgres.exe -D d:\9.4\data > LOG: database system was shut down at 2013-10-05 00:43:33 NZDT > LOG: database system is ready to accept connections > LOG: autovacuum launcher started > PANIC: space reserved for WAL record does not match what was written: > CurrPos = 18446744071562067968 EndPos = 2147483648 Could it be that MAXALIGN/TYPEALIGN doesn't really work for values bigger than 32bit? #define MAXALIGN(LEN) TYPEALIGN(MAXIMUM_ALIGNOF, (LEN)) #define TYPEALIGN(ALIGNVAL,LEN) \ (((intptr_t) (LEN) + ((ALIGNVAL) - 1)) & ~((intptr_t) ((ALIGNVAL) - 1))) Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] space reserved for WAL record does not match what was written: panic on windows
In HEAD of 9.4 I'm getting the following: D:\9.4\bin>postgres.exe -D d:\9.4\data LOG: database system was shut down at 2013-10-05 00:43:33 NZDT LOG: database system is ready to accept connections LOG: autovacuum launcher started PANIC: space reserved for WAL record does not match what was written: CurrPos = 18446744071562067968 EndPos = 2147483648 STATEMENT: insert into test (items) select x.x from generate_series(1,1000) x(x); LOG: server process (PID 5476) exited with exit code 3 DETAIL: Failed process was running: insert into test (items) select x.x from generate_series(1,1000) x(x); LOG: terminating any other active server processes debug_assertions = on I made a slight change to the line that causes the panic to print out the values of CurrPos and EndPos, as you can see above. Only changes made to postgresql.conf are: checkpoint_segments = 64 # in logfile segments, min 1, 16MB each checkpoint_timeout = 15min # range 30s-1h I discovered this was happening just after I bumped the checkpoint segment up to 64 for bulk loading some test data. create table test ( id serial not null, name varchar(64) not null default 'name of something', price numeric(10,2) not null default 1000.00, active boolean not null default true, items int not null default 1, description text not null default 'description of item', primary key(id) ); insert into test (items) select x.x from generate_series(1,1000) x(x); I'm running this on windows 7 64bit with postgres compiled as 32bit. Regards David
[HACKERS] [PATCH] bgworker doc typo fixes
Hi, I noticed a couple of missing words (at least to my reading) in the bgworker doc. This changes "...is started including the module name..." to "...is started by including the module name..." and "...to obtain information the status of the worker." to "...to obtain information regarding the status of the worker." Thanks, Maciek bgworker-doc-typo-fixes.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GIN improvements part 1: additional information
On Fri, Oct 4, 2013 at 12:28 PM, Heikki Linnakangas wrote: > Aside from the pg_upgrade discussion, here's an updated version of the > patch, rebased over master. It also contains some other misc refactoring > I've done while reading through the patch. I haven't tested this much, I > may well have also broken something, but I wanted to post an update before > the weekend. > > Thinking about the page format, I think we should start using the > pd_lower/upper pointers in the data page format. For a non-leaf page, > pd_upper would always point to the beginning of the special area, and > pd_lower would indicate the end of PostingItems. For a leaf page, pd_lower > would indicate the end of the compressed posting list, and pd_upper would > point to the "leaf-index" at the end of the page. That matches the standard > page layout in the sense that the space between pd_lower and pd_upper is > free, although the data stored in the non-free areas would be quite > different. That would allow us to mark full-page images with buffer_std, > allowing the "gap" to be left out. I think that would be a more natural way > to keep track of the used/unused space on the page, anyway, compared to the > current maxoff/endoffset field in the special area. > > In the attached patch, I in fact already did that for data leaf pages, but > didn't change the format of non-leaf pages yet. If we want to support > pg_upgrade, we might want to refrain from changing the non-leaf format. In GinDataLeafPageGetPostingList* you use sizeof(ItemPointerData) without MAXALIGN. Is it an error or you especially use 2 extra bytes on leaf page? -- With best regards, Alexander Korotkov.
Re: [HACKERS] SSI freezing bug
On 04.10.2013 14:02, Andres Freund wrote: But locks on those still can have meaning, right? From my very limited understanding of predicate.c/SSI I don't see why we cannot have meaningful conflicts on tuples that are already vacuumable. You'd need some curiously interleaved transactions, sure, but it seems possible? To conflict with a lock, a backend would need to read or update the tuple the lock is on. If the tuple is vacuumable, it's no longer visible to anyone, so no backend can possibly read or update it anymore. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI freezing bug
On 2013-10-04 13:51:00 +0300, Heikki Linnakangas wrote: > On 04.10.2013 13:23, Andres Freund wrote: > >On 2013-10-03 21:14:17 -0700, Dan Ports wrote: > >>On Thu, Oct 03, 2013 at 06:19:49AM -0700, Kevin Grittner wrote: > >>>Heikki Linnakangas wrote: > IMHO it would be better to remove xmin from the lock key, and vacuum > away the old predicate locks when the corresponding tuple is vacuumed. > The xmin field is only required to handle the case that a tuple is > vacuumed, and a new unrelated tuple is inserted to the same slot. > Removing the lock when the tuple is removed fixes that. > >> > >>This seems definitely safe: we need the predicate locks to determine if > >>someone is modifying a tuple we read, and certainly if it's eligible > >>for vacuum nobody's going to be modifying that tuple anymore. > > > >But we're talking about freezing a tuple, not removing a dead tuple. I > >don't see anything preventing modification of a frozen tuple. Right? > > Right, but if we no longer include xmin in the lock key, freezing a tuple > makes no difference. Currently, the problem is that when a tuple is frozen, > the locks on the tuple on the tuple are "lost", as the xmin+ctid of the lock > no longer matches xmin+ctid of the tuple. > > Removing xmin from the lock altogether solves that problem, but it > introduces the possibility that when an old tuple is vacuumed away and a new > tuple is inserted on the same slot, a lock on the old tuple is confused to > be a lock on the new tuple. And that problem can be fixed by vacuuming > locks, when the corresponding tuple is vacuumed. But locks on those still can have meaning, right? From my very limited understanding of predicate.c/SSI I don't see why we cannot have meaningful conflicts on tuples that are already vacuumable. You'd need some curiously interleaved transactions, sure, but it seems possible? ISTM we'd need to peg the xmin horizon for vacuum to the oldest xmin predicate.c keeps track of. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI freezing bug
On 04.10.2013 13:23, Andres Freund wrote: On 2013-10-03 21:14:17 -0700, Dan Ports wrote: On Thu, Oct 03, 2013 at 06:19:49AM -0700, Kevin Grittner wrote: Heikki Linnakangas wrote: IMHO it would be better to remove xmin from the lock key, and vacuum away the old predicate locks when the corresponding tuple is vacuumed. The xmin field is only required to handle the case that a tuple is vacuumed, and a new unrelated tuple is inserted to the same slot. Removing the lock when the tuple is removed fixes that. This seems definitely safe: we need the predicate locks to determine if someone is modifying a tuple we read, and certainly if it's eligible for vacuum nobody's going to be modifying that tuple anymore. But we're talking about freezing a tuple, not removing a dead tuple. I don't see anything preventing modification of a frozen tuple. Right? Right, but if we no longer include xmin in the lock key, freezing a tuple makes no difference. Currently, the problem is that when a tuple is frozen, the locks on the tuple on the tuple are "lost", as the xmin+ctid of the lock no longer matches xmin+ctid of the tuple. Removing xmin from the lock altogether solves that problem, but it introduces the possibility that when an old tuple is vacuumed away and a new tuple is inserted on the same slot, a lock on the old tuple is confused to be a lock on the new tuple. And that problem can be fixed by vacuuming locks, when the corresponding tuple is vacuumed. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI freezing bug
On 2013-10-03 21:14:17 -0700, Dan Ports wrote: > On Thu, Oct 03, 2013 at 06:19:49AM -0700, Kevin Grittner wrote: > > Heikki Linnakangas wrote: > > > IMHO it would be better to remove xmin from the lock key, and vacuum > > > away the old predicate locks when the corresponding tuple is vacuumed. > > > The xmin field is only required to handle the case that a tuple is > > > vacuumed, and a new unrelated tuple is inserted to the same slot. > > > Removing the lock when the tuple is removed fixes that. > > This seems definitely safe: we need the predicate locks to determine if > someone is modifying a tuple we read, and certainly if it's eligible > for vacuum nobody's going to be modifying that tuple anymore. But we're talking about freezing a tuple, not removing a dead tuple. I don't see anything preventing modification of a frozen tuple. Right? Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] insert throw error when year field len > 4 for timestamptz datatype
On 03 October 2013 19:30 Bruce Momjian wrote: >On Thu, Oct 3, 2013 at 11:54:14AM +0530, Rushabh Lathia wrote: >> Thanks Bruce. >> >> Yes for me main problem was to make assumption that a 5-digit number >> is a year, as was bit worried about side effect of that assumption in >> the date/time module. I did tested patch shared by you with various >> test and so far it looks good to me. >> >> I would like reviewer to review/test the patch and share his comments. >> >> Attaching the git patch again with this mail. >> >> Assigning to Reviewer. >Oh, great. If everyone likes it I can apply it. With Year length of 6 digits has some inconsistency problem, The tests are carried out on a default configuration. select timestamptz '199910108 01:01:01 IST'; -- works select timestamptz '19991 01 08 01:01:01 IST'; -- works select timestamptz '1999100108 01:01:01 IST'; -- works select timestamptz '199910 01 08 01:01:01 IST'; -- Not working select timestamptz 'January 8, 19991 01:01:01 IST'; -- works select timestamptz 'January 8, 199910 01:01:01 IST'; -- Not working CREATE TABLE TIMESTAMPTZ_TST (a int , b timestamptz); INSERT INTO TIMESTAMPTZ_TST VALUES(1, '10312 23:58:48 IST'); -- works INSERT INTO TIMESTAMPTZ_TST VALUES(2, '1 03 12 23:58:48 IST'); -- works INSERT INTO TIMESTAMPTZ_TST VALUES(3, '100312 23:58:48 IST'); -- works INSERT INTO TIMESTAMPTZ_TST VALUES(4, '10 03 12 23:58:48 IST'); -- Not working please correct me if anything wrong in the tests. Regards, Hari babu. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [RFC] Extend namespace of valid guc names
Hi Robert, On 2013-10-03 14:39:46 -0400, Robert Haas wrote: > On Mon, Sep 23, 2013 at 9:36 AM, Alvaro Herrera > wrote: > > I think the idea that we should consider a different way of handling > > tabular configuration data has merit. In fact, how much sense does it > > make to have these options (the ones for which this patch is being > > written) be GUCs in the first place? ALTER USER/DATABASE don't work for > > them, they can't be usefully changed in the commandline, there's no > > working SET. > > > > If we had some way to plug these into pg_hba.conf parsing machinery > > (which is tabular data), I would suggest that. But that doesn't sound > > really sensible. I think the idea of putting this configuratio data > > in a separate file, and perhaps a more convenient format than > > three-level GUC options, should be considered. > > All very good points, IMHO. In a lot of cases, what you want is > > sneazle.list='foo,bar' > sneazle.foo.prop1='zatz' > sneazle.bar.prop1='frotz' > etc. > > But that means that the set of GUCs that exist to be SET needs to > change every time sneazle.list changes, and we haven't got any good > mechanism for that. It'd be pretty easy to have a function that removes everything inside a certain namespace. That'd be enough to make EmitWarningsOnPlaceholders() work, right? > I really think we're trying to squeeze a square > peg into a round hole here, and I accordingly propose to mark this > patch rejected. > It seems to me that if an extension wants to read and parse a > configuration file in $PGDATA, it doesn't need any special core > support for that. If there's enough consistency in terms of what > those configuration files look like across various extensions, we > might choose to provide a set of common tools in core to help parse > them. But I'm not too convinced any useful pattern will emerge. The problem is that such configuration formats needs to deal with a host of already solved things like: * triggering reload across backends * presentation layer: SHOW/pg_settings * The ability to override settings on certain levels: SET/ALTER DATABASE/ALTER ... * Correct handling of invalid values on reload and such * parsed early enough to allocate shared memory That's quite the truckload of things that need to be done by any new format. I don't really understand the resistance to the patch. It's a two line change that doesn't allow anything that wasn't already allowed by other means (SET, SELECT set_config(), -c). It sure isn't perfect for everything but for some scenarios it improves the scenario sufficiently that it'd make at least one extension author happy ;) > Another option is to store the data in an actual table. One could > have sneazle.configtable='dbname.schemaname.tablename', for example. Doesn't really work if your extension needs to do stuff during startup tho. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax
On 10/4/13 12:28 AM, Robert Haas wrote: In fact, we can already get approximately the desired effect already: rhaas=# update foo as after set a = before.a + 1 from foo as before where before.a = after.a returning before.a, after.a; a | a ---+--- 1 | 2 (1 row) Now this is a hack, because we don't really want to add an extra scan/join just to get the behavior we want. But it seems to me significant that this processing makes Vars that refer to the target table refer to the new values, and if we got rid of it, they'd refer to the old values. Can't we contrive to make AFTER.x parse into the same Var node that x currently does? Then we don't need an RTE for it. This part sounds reasonable from here. And maybe BEFORE.x ought to parse to the same node that just plain x does but with some marking, or some other node wrapped around it (like a TargetEntry with some flag set?) that suppresses this processing. I'm just shooting from the hip here; that might be wrong in detail, or even in broad strokes, but it just looks to me like the additional RTE kind is going to bleed into a lot of places. I might be completely in the woods here, but I believe something like this was attempted by Karol earlier, and it failed if two concurrent transactions did something similar to: UPDATE foo SET a = a + 1 RETURNING BEFORE.a; Both of them would see BEFORE.a = 0, because that's what the "a" evaluated to from the tuple we got before EvalPlanQual. But maybe what you're suggesting doesn't have this problem? Regards, Marko Tiikkaja -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GSOC13 proposal - extend RETURNING syntax
W dniu 04.10.2013 02:51, Robert Haas pisze: > Do you have a link to previous discussion on the mailing list? Sorry, most of discussion was at IRC channel. > I'm not positive there's enough information available > at that stage, but if p_target_rangetblentry is populated at that > point, you should be able to make AFTER.x translate to a Var > referencing that range table entry. It's not enough. Even if we know "where we are", there are more issues. The main question is: how should we pass information about "hello, I'm specific Var, don't evaluate me like others"? We can add two fields to Var structure (flag - normal/before/after and no. column) - however it needs to modify copyObject for Vars (at now it's done e.g. in flatten_join_alias_vars_mutator for varoattno and varnoold). If copyObject is modified, sure code in flatten_join_alias_vars_mutator/pullup_replace_vars_callback will be useless. I don't know if modifying pg at the low-level (changing structure of Var and behaviour of copyObject) is good idea. Yes if the community really want it but it needs more "votes". There is "medium" solution: changing Var structure and do the "copy" like now (in mutator and callback) but w/o the condition statement (for the new fields). I think it might need to modify more places in code because of "comparing" vars (maybe we'd need to include new fields while comparision). Regards, Karol Trzcionka -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] GIN improvements part 1: additional information
Aside from the pg_upgrade discussion, here's an updated version of the patch, rebased over master. It also contains some other misc refactoring I've done while reading through the patch. I haven't tested this much, I may well have also broken something, but I wanted to post an update before the weekend. Thinking about the page format, I think we should start using the pd_lower/upper pointers in the data page format. For a non-leaf page, pd_upper would always point to the beginning of the special area, and pd_lower would indicate the end of PostingItems. For a leaf page, pd_lower would indicate the end of the compressed posting list, and pd_upper would point to the "leaf-index" at the end of the page. That matches the standard page layout in the sense that the space between pd_lower and pd_upper is free, although the data stored in the non-free areas would be quite different. That would allow us to mark full-page images with buffer_std, allowing the "gap" to be left out. I think that would be a more natural way to keep track of the used/unused space on the page, anyway, compared to the current maxoff/endoffset field in the special area. In the attached patch, I in fact already did that for data leaf pages, but didn't change the format of non-leaf pages yet. If we want to support pg_upgrade, we might want to refrain from changing the non-leaf format. - Heikki gin-packed-postinglists-8-heikki.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plpgsql.print_strict_params
On 10/3/13 6:55 PM, Robert Haas wrote: This looks like a nice clean patch. My only concern is that it makes "on" and "off" unreserved plpgsql keywords. It looks like that will make them unusable as unquoted identifiers in a few contexts in which they can now be used. Has there been any discussion about whether that's OK? I don't think there has. I originally had more ideas for options which you could turn on/off, which I believe might have justified reserving them, but I'm not sure any of those will ever happen, at least not as a simple on/off option. Did you have a better idea for the syntax? The only thing I can think of is print_strict_params and no_print_strict_params, and I'm not very fond of that. Also, in what contexts are unreserved keywords a problem in modern PL/PgSQL? Regards, Marko Tiikkaja -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Patch for fail-back without fresh backup
On Fri, Oct 4, 2013 at 1:46 PM, Sawada Masahiko wrote:
> On Fri, Sep 27, 2013 at 6:44 PM, Sawada Masahiko
> wrote:
>> On Fri, Sep 27, 2013 at 5:18 PM, Pavan Deolasee
>> wrote:
>>> On Fri, Sep 27, 2013 at 1:28 PM, Sawada Masahiko
>>> wrote:
>
Thank you for comment. I think it is good simple idea.
In your opinion, if synchronous_transfer is set 'all' and
synchronous_commit is set 'on',
the master wait for data flush eve if user sets synchronous_commit to
'local' or 'off'.
For example, when user want to do transaction early, user can't do this.
we leave the such situation as constraint?
>>>
>>> No, user can still override the transaction commit point wait. So if
>>>
>>> synchronous_transfer is set to "all":
>>> - If synchronous_commit is ON - wait at all points
>>> - If synchronous_commit is OFF - wait only at buffer flush (and other
>>> related to failback safety) points
>>>
>>> synchronous_transfer is set to "data_flush":
>>> - If synchronous_commit is either ON o OFF - do not wait at commit points,
>>> but wait at all other points
>>>
>>> synchronous_transfer is set to "commit":
>>> - If synchronous_commit is ON - wait at commit point
>>> - If synchronous_commit is OFF - do not wait at any point
>>>
>>
>> Thank you for explain. Understood.
>> if synchronous_transfer is set 'all' and user changes
>> synchronous_commit to 'off'( or 'local') at a transaction,
>> the master server wait at buffer flush, but doesn't wait at commit
>> points. Right?
>>
>> In currently patch, synchronous_transfer works in cooperation with
>> synchronous_commit.
>> But if user changes synchronous_commit at a transaction, they are not
>> in cooperation.
>> So, your idea might be better than currently behaviour of
>> synchronous_transfer.
>
> I attached the v11 patch which have fixed following contents.
You added several checks into SyncRepWaitForLSN() so that it can handle both
synchronous_transfer=data_flush and =commit. This change made the source code
of the function very complicated, I'm afraid. To simplify the source code,
what about just adding new wait-for-lsn function for data_flush instead of
changing SyncRepWaitForLSN()? Obviously that new function and
SyncRepWaitForLSN()
have the common part. I think that it should be extracted as separate function.
+ * Note that if sync transfer is requested, we can't regular
maintenance until
+ * standbys to connect.
*/
-if (synchronous_commit > SYNCHRONOUS_COMMIT_LOCAL_FLUSH)
+if (synchronous_commit > SYNCHRONOUS_COMMIT_LOCAL_FLUSH &&
!SyncTransRequested())
Per discussion with Pavan, ISTM we don't need to avoid setting
synchronous_commit
to local even if synchronous_transfer is data_flush. But you did that here. Why?
When synchronous_transfer = data_flush, anti-wraparound vacuum can be blocked.
Is this safe?
+#synchronous_transfer = commit# data page synchronization level
+# commit, data_flush or all
This comment seems confusing. I think that this parameter specifies when to
wait for replication.
+typedef enum
+{
+SYNCHRONOUS_TRANSFER_COMMIT,/* no wait for flush data page */
+SYNCHRONOUS_TRANSFER_DATA_FLUSH,/* wait for data page flush only
+ * no wait for WAL */
+SYNCHRONOUS_TRANSFER_ALL/* wait for data page flush and WAL*/
+}SynchronousTransferLevel;
These comments also seem confusing. For example, I think that the meaning of
SYNCHRONOUS_TRANSFER_COMMIT is something like "wait for replication on
transaction commit".
@@ -521,6 +531,13 @@ smgr_redo(XLogRecPtr lsn, XLogRecord *record)
*/
XLogFlush(lsn);
+/*
+ * If synchronous transfer is requested, wait for failback safe standby
+ * to receive WAL up to lsn.
+ */
+if (SyncTransRequested())
+SyncRepWaitForLSN(lsn, true, true);
If smgr_redo() is called only during recovery, SyncRepWaitForLSN() doesn't need
to be called here.
Regards,
--
Fujii Masao
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
