Re: [HACKERS] Heads up: 8.3beta3 to be wrapped this evening
> Any last-minute fixes out there? > > With luck this will be the last beta --- we are thinking RC1 in about > two weeks and final in early December, if no showstopper bugs are > reported. So get out there and test it ... > I will not have time to fix the default TS parser before then. There's a faint chance I can get to it this weekend. cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Exposing quals
> > On Mon, 2008-05-05 at 12:01 -0700, David Fetter wrote: > >> Please find attached the patch, and thanks to Neil Conway and Korry >> Douglas for the code, and to Jan Wieck for helping me hammer out the >> scheme above. Mistakes are all mine ;) > > I see no negative comments to this patch on -hackers. > > This was discussed here > http://wiki.postgresql.org/wiki/PgCon_2008_Developer_Meeting#SQL.2FMED > and I had understood the consensus to be that we would go ahead with > this? > > The notes say "Heikki doesn't think this is a long term solution", but > in the following discussion it was the *only* way of doing this that > will work with non-PostgreSQL databases. So it seems like the way we > would want to go, yes? > > So, can we add this to the CommitFest July page so it can receive some > substantial constructive/destructive comments? > > This could be an important feature in conjunction with Hot Standby. The notes say at the end: "Jan thinks that showing the node tree will work better. But others don't agree with him -- it wouldn't work for PL/perlU. But Jan thinks it would work to give it a pointer to the parse tree and the range, we'd need to add an access function for the PL." For the record, I agree with Jan's suggestion of passing a pointer to the parse tree, and offline gave David a suggestion verbally as to how this could be handled for PL/PerlU. I don't think we should be tied too closely to a string representation, although possibly the first and simplest callback function would simply stringify the quals. cheers andrew -- 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] Revisiting default_statistics_target
> Greg Smith writes: >> Yesterday Jignesh Shah presented his extensive benchmark results >> comparing >> 8.4-beta1 with 8.3.7 at PGCon: >> http://blogs.sun.com/jkshah/entry/pgcon_2009_performance_comparison_of > >> While most cases were dead even or a modest improvement, his dbt-2 >> results >> suggest a 15-20% regression in 8.4. Changing the >> default_statistics_taget >> to 100 was responsible for about 80% of that regression. The remainder >> was from the constraint_exclusion change. That 80/20 proportion was >> mentioned in the talk but not in the slides. Putting both those back to >> the 8.3 defaults swapped things where 8.4b1 was ahead by 5% instead. > > Yeah, I saw that talk and I'm concerned too, but I think it's premature > to conclude that the problem is precisely that stats_target is now too > high. I'd like to see Jignesh check through the individual queries in > the test and make sure that none of them had plans that changed for the > worse. The stats change might have just coincidentally tickled some > other planning issue. Wouldn't he just need to rerun the tests with default_stats_target set to the old value? I presume he has actually done this already in order to come to the conclusion he did about the cause of the regression. cheers andrew -- 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] Details
See the attached file for details ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
Re: [HACKERS] Stats Collector Error 7.4beta1 and 7.4beta2
This analysis makes sense - I think using memcmp is clearly wrong here. cheers andrew Jan Wieck said: > On a second thought, > > I still believe that this is just garbage in the padding bytes after > the IPV4 address. The code currently bind()'s and connect()'s > explicitly to an AF_INET address. So all we ever should see is > something from and AF_INET address. Everything else in the sin_family > has to be discarded. I do not think it is allowed to bind() and > connect() to an IPV4 address and then get anything other than an IPV4 > address back from the system. If that is the case, the whole idea is > broken. > > An AF_INET address now has only two relevant fields, the sin_port and > sin_addr. If they are the same, everything is fine. So the correct > check would be that 1. fromlen > sizeof(sin_family), 2. sin_family == > AF_INET, 3. sin_port and sin_addr identical. > > After reading Kurt's quoting of the SUS manpage I have to agree with > Tom in that we cannot skip the check entirely. It says it limits for > recv() but we are using recvfrom() ... there might be a little > difference on that platform ... > > ---(end of broadcast)--- TIP 5: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faqs/FAQ.html
[HACKERS] Direct Client Req from ProV-----PostGre Database Developer at San Diego, CA
Hi, This is Andrew with ProV International. I have direct client requirement for the position of PostGre Database Developer at San Diego, CA. For you reference given below is the requirement. If you find this interesting and matching to skills then please send across your updated resume in word format ASAP. If you have any query then please let me know. Position Title: PostGre Database DeveloperLocation: San Diego, CADuration: 6 MonthResponsibilities:This position involves creating tables, views, functions and stored procedures to support front end OLTP and reporting applications. The ideal developer will have thorough knowledge of SQL (PL/pgSQL), experience with atleast one other PostgreSQL language (e.g. PL/Perl), and extensive experience with complex stored procedures, code optimization, and index tuning in PostgreSQL.Skills Required:5+ years database development with PostgreSQLKnowledge of at least one other language in addition to PL/pgSQL, such as PL/Perl or PL/Java.Experience implementing PostgreSQL replication using Slony-I. Some experience with either SQL Server 2000 or Oracle 9i/10g.Significant background in creating complex stored procedures and SQL scriptsUnderstanding of database normalization concepts. Some experience in logical and physical database design andimplementationPrior experience working in a project oriented environment and meeting deadlines under tight time constraints Thanks and Regards,AndrewProV InternationalPh: 408-241-7795 ext 40[EMAIL PROTECTED]www.provintl.com
Re: [HACKERS] pgbench enhancements
> Hi, > > I have committed contrib/pgbench enhanments contributed by Tomoaki > Sato from SRA OSS, Inc. Japan. > > - predefined variable "tps" > The value of variable tps is taken from the scaling factor > specified by -s option. > > - -D option > Variable values can be defined by -D option. > > - \set command now allows arithmetic calculations. > Was there a previous patch posted for this? I don't recall seeing it. cheers andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] pgbench enhancements
>> > Hi, >> > >> > I have committed contrib/pgbench enhanments contributed by Tomoaki >> > Sato from SRA OSS, Inc. Japan. >> > >> > - predefined variable "tps" >> > The value of variable tps is taken from the scaling factor >> > specified by -s option. >> > >> > - -D option >> > Variable values can be defined by -D option. >> > >> > - \set command now allows arithmetic calculations. >> > >> >> >> Was there a previous patch posted for this? I don't recall seeing it. > > Are you saying that I should have posted patches and called for > discussion? I omit the step because a) pgbench is a contrib program, > b) the changes do not break the backward compatibility. > > I always call for discussions for questionable part. (for example, > column type change proposal for pgbench tables). It's not a big deal, but I was under the impression that even in these circumstances a patch should be posted first, especially if it changes user visible behaviour. cheers andrew ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] [PATCHES] Patch for VS.Net 2005's strxfrm() bug
> Bruce Momjian wrote: >>>> Why is this better than: >>>> >>>> #if _MSC_VER == 1400 >>>> >>>> >>>> Surely this will not be true if _MSC_VER is undefined? >>> I experienced injustice and the reason of in OSX for it. >> >> What was the problem with OSX? Did it throw a warning of you did an >> equality test on an undefined symbol? > > The following if evaluated to true on osx, although I'm pretty sure that > _MSC_VER isn't defined on osx ;-) > #if (_MSC_VER < 1300) > ... > #endif > > replacing it with > #ifdef WIN32 > #if (_MSC_VER < 1300) > ... > #endif > #endif > > fixed the problem. > No doubt, but that's quite a different test. cheers andrew ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] 8.2 features status
> > "Joshua D. Drake" <[EMAIL PROTECTED]> writes: > >> Those responsibilities include better communication, feature tracking >> and >> milestones... > > Wow, if we had all those we could have as efficient a release-engineering > process as Mozilla! > > This is not really a good argument. Might it not be possible that there is a sweeter spot somewhere in the middle? I don't think anyone wants something very heavy handed. cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] 8.2 features status
> There seems to be a lack of recognition here of how free software > development > works. When people are contributing their time scratching an itch for > their > own edification the LAST thing they want is to have a manager to report > to. > I am sick of hearing lectures on this. It is simply NOT true that free software follows a single model of development. There are many projects and all have their own methods, some formal and some not very formal. But the idea that there is ONE method is simply nonsense. You really ought to know better. >> > As far as the "problem in need of solving," it's what Andrew Dunstan >> > referred to as "splendid isolation," which is another way of saying, >> > "letting the thing you've taken on gather dust while people think >> > you're working on it." > > Really you guys are talking as if the developers that are working for the > most > part on an entirely volunteer basis have some sort of responsibility to > you. > They do not. If they don't feel like tell you where they're at then feel > free > to ask for your money back. > > Now if you think you have some tool that will make it easier for > developers to > do something they honestly want to do then feel free to suggest it and > make it > available. But if you want to dictate how programmers work for the gain of > others you're going to have a hard time swimming against the current. > > Now not all the developers working on Postgres are unpaid volunteers. But > the > ones that aren't have their own managers to report to already and their > own > timelines and other responsibilities to deal with. They're not being paid > to > meet yours. > Really, you are suggesting that a volunteer coimmunity is incapable of actually organising itself in any coherent fashion, and that only money will motivate people to act with any sense of responsibility to others. FYI I have never got a penny for the work I have done on Postgres. I would not object to an occasional query about items I had undertaken to implement. Greg, you are on an utterly wrong track here. Try to look about a bit more broadly. cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] 8.2 features status
> I don't object to someone informally polling people who have claimed a > TODO item and not produced any visible progress for awhile. But I think > anything like "thou shalt report in once a week" will merely drive > people away from publicly claiming items, if not drive them away from > doing anything at all. The former is much more what I had in mind than the latter. Let's do that. cheers andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] 8.2 features status
> [EMAIL PROTECTED] writes: >>> I don't object to someone informally polling people who have claimed a >>> TODO item and not produced any visible progress for awhile. But I >>> think >>> anything like "thou shalt report in once a week" will merely drive >>> people away from publicly claiming items, if not drive them away from >>> doing anything at all. > >> The former is much more what I had in mind than the latter. Let's do >> that. > > Like I said, no objection here. But who exactly is "we" --- ie, who's > going to do the legwork? We surely don't want multiple people pestering > the same developer ... > Perl has its pumpking ... maybe we need a designated "holder of the trunk". I see that as a Core function. cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] 8.2 features status
Ron Mayer wrote: > >> We have not had that many cases where lack of >> communication was a problem. > > One could say too much communication was the problem this time. > > I get the impression people implied they'd do something on a TODO > and didn't. Arguably the project had been better off if noone > had claimed the TODO, so if another company/team/whatever needed > the feature badly, they could have worked on it themselves rather > than waiting in hope of the feature. Of course they could have > done this anyway - but if they see it on an implied roadmap document > for the next release they're more likely to wait. > This is just perverse. Surely you are not seriously suggesting that we should all develop in secret and then spring miracles fully grown on the community? We have bumped patches before because they have done things without discussing them, and were found not to be accepatble. The more complex features get, the more communication is needed. cheers andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] 8.2 features status
Tom Lane wrote: > Martijn van Oosterhout writes: >> On Sat, Aug 05, 2006 at 12:19:54AM -0400, Matthew T. O'Connor wrote: >>> FTI is a biggie in my mind. I know it ain't happening for 8.2, but is >>> the general plan to integrate TSearch2 directly into the backend? > >> When the Tsearch developers say so I think. > > Yeah, that's my take too. Oleg and Teodor obviously feel it's not "done" > yet, and ISTM leaving it in contrib gives them more flexibility in a > couple of ways: > * they can make user-visible API changes without people getting as upset > as if they were changing core features; > * because it is a removable contrib module, they can (and do) offer > back-ports of newer versions to existing PG release branches. > > I think some descendant of tsearch2 will eventually be in core, but > we'll wait till we're pretty certain it's feature-stable. > My impression from this post http://archives.postgresql.org/pgsql-hackers/2006-07/msg00556.php was that moving it into core should be doable for 8.3. I hope I didn't misunderstand. cheers andrew ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] proposal for 8.3: Simultaneous assignment for
Tom Lane wrote: > "Pavel Stehule" <[EMAIL PROTECTED]> writes: >> a,b,c := out3fce(1); -- Simultaneous assignment > > I thought we rejected that idea once already, on the grounds that it > would make it too hard to tell the difference between intended code > and typos. > In any case, I had some questions: . is it compatible with PLSQL? . can the effect be achieved by assigning to a composite? cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Buildfarm failure on ecpg/test/pgtypeslib
Tom Lane wrote: > "Jim C. Nasby" <[EMAIL PROTECTED]> writes: >> BTW, what time does the anonymous-cvs rsync normally finish? Right now >> my build starts at 5 after, but I suspect that's the worst possible time >> for it... > > Marc recently said it starts at 19 after, so somewhere near half past is > probably the optimal time. (I'd suggest picking a random time near half > past, else all the buildfarm members will gang up on the server at once > ...) > is this also true of the cvsup server? cheers andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] Buildfarm failure on ecpg/test/pgtypeslib
Jim Nasby wrote: > Oh, I didn't realize there was a CVSup server. I think it'd be good > to promote that over CVS, as it's (supposedly) much easier on the > hosting machine. > > Andrew, is there a way to get the buildfarm to use cvsup instead of > cvs? Does the script just call cvs via the shell? > You can call cvs directly against your csvup repo copy if it's on the same machine, or set up a pserver against the repo copy. The buildfarm howto at http://pgfoundry.org/docman/view.php/140/4/PGBuildFarm-HOWTO.txt has some extra details. Buildfarm just calls your cvs client. cheers andrew ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] perl namespace for postgres specific modules?
Andrew Hammond wrote: > I need to write a perl module which will parse a .pgpass file into a > reasonable data-structure in memory. I may extend it later to go in the > other direction (given a populated datastructure, write a .pgpass). > > The first question that came to mind is what namespace should I put > this under? Is there any precedent for perl modules intended to support > postgresql administration? If not, I suggest > > PostgreSQL::pgpass > IIRC it is conventional to begin each section of a perl namespace with an upper case char. That's what I tend to do at any rate. (Of course, namespaces aren't really hierarchical, but it's a nice illusion.) Other than that it seems reasonable. cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] domains code query
Tom Lane wrote: > Andrew Dunstan <[EMAIL PROTECTED]> writes: >> domains.c contains the followng snippet in domain_in(): > >> else* if (my_extra->domain_type != domainType) >> domain_state_setup(my_extra, domainType, false, >> fcinfo->flinfo->fn_mcxt); > >> We were just looking at this code (in the context of implementing enums) >> and wondered when this case might arise. >> Would it be when more than one domain is used in a table row? Or are we >> smarter than that? > > I think it's just defensive programming. The logic was copied from > array_in which does something similar, but AFAIR there's not really > any code path which would feed differing input types to the same > function call within a single query execution. Still, since it takes > only an extra comparison or so to handle the scenario, why not? > Sure. We were hoping it was something like that. cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] Enum proposal / design
We forgot to mention that we'll need to implement domains over enums and
arrays of enums too.
cheers
andrew
Tom Dunstan wrote:
> Hi guys
>
> Andrew and I got together and worked out a more detailed idea of how we
> want to add enums to the postgresql core. This follows on from his
> original enumkit prototype last year [1]. Here's a more formal proposal
> / design with what we came up with. Comments / criticism hereby solicited.
>
>
> How they will work (once created) is more or less the same as last time
> with the enumkit, with the exception of how they're created.
>
> Enum types will be created with a specialised version of the CREATE TYPE
> command thusly:
>
> CREATE TYPE rgb AS ENUM ('red', 'green', 'blue');
>
> They can then be used as column types, being input in quoted string form
> as with other user types:
>
> CREATE TABLE enumtest (col rgb);
> INSERT INTO enumtest VALUES ('red');
>
> Input is to be case sensitive, and ordering is to be in the definition
> order, not the collation order of the text values (ie 'red' < 'green' in
> the example above). See the original thread for more discussion and
> usage examples.
>
>
> The implementation will work as below. I've included something of a list
> of stuff to do as well.
>
> On disk, enums will occupy 4 bytes: the high 22 bits will be an enum
> identifier, with the bottom 10 bits being the enum value. This allows
> 1024 values for a given enum, and 2^22 different enum types, both of
> which should be heaps. The exact distribution of bits doesn't matter all
> that much, we just picked some that we were comfortable with.
>
> The identifier is required as output functions are not fed information
> about which exact type they are being asked to format (see below).
>
> The creation of a new pg_enum catalog is required. This will hold:
> - the type OID for the enum, from pg_type
> - the enum identifier for on disk storage
> - the enum values in definition order, as an array of text values
>
> The CREATE TYPE command will create a row in pg_type and a row in
> pg_enum. We will get a new enum id by scanning pg_enum and looking for
> the first unused value, rather than using a sequence, to make reuse of
> enum ids more predictable.
>
> Two new syscaches on pg_enum will be created to simplify lookup in the
> i/o functions: one indexed by type oid for the input function, and one
> indexed by enum id for the output function.
>
> All functions will be builtins; there will be no duplicate entries of
> them in pg_proc as was required for the enumkit.
>
> The i/o functions will both cache enum info in the same way that the
> domain and composite type i/o functions do, by attaching the data to the
> fcinfo->flinfo->fn_extra pointer. The input function will look up the
> enum data in the syscache using the type oid that it will be passed, and
> cache it in a hashtable or binary tree for easy repeated lookup. The
> output function will look up the enum data in the syscache using the
> enum id stripped from the high 22 bits of the on-disk value and cache
> the data as a straight array for easy access, with the enum value being
> used as a index into the array.
>
> The other functions will all work pretty much like they did in the
> enumkit, with comparison operators more or less treating the enum as its
> integer representation.
>
> The grammar will have to be extended to support the new CREATE TYPE
> syntax. This should not require making ENUM a reserved word. Likewise
> psql will be extended to learn the new grammar. There's probably a bit
> of work to do in DROP TYPE to make sure it deletes rows from pg_enum
> when appropriate.
>
> pg_dump must be taught how to dump enums properly.
>
> We'll need some regression tests, maybe including one in one of the PL
> testsuites to ensure that the io functions work happily when called from
> a non-standard direction.
>
> Documentation etc.
>
>
> General discussion:
>
> While we would really like to have had a 2 byte representation on disk
> (or even 1 for most cases), with the stored value being *just* the enum
> ordinal and not containing any type info about the enum type itself,
> this is difficult. Since the output function cleanup [2] [3], postgresql
> doesn't pass through the expected output type to output functions. This
> makes it difficult to tell the difference between e.g. the first value
> of the various enums, which would all have an integer representation of
> 0. We could have gone down the path of having the output function look
> up its expected type from the fcinfo->flinfo struct, as
Re: [HACKERS] [PATCHES] plpython improvements
Tom Lane wrote: > Anyone in a position to review the pending plpython patch? > http://archives.postgresql.org/pgsql-patches/2006-08/msg00151.php > > After that little fiasco with plperl I'm disinclined to apply anything > without review by somebody who's pretty familiar with the PL in > question ... and Python's not my language. > > does it have docs? cheers andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
Re: [HACKERS] OTRS
Peter Eisentraut wrote: > OTRS was recommended to me as a bug tracker. Has anyone used that? > Not me, but I see that they use bugzilla for bug tracking ... see http://bugs.otrs.org/index.cgi cheers andrew ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
[HACKERS] look up tables while parsing queries
Hi I am modifying the source code. I want to look up some information from some tables while parsing the queries. What functions I can use to look up tables? btw I am using version 7.3. Thanks. -- andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [HACKERS] look up tables while parsing queries
On 2/5/06, Neil Conway <[EMAIL PROTECTED]> wrote: > If you're referring to the raw parser (parser/gram.y), you should not > attempt to access any tables. For one thing, the raw parser might be > invoked outside a transaction. The statement might also refer to a table > created earlier in the same query string, which would mean the > referenced table would not exist when the latter part of the query > string is parsed. > > Instead, database access should be done in the analysis phase -- see > transformStmt() in parser/analyze.c and friends. There are plenty of > examples in the code of how to access tables, which should be a helpful > guide. > > -Neil It is not in the raw parser. I meant inside the transformStmt(). What is "friends"? Could you possibly point out at least one place that illustrates how to access tables? Thanks. -- andrew ---(end of broadcast)--- TIP 6: explain analyze is your friend
[HACKERS] adding a new catalog
Hi I am trying to add a new catalog to the system. I had followed the instructions in the comments. Now I can see the definition of the new catalog table and its index in file "postgres.bki" after doing make. However, initdb still did not create the new catalog table. From the debug information of initdb, it only creates other catalogs. What steps did I miss here? -- andrew ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] adding a new catalog
oh, my mistake. I only do "make install-bin". Now it is successfully created. Thanks. On 2/8/06, Alvaro Herrera <[EMAIL PROTECTED]> wrote: > andrew wrote: > > > I am trying to add a new catalog to the system. I had followed the > > instructions in the comments. Now I can see the definition of the new > > catalog table and its index in file "postgres.bki" after doing make. > > > > However, initdb still did not create the new catalog table. From the > > debug information of initdb, it only creates other catalogs. What > > steps did I miss here? > > Are you sure that the postgres.bki file that initdb is picking up > contains your modifications? i.e. did you "make install" in the whole > source tree? > > -- > Alvaro Herrerahttp://www.CommandPrompt.com/ > PostgreSQL Replication, Consulting, Custom Development, 24x7 support > -- andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
[HACKERS] Re: [COMMITTERS] pgsql: Respect TEMP_CONFIG when running contrib regression tests.
On 02/26/2016 10:59 PM, Robert Haas wrote: On Sat, Feb 27, 2016 at 9:00 AM, Andrew Dunstan wrote: Sure. Saving three lines of Makefile duplication is hardly a world-shattering event, so I thought there might be some other purpose. But I'm not against saving three lines of duplication either, if it won't break anything. The point is that we should do this for several other test sets as well as contrib - isolation tests, PL tests and ecpg tests. OK, I was wondering about that. I can try to write a patch, or someone else can, but if you already understand what needs to be done, perhaps you should just go ahead. What I had in mind was something like the attached. In testing this seems to do the right thing, and the nice part is that it will be picked up by the buildfarm in the one case that's relevant, namely the ecpg tests. The only fly in the ointment is that there are a few places that set --temp-config explicitly: ./contrib/test_decoding/Makefile:--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ ./contrib/test_decoding/Makefile:--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ ./src/test/modules/commit_ts/Makefile:REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/commit_ts/commit_ts.conf ./src/test/modules/test_rls_hooks/Makefile:REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/test_rls_hooks/rls_hooks.conf Perhaps what we need to do is modify pg_regress.c slightly to allow more than one --temp-config argument. But that could be done later. cheers andrew diff --git a/contrib/contrib-global.mk b/contrib/contrib-global.mk index ba49610..6ac8e9b 100644 --- a/contrib/contrib-global.mk +++ b/contrib/contrib-global.mk @@ -1,9 +1,4 @@ # contrib/contrib-global.mk -# file with extra config for temp build -ifdef TEMP_CONFIG -REGRESS_OPTS += --temp-config=$(TEMP_CONFIG) -endif - NO_PGXS = 1 include $(top_srcdir)/src/makefiles/pgxs.mk diff --git a/src/Makefile.global.in b/src/Makefile.global.in index e94d6a5..47b265e 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -524,14 +524,20 @@ ifdef NO_LOCALE NOLOCALE += --no-locale endif +# file with extra config for temp build +TEMP_CONF = +ifdef TEMP_CONFIG +TEMP_CONF += --temp-config=$(TEMP_CONFIG) +endif + pg_regress_locale_flags = $(if $(ENCODING),--encoding=$(ENCODING)) $(NOLOCALE) -pg_regress_check = $(with_temp_install) $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) +pg_regress_check = $(with_temp_install) $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --temp-instance=./tmp_check $(TEMP_CONF) --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) pg_regress_installcheck = $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --bindir='$(bindir)' $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) pg_regress_clean_files = results/ regression.diffs regression.out tmp_check/ log/ -pg_isolation_regress_check = $(with_temp_install) $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) +pg_isolation_regress_check = $(with_temp_install) $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) --temp-instance=./tmp_check $(TEMP_CONF) --bindir= $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) pg_isolation_regress_installcheck = $(top_builddir)/src/test/isolation/pg_isolation_regress --inputdir=$(srcdir) $(pg_regress_locale_flags) $(EXTRA_REGRESS_OPTS) ## diff --git a/src/interfaces/ecpg/test/Makefile b/src/interfaces/ecpg/test/Makefile index a4ac021..4ed785b 100644 --- a/src/interfaces/ecpg/test/Makefile +++ b/src/interfaces/ecpg/test/Makefile @@ -78,11 +78,11 @@ endif REGRESS_OPTS = --dbname=regress1,connectdb --create-role=connectuser,connectdb $(EXTRA_REGRESS_OPTS) check: all - $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule + $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check $(TEMP_CONF) --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule # the same options, but with --listen-on-tcp checktcp: all - $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule_tcp --host=localhost + $(with_temp_install) ./pg_regress $(REGRESS_OPTS) --temp-instance=./tmp_check $(TEMP_CONF) --bindir= $(pg_regress_locale_flags) $(THREAD) --schedule=$(srcdir)/ecpg_schedule_tcp --host=localhost installcheck: all ./pg_regress $(REGRESS_OPTS) --bindir='$(bindir)' $(pg_regress_locale_f
[HACKERS] Re: [COMMITTERS] pgsql: Respect TEMP_CONFIG when running contrib regression tests.
On 02/27/2016 09:25 AM, Robert Haas wrote: On Sat, Feb 27, 2016 at 7:08 PM, Andrew Dunstan wrote: What I had in mind was something like the attached. In testing this seems to do the right thing, and the nice part is that it will be picked up by the buildfarm in the one case that's relevant, namely the ecpg tests. The only fly in the ointment is that there are a few places that set --temp-config explicitly: ./contrib/test_decoding/Makefile:--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ ./contrib/test_decoding/Makefile:--temp-config $(top_srcdir)/contrib/test_decoding/logical.conf \ ./src/test/modules/commit_ts/Makefile:REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/commit_ts/commit_ts.conf ./src/test/modules/test_rls_hooks/Makefile:REGRESS_OPTS = --temp-config=$(top_srcdir)/src/test/modules/test_rls_hooks/rls_hooks.conf Perhaps what we need to do is modify pg_regress.c slightly to allow more than one --temp-config argument. But that could be done later. Well, I'm pretty interested in using --temp-config for parallelism testing; I want to be able to run the whole regression test suite with a given --temp-config. I'm in agreement with this change but if it doesn't play well with that need, I suppose I'll be writing that pg_regress.c patch sooner rather than later. "doesn't meet your need" is probably a better way of putting it. The facility's use has grown beyond what I originally envisaged, so I think we will need that patch. Would you like me to apply what I have? cheers andrew -- 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] Re: [COMMITTERS] pgsql: Respect TEMP_CONFIG when running contrib regression tests.
On 02/27/2016 01:24 PM, John Gorman wrote: On Sat, Feb 27, 2016 at 9:25 AM, Robert Haas <mailto:robertmh...@gmail.com>> wrote: On Sat, Feb 27, 2016 at 7:08 PM, Andrew Dunstan mailto:and...@dunslane.net>> wrote: > Perhaps what we need to do is modify pg_regress.c slightly to allow more > than one --temp-config argument. But that could be done later. Well, I'm pretty interested in using --temp-config for parallelism testing; I want to be able to run the whole regression test suite with a given --temp-config. I'm in agreement with this change but if it doesn't play well with that need, I suppose I'll be writing that pg_regress.c patch sooner rather than later. Here is a patch to allow pg_regress to include several --temp-config files. Thanks, wonderfully small patch. Applied. cheers andrew -- 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] Equivalent of --enable-tap-tests in MSVC scripts
On 03/01/2016 08:00 AM, Michael Paquier wrote:
Hi all,
As of now the MSVC scripts control if TAP tests are enabled or not
using a boolean flag as $config->{tap_tests}. However, this flag is
just taken into account in vcregress.pl, with the following issues:
1) config_default.pl does not list tap_tests, so it is unclear to
users to enable them. People need to look into vcregress.pl as a start
point.
2) GetFakeConfigure() does not translate $config->{tap_tests} into
--enable-tap-tests, leading to pg_config not reporting it in
CONFIGURE. This is inconsistent with what is done in ./configure.
Attached is a patch to address those two issues.
Good work. There seem to be some unrelated whitespace changes. Shouldn't
this just be two extra lines?
cheers
andrew
--
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] VS 2015 support in src/tools/msvc
On 03/03/2016 09:02 AM, Michael Paquier wrote: Hi all, Microsoft provides a set of VMs that one can use for testing and Windows 10 is in the set: https://dev.windows.com/en-us/microsoft-edge/tools/vms/windows/ I have grabbed one and installed the community version of Visual Studio 2015 so I think that I am going to be able to compile Postgres with VS2015 with a bit of black magic. My goal is double: 1) As far as things have been discussed, VS2015 is making difficult the compilation of Postgres, particularly for locales. So I'd like to see what are the problems behind that and see if we can patch it properly. This would facilitate the integration of cmake as well for Windows. 2) src/tools/msvc stuff has support only up to VS2013. I think that it would be nice to bump that a bit and get something for 9.5~. So, would there be interest for a patch on the perl scripts in src/tools/msvc or are they considered a lost cause? Having a look at the failures could be done with the cmake work, but it seems a bit premature to me to look at that for the moment, and having support for VS2015 with 9.5 (support for new versions of VS won a backpatch the last couple of years) would be a good thing I think. I am not holding my breath on cmake. Until we have something pretty solid on that front I'm going to assume it's not happening. If we're going to support VS2015 (and I think we should) then it should be supported for all live branches if possible. I'm assuming the changes would be pretty localized, at least in src/tools/msvc, and adding a new compile shouldn't break anything with existing compilers. cheers andrew -- 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] VS 2015 support in src/tools/msvc
On 03/05/2016 12:46 AM, Tom Lane wrote: Michael Paquier writes: On Sat, Mar 5, 2016 at 1:41 PM, Petr Jelinek wrote: I vote for just using sed considering we need flex and bison anyway. OK cool, we could go with something else than sed to generate probes.h but that seems sensible considering that this should definitely be back-patched. Not sure what the others think about adding a new file in the source tarball by default though. AFAIK, sed flex and bison originate from three separate source projects; there is no reason to suppose that the presence of flex and bison on a particular system guarantee the presence of sed. I thought the proposal to get rid of the psed dependence in favor of some more perl code was pretty sane. Here is a translation into perl of the sed script, courtesy of the s2p incarnation of psed: <https://gist.github.com/adunstan/d61b1261a4b91496bdc6> The sed script appears to have been stable for a long time, so I don't think we need to be too concerned about possibly maintaining two versions. cheers andrew -- 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] VS 2015 support in src/tools/msvc
On 03/05/2016 01:31 PM, Michael Paquier wrote: On Sat, Mar 5, 2016 at 11:34 PM, Andrew Dunstan wrote: Here is a translation into perl of the sed script, courtesy of the s2p incarnation of psed: <https://gist.github.com/adunstan/d61b1261a4b91496bdc6> The sed script appears to have been stable for a long time, so I don't think we need to be too concerned about possibly maintaining two versions. That's 95% of the work already done, nice! If I finish wrapping up a patch for this issue at least would you backpatch? It would be saner to get rid of this dependency everywhere I think regarding compilation with perl 5.22. Sure. cheers andrew -- 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] Allowing to run a buildfarm animal under valgrind
On 03/07/2016 08:39 PM, Andres Freund wrote:
Hi,
I'm setting up a buildfarm animal that runs under
valgrind. Unfortunately there's not really any good solution to force
make check et al. to start postgres wrapped in valgrind. For now I've
resorted to adding something like
sub replace_postgres
{
my $srcdir=$use_vpath ? "../pgsql/" : ".";
my $builddir=abs_path("$pgsql");
$srcdir=abs_path("$pgsql/$srcdir");
chdir "$pgsql/src/backend/";
rename "postgres", "postgres.orig";
sysopen my $fh, "postgres", O_CREAT|O_TRUNC|O_RDWR, 0700
or die "Could not create postgres wrapper";
print $fh <<"END";
#!/bin/bash
~/src/valgrind/vg-in-place \\
--quiet \\
--error-exitcode=128 \\
--suppressions=$srcdir/src/tools/valgrind.supp \\
--trace-children=yes --track-origins=yes --read-var-info=yes \\
--leak-check=no \\
$builddir/src/backend/postgres.orig \\
"\$@"
END
close $fh;
chdir $branch_root;
}
to the buildfarm client.
i.e. a script that replaces the postgres binary with a wrapper that
invokes postgres via valgrind.
That's obviously not a very good approach though. It's buildfarm
specific and thus can't be invoked by developers and it doesn't really
support being installed somewhere.
Does anybody have a better idea about how to do this?
Why not just create a make target which does this? It could be run after
'make' and before 'make check'. I would make it assume valgrind was
installed and in the path rather than using vg-in-place.
If that doesn't work and you want to do something with the buildfarm,
probably a buildfarm module would be the way to go. We might need to add
a new module hook to support it, to run right after make(), but that
would be a one line addition to run_build.pl.
cheers
andrew
--
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] VS 2015 support in src/tools/msvc
On 03/08/2016 08:32 AM, Michael Paquier wrote: On Mon, Mar 7, 2016 at 10:40 PM, Michael Paquier wrote: On Sun, Mar 6, 2016 at 5:55 AM, Andrew Dunstan wrote: On 03/05/2016 01:31 PM, Michael Paquier wrote: On Sat, Mar 5, 2016 at 11:34 PM, Andrew Dunstan wrote: Here is a translation into perl of the sed script, courtesy of the s2p incarnation of psed: <https://gist.github.com/adunstan/d61b1261a4b91496bdc6> The sed script appears to have been stable for a long time, so I don't think we need to be too concerned about possibly maintaining two versions. That's 95% of the work already done, nice! If I finish wrapping up a patch for this issue at least would you backpatch? It would be saner to get rid of this dependency everywhere I think regarding compilation with perl 5.22. Sure. OK, so after some re-lecture of the script and perltidy-ing I finish with the attached. How does that look? Attached is a new set for the support of MS 2015 + the psed issue, please use those ones for testing: - 0001 is the replacement of psed by a dedicated perl script to generate probes.h - 0002 Fix of the declaration of TIMEZONE_GLOBAL and TZNAME_GLOBAL for WIN32 - 0003 addresses the issue with lc_codepage missing from _locale_t. - 0004 adds support for MS2015 in src/tools/scripts/ Thanks for doing this work. Do we already have a hard dependency on perl for all non-Windows builds? I know it's been discussed but I don't recall. If so, back to what version? The comment in the codepage patch is a bit unclear. Can you reword it a bit? cheers andrew -- 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] VS 2015 support in src/tools/msvc
On 03/08/2016 10:43 AM, Tom Lane wrote: Andrew Dunstan writes: Do we already have a hard dependency on perl for all non-Windows builds? I know it's been discussed but I don't recall. If so, back to what version? I think the policy is we require perl for building from a git pull, but not for building from a tarball. Thus, any files that perl is used to produce have to be shipped in tarballs. However, there definitely *is* a hard requirement on perl for Windows builds, even from tarballs, and I thought this patch was only about the Windows build? Michael's patch proposes to replace the use of sed to generate probes.h with the perl equivalent everywhere. That has the advantage that we keep to one script to generate probes.h, but it does impose a perl dependency. We could get around that by shipping probes.h in tarballs, which seems like a perfectly reasonable thing to do. If we don't like that we can fall back to using the perl script just for MSVC builds. cheers andrew -- 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] VS 2015 support in src/tools/msvc
On 03/08/2016 11:17 AM, Tom Lane wrote: On the whole, I'd rather that this patch left the non-Windows side alone. OK, that's why I raised the issue. We'll do it that way. As I noted upthread, the sed script has been very stable so the overhead of having to maintain two scripts is pretty minimal. cheers andrew -- 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] Alter or rename enum value
On 03/09/2016 09:56 AM, Matthias Kurz wrote: Hi! Right now it is not possible to rename an enum value. Are there plans to implement this anytime soon? I had a bit of a discussion on the IRC channel and it seems it shouldn't be that hard to implement this. Again, I am talking about renaming the values, not the enum itself. I don't know of any plans, but it would be a useful thing. I agree it wouldn't be too hard. The workaround is to do an update on pg_enum directly, but proper SQL support would be much nicer. cheers andrew -- 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] Alter or rename enum value
On 03/09/2016 11:07 AM, Tom Lane wrote: Andrew Dunstan writes: On 03/09/2016 09:56 AM, Matthias Kurz wrote: Right now it is not possible to rename an enum value. Are there plans to implement this anytime soon? I don't know of any plans, but it would be a useful thing. I agree it wouldn't be too hard. The workaround is to do an update on pg_enum directly, but proper SQL support would be much nicer. I have a vague recollection that we discussed this at the time the enum stuff went in, and there are concurrency issues? Don't recall details though. Rings a vague bell, but should it be any worse than adding new labels? cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] enums and indexing
Currently we don't have a way to create a GIN index on an array of enums, or to use an enum field in a GIST index, so it can't be used in an exclusion constraint, among other things. I'd like to work on fixing that if possible. Are there any insuperable barriers? If not, what do we need to do? cheers andrew -- 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] Perl's newSViv() versus 64-bit ints?
On 03/11/2016 06:49 PM, Tom Lane wrote: Anybody know what will happen when passing a uint64 to newSViv()? A perl IV is guaranteed large enough to hold a pointer, if that's any help. But for an unsigned value you might be better off calling newSVuv() cheers andrew -- 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] VS 2015 support in src/tools/msvc
On 03/08/2016 07:40 PM, Michael Paquier wrote: On Wed, Mar 9, 2016 at 1:07 AM, Andrew Dunstan wrote: Michael's patch proposes to replace the use of sed to generate probes.h with the perl equivalent everywhere. That has the advantage that we keep to one script to generate probes.h, but it does impose a perl dependency. Good point. It did not occur to me that this would bring a hard dependency for non-Windows builds. Let's keep both scripts then. The attached is changed to do so. Actually, it wasn't, but I fixed it and committed it. Still to do: the non-perl pieces. cheers andrew -- 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] btree_gin and btree_gist for enums
On 03/18/2016 09:54 AM, Robert Haas wrote: On Thu, Mar 17, 2016 at 11:21 AM, Andrew Dunstan wrote: On 03/17/2016 06:44 AM, Andrew Dunstan wrote: Here is a patch to add enum support to btree_gin and btree_gist. I didn't include distance operations, as I didn't think it terribly important, and there isn't a simple way to compute it sanely and efficiently, so no KNN support. This time including the data file for the gist regression tests. Are you intending this as a 9.7 submission? Because it's pretty late for 9.6. I guess so. I would certainly find it hard to argue that it should be in 9.6 unless there is a consensus on it. cheers andrew -- 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] VS 2015 support in src/tools/msvc
On 03/20/2016 12:02 AM, Michael Paquier wrote: On Sun, Mar 20, 2016 at 8:06 AM, Andrew Dunstan wrote: Still to do: the non-perl pieces. The patch to address locales is the sensitive part. The patch from Petr is taking the correct approach though I think that we had better simplify it a bit and if possible we had better not rely on the else block it introduces. OK, the please send an updated set of patches for what remains. cheers andrew -- 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] btree_gin and btree_gist for enums
On 03/24/2016 12:40 PM, Matt Wilmas wrote: It would be *really* nice to have this in 9.6. It seems it's simply filling out functionality that should already be there, right? An oversight/bug fix so it works "as advertised?" :-) I think that would be stretching the process a bit far. I'm certainly not prepared to commit it unless there is a general consensus to make an exception for it. Is any other btree type-compatibility missing from these modules? (I notice the btree_gin docs don't mention "numeric," but it works.) uuid would be another type that should be fairly easily covered but isn't. I just looked over the patch, and the actual code addition/changes seem pretty small and straightforward (or am I wrong?). Yes, sure, it's all fairly simple. Took me a little while to understand what I was doing, but once I did it was pretty much plain sailing. cheers andrew -- 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] VS 2015 support in src/tools/msvc
On 03/25/2016 08:31 AM, Michael Paquier wrote: On Fri, Mar 25, 2016 at 9:09 PM, Robert Haas wrote: On Thu, Mar 24, 2016 at 1:07 PM, Petr Jelinek wrote: On 24/03/16 17:28, Robert Haas wrote: On Wed, Mar 23, 2016 at 3:17 AM, Michael Paquier wrote: - 0001 fixes the global declarations of TIMEZONE_GLOBAL and TZNAME_GLOBAL to be WIN32-compliant. I got bitten by that in the ECPG compilation. So this isn't going to break other Windows builds? I mean, if we've got the names for those symbols wrong, how is this working right now? We didn't older versions just defined the other variants as well, but the _timezone and _tzname have been around since at least VS2003. I am unable to parse this sentence. Sorry. Petr means that both _timezone and _tzname are objects defined in Visual Studio since more or less its 2003 release (https://msdn.microsoft.com/en-us/library/htb3tdkc%28v=vs.71%29.aspx). The oldest version on the buildfarm is Visual Studio 2005, and I agree with him that there is no need to worry about older versions than VS2003. The issue is that VS2015 does *not* define timezone and tzname (please note the prefix underscore missing in those variable names), causing compilation failures. That's why I am suggesting such a change in this patch: this will allow the code to compile on VS2015, and that's compatible with VS2003~. OK, sounds good. I don't have a spare machine on which to install VS2015, nor time to set one up, so I'm going to have to trust the two of you (Michael and Petr) that this works. Will either of you be setting up a buildfarm animal with VS2015? cheers andrew -- 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] Alter or rename enum value
On 03/25/2016 04:13 AM, Matthias Kurz wrote: Hopefully at the commitfest at least the transaction limitation will/could be tackled - that would help us a lot already. I don't believe anyone knows how to do that safely. Enums pose special problems here exactly because unlike all other types the set of valid values is mutable. cheers andre -- 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] btree_gin and btree_gist for enums
On 03/24/2016 12:40 PM, Matt Wilmas wrote: (I notice the btree_gin docs don't mention "numeric," but it works.) Numeric does work - we have regression tests to prove it, do we should fix the docs. But I'm also curious to know why apparently we don't have distance operator support for numeric. cheers andrew -- 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] Alter or rename enum value
On 03/25/2016 03:22 PM, Christophe Pettus wrote: On Mar 25, 2016, at 11:50 AM, Andrew Dunstan wrote: I don't believe anyone knows how to do that safely. The core issue, for me, is that not being able to modify enum values in a transaction breaks a very wide variety of database migration tools. Even a very brutal solution like marking indexes containing the altered type invalid on a ROLLBACK would be preferable to the current situation. Well, let's see if we can think harder about when it will be safe to add new enum values and how we can better handle unsafe situations. The danger AIUI is that the new value value will get into an upper level page of an index, and that we can't roll that back if the transaction aborts. First, if there isn't an existing index using the type we should be safe - an index created in the current transaction is not a problem because if the transaction aborts the whole index will disappear. Even if there is an existing index, if it isn't touched by the current transaction the we should still be safe. However, if it is touched we could end up with a corrupted index if the transaction aborts. Maybe we could provide an option to reindex those indexes if they have been touched in the transaction and it aborts. And if it's not present we could instead abort the transaction as soon as it detects something that touches the index. I quite understand that this is all hand-wavy, but I'm trying to get a discussion started that goes beyond acknowledging the pain that the current situation involves. cheers andrew -- 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] Alter or rename enum value
On 03/26/2016 12:35 AM, David G. Johnston wrote: On Friday, March 25, 2016, Andrew Dunstan <mailto:and...@dunslane.net>> wrote: On 03/25/2016 04:13 AM, Matthias Kurz wrote: Hopefully at the commitfest at least the transaction limitation will/could be tackled - that would help us a lot already. I don't believe anyone knows how to do that safely. Enums pose special problems here exactly because unlike all other types the set of valid values is mutable. Yeah, I'm not sure there is much blue sky here as long as the definition of an enum is considered system data. It probably needs to be altered so that a user can create a table of class enum with a known layout that PostgreSQL can rely upon to perform optimizations and provide useful behaviors - at least internally. The most visible behavior being displaying the label while ordering using its position. The system, seeing a data type of that class, would have an implicit reference between columns of that type and the source table. You have to use normal cascade update/delete/do-nothing while performing DML on the source table. In some ways it would be a specialized composite type, and we could leverage that to you all the syntax available for those - but without having a different function for each differently named enum classed table since they all would share a common structure, differing only in name. But the tables would be in user space and not a preordained relation in pg_catalog. Maybe require they all inherit from some template but empty table... We don't have the luxury of being able to redesign this as a green fields development. cheers andrew -- 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] Alter or rename enum value
On 03/26/2016 10:25 AM, Tom Lane wrote: Andrew Dunstan writes: We don't have the luxury of being able to redesign this as a green fields development. I'm not actually convinced that we need to do anything. SQL already has a perfectly good mechanism for enforcing that a column contains only values of a mutable set defined in another table --- it's called a foreign key. The point of inventing enums was to provide a lower-overhead solution for cases where the allowed value set is *not* mutable. So okay, if we can allow certain cases of changing the value set without increasing the overhead, great. But when we can't do it without adding a whole lot of complexity and overhead (and, no doubt, bugs), we need to just say no. Maybe the docs about enums need to be a little more explicit about pointing out this tradeoff. OK, but we (in fact, you and I, for the most part) have provided a way to add new values. The trouble people have is the transaction restriction on that facility, because all the other changes they make with migration tools can be nicely wrapped in a transaction. And the thing is, in my recent experience on a project using quite a few enums, none of the transactions I'd like to include these mutations of enums in does anything that would be at all dangerous. It would be nice if we could find a less broad brush approach to dealing with the issue. cheers andrew -- 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] Alter or rename enum value
On 03/27/2016 12:43 AM, Christophe Pettus wrote: On Mar 26, 2016, at 7:40 AM, Andrew Dunstan wrote: It would be nice if we could find a less broad brush approach to dealing with the issue. I don't know how doable this is, but could we use the existing mechanism of marking an index invalid if it contains an enum type to which a value was added, and the transaction was rolled back? For the 90% use case, that would be acceptable, I would expect. The more I think about this the more I bump up against the fact that almost anything we do might want to do to ameliorate the situation is going to be rolled back. The only approach I can think of that doesn't suffer from this is to abort if an insert/update will affect an index on a modified enum. i.e. we prevent the possible corruption from happening in the first place, as we do now, but in a much more fine grained way. cheers andrew -- 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] raw output from copy
On 03/28/2016 06:26 PM, Tom Lane wrote: Pavel Stehule writes: [ copy-raw-format-20160227-03.patch ] I looked at this patch. I'm having a hard time accepting that it has a use-case large enough to justify it, and here's the reason: it's a protocol break. Conveniently omitting to update protocol.sgml doesn't make it not a protocol break. (libpq.sgml also contains assorted statements that are falsified by this patch.) You could argue that it's the user's own fault if he tries to use COPY RAW with client-side code that hasn't been updated to support it. Maybe that's okay, but I wonder if we're opening ourselves up to problems. Maybe even security-grade problems. In terms of specific code that hasn't been updated, ecpg is broken by this patch, and I'm not very sure what libpq's PQbinaryTuples() ought to do but probably something other than what it does today. There's also a definitional question of what we think PQfformat() ought to do; should it return "2" for the per-field format? Or maybe the per-field format is still "1", since it's after all the same binary data format as for COPY BINARY, and only the overall copy format reported by PQbinaryTuples() should change to "2". BTW, I'm not really sure why the patch is trying to enforce single row and column for the COPY OUT case. I thought the idea for that was that we'd just shove out the data without any delimiters, and if it's more than one datum it's the user's problem whether he can identify the boundaries. On the input side we would have to insist on one column since we're not going to attempt to identify boundaries (and one row would fall out of the fact that we slurp the entire input and treat it as one datum). Anyway this is certainly not committable as-is, so I'm setting it back to Waiting on Author. But the fact that both libpq and ecpg would need updates makes me question whether we can safely pretend that this isn't a protocol break. In that case I humbly submit that there is a case for reviving the psql patch I posted that kicked off this whole thing and lets you export a piece of binary data from psql quite easily. It should certainly not involve any protocol break. cheers andrew -- 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] raw output from copy
On 03/28/2016 11:18 PM, Pavel Stehule wrote: Anyway this is certainly not committable as-is, so I'm setting it back to Waiting on Author. But the fact that both libpq and ecpg would need updates makes me question whether we can safely pretend that this isn't a protocol break. In that case I humbly submit that there is a case for reviving the psql patch I posted that kicked off this whole thing and lets you export a piece of binary data from psql quite easily. It should certainly not involve any protocol break. The psql only solution can work only for output. Doesn't help with input. The I would suggest we try to invent something for psql which does help with it. I just don't see this as an SQL problem. Pretty much any driver library will have no difficulty in handling binary input and output. It's only psql that has an issue, ISTM, and therefore I believe that's where the fix should go. What else is going to use this? As an SQL change this seems like a solution in search of a problem. If someone can make a good case that this is going to be of general use I'll happily go along, but I haven't seen one so far. cheers andrdew -- 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] Alter or rename enum value
On 03/27/2016 10:20 AM, Tom Lane wrote: Andrew Dunstan writes: The more I think about this the more I bump up against the fact that almost anything we do might want to do to ameliorate the situation is going to be rolled back. The only approach I can think of that doesn't suffer from this is to abort if an insert/update will affect an index on a modified enum. i.e. we prevent the possible corruption from happening in the first place, as we do now, but in a much more fine grained way. Perhaps, instead of forbidding ALTER ENUM ADD in a transaction, we could allow that, but not allow the new value to be *used* until it's committed? This could be checked cheaply during enum value lookup (ie, is xmin of the pg_enum row committed). What you really need is to prevent the new value from being inserted into any indexes, but checking that directly seems far more difficult, ugly, and expensive than the above. I do not know whether this would be a meaningful improvement for common use-cases, though. (It'd help if people were more specific about the use-cases they need to work.) I think this is a pretty promising approach, certainly well worth putting some resources into investigating. One thing I like about it is that it gives a nice cheap negative test, so we know if the xmin is committed we are safe. So we could start by rejecting anything where it's not, but later might adopt a more refined but expensive tests for cases where it isn't committed without imposing a penalty on anything else. cheers andrew -- 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] VS 2015 support in src/tools/msvc
On 03/29/2016 08:48 PM, Michael Paquier wrote: On Wed, Mar 30, 2016 at 12:45 AM, Robert Haas wrote: On Tue, Mar 29, 2016 at 11:31 AM, Petr Jelinek wrote: I have machine ready, waiting for animal name and secret. Great! Nice. Thanks. It will obviously fail until we push the 0002 and 0004 though. I think it would be a shame if we shipped 9.6 without MSVC 2015 support, but it'd be nice if Andrew or Magnus could do the actual committing, 'cuz I really don't want to be responsible for breaking the Windows build. I'm fine to back you up as needed. That's not a committer guarantee, still better than nothing I guess. And I make a specialty of looking at VS-related bugs lately :) I am currently travelling, but my intention is to deal with the remaining patches when I'm back home this weekend, unless someone beats me to it. cheers andrew -- 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] [postgresSQL] [bug] Two or more different types of constraints with same name creates ambiguity while drooping.
On 03/30/2016 10:21 AM, Tom Lane wrote: Amit Langote writes: On 2016/03/30 15:16, Harshal Dhumal wrote: If we create two different type of constrains (lets say primary key and foreign key) on same table with same name (lets say 'key' ) then its shows same drop query for both constrains. I have a vague recollection that non-uniqueness of constraint names may have been intentional at some point. But yeah, the existence of the ALTER TABLE DROP CONSTRAINT syntax seems to make that a pretty bad idea. It seems that, whereas name uniqueness check occurs when creating a named FK constraint, the same does not occur when creating a named PK constraint or any index-based constraint for that matter (they are handled by different code paths - in the latter's case, name conflicts with existing relations is checked for when creating the constraint index) I think that if we want to ensure uniqueness of constraint names, this is really approaching it the wrong way, as it still fails to provide any guarantees (consider concurrent index creation, for example). What we need is a unique index on pg_constraint. The problem with that is that pg_constraint contains both table-related and type (domain) related constraints; but it strikes me that we could probably create a unique index on (conrelid, contypid, conname). Given the convention that conrelid is zero in a type constraint and contypid is zero in a table constraint, this should work to enforce per-table or per-type constraint name uniqueness. The cost of an extra index is a bit annoying, but we could probably make it help pay for itself by speeding up assorted searches. +1, but does that mean people will have to change constraint names to be compliant before running pg_upgrade? cheers andrew -- 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] pgindent-polluted commits
On 01/13/2016 12:13 PM, Tom Lane wrote: Simon Riggs writes: On 13 January 2016 at 14:48, Noah Misch wrote: I've noticed commits, from a few of you, carrying pgindent changes to lines the patch would not otherwise change. Could we review again why this matters? Basically this is trading off convenience of the committer (all of the alternatives Noah mentions are somewhat annoying) versus the convenience of post-commit reviewers. I'm not sure that his recommendation is the best trade-off, nor that the situation is precisely comparable to pre-commit review. There definitely will be pre-commit review, there may or may not be any post-commit review. I'm willing to go with the "separate commit to reindent individual files" approach if there's a consensus that that makes for a cleaner git history. But I'm not 100% convinced it matters. I do think it makes life easier when going through the git history if semantic changes are separated from formatting changes. cheers andrew -- 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] SET syntax in INSERT
On 01/14/2016 03:00 PM, Marko Tiikkaja wrote: On 2016-01-14 20:50, Vitaly Burovoy wrote: On 1/14/16, Tom Lane wrote: Assume a table with an int-array column, and consider INSERT INTO foo SET arraycol[2] = 7, arraycol[4] = 11; Right part is a column name, not an expression. Isn't it? So "arraycol[2]" is not possible there. I think the idea here was that it's allowed in UPDATE. But I don't see the point of allowing that in an INSERT. Right. Why not just forbid anything other than a plain column name on the LHS for INSERT, at least as a first cut. cheers andrew -- 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] Removing service-related code in pg_ctl for Cygwin
On 01/14/2016 12:38 AM, Michael Paquier wrote:
Hi all,
Beginning a new thread seems more adapted regarding $subject but
that's mentioned here as well:
http://www.postgresql.org/message-id/CAB7nPqQXghm_SdB5iniupz1atzMxk=95gv9a8ocdo83sxcn...@mail.gmail.com
It happens based on some investigation from Andrew that cygwin does
not need to use the service-related code, aka register/unregister
options or similar that are proper to WIN32 and rely instead on
cygrunsrv with a SYSV-like init file to manage the service. Based on
my tests with cygwin, I am able to see as well that a server started
within a cygwin session is able to persist even after it is closed,
which is kind of nice and I think that it is a additional reason to
not use the service-related code paths. Hence what about the following
patch, that makes cygwin behave like any *nix OS when using pg_ctl?
This simplifies a bit the code.
Marco, as I think you do some packaging for Postgres in Cygwin, and
others, does that sound acceptable?
I think we can be a bit more adventurous and remove all the
Cygwin-specific code. See attached patch, which builds fine on buildfarm
cockatiel.
cheers
andrew
diff --git a/src/bin/pg_ctl/pg_ctl.c b/src/bin/pg_ctl/pg_ctl.c
index 919d764..98aa1a0 100644
--- a/src/bin/pg_ctl/pg_ctl.c
+++ b/src/bin/pg_ctl/pg_ctl.c
@@ -39,13 +39,6 @@
#include "getopt_long.h"
#include "miscadmin.h"
-#if defined(__CYGWIN__)
-#include
-#include
-/* Cygwin defines WIN32 in windows.h, but we don't want it. */
-#undef WIN32
-#endif
-
/* PID can be negative for standalone backend */
typedef long pgpid_t;
@@ -105,7 +98,7 @@ static char backup_file[MAXPGPATH];
static char recovery_file[MAXPGPATH];
static char promote_file[MAXPGPATH];
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef WIN32
static DWORD pgctl_start_type = SERVICE_AUTO_START;
static SERVICE_STATUS status;
static SERVICE_STATUS_HANDLE hStatus = (SERVICE_STATUS_HANDLE) 0;
@@ -133,7 +126,7 @@ static void do_kill(pgpid_t pid);
static void print_msg(const char *msg);
static void adjust_data_dir(void);
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef WIN32
#if (_MSC_VER >= 1800)
#include
#else
@@ -165,7 +158,7 @@ static void unlimit_core_size(void);
#endif
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef WIN32
static void
write_eventlog(int level, const char *line)
{
@@ -207,20 +200,11 @@ write_stderr(const char *fmt,...)
va_list ap;
va_start(ap, fmt);
-#if !defined(WIN32) && !defined(__CYGWIN__)
+#ifndef WIN32
/* On Unix, we just fprintf to stderr */
vfprintf(stderr, fmt, ap);
#else
-/*
- * On Cygwin, we don't yet have a reliable mechanism to detect when
- * we're being run as a service, so fall back to the old (and broken)
- * stderr test.
- */
-#ifdef __CYGWIN__
-#define pgwin32_is_service() (isatty(fileno(stderr)))
-#endif
-
/*
* On Win32, we print to stderr if running on a console, or write to
* eventlog if running as a service
@@ -718,7 +702,7 @@ test_postmaster_connection(pgpid_t pm_pid, bool do_checkpoint)
#endif
/* No response, or startup still in process; wait */
-#if defined(WIN32)
+#ifdef WIN32
if (do_checkpoint)
{
/*
@@ -1342,7 +1326,7 @@ do_kill(pgpid_t pid)
}
}
-#if defined(WIN32) || defined(__CYGWIN__)
+#ifdef WIN32
#if (_MSC_VER < 1800)
static bool
@@ -1408,20 +1392,6 @@ pgwin32_CommandLine(bool registration)
}
}
-#ifdef __CYGWIN__
- /* need to convert to windows path */
- {
- char buf[MAXPGPATH];
-
-#if CYGWIN_VERSION_DLL_MAJOR >= 1007
- cygwin_conv_path(CCP_POSIX_TO_WIN_A, cmdPath, buf, sizeof(buf));
-#else
- cygwin_conv_to_full_win32_path(cmdPath, buf);
-#endif
- strcpy(cmdPath, buf);
- }
-#endif
-
/* if path does not end in .exe, append it */
if (strlen(cmdPath) < 4 ||
pg_strcasecmp(cmdPath + strlen(cmdPath) - 4, ".exe") != 0)
@@ -1775,10 +1745,8 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
if (!OpenProcessToken(GetCurrentProcess(), TOKEN_ALL_ACCESS, &origToken))
{
/*
- * Most Windows targets make DWORD a 32-bit unsigned long. Cygwin
- * x86_64, an LP64 target, makes it a 32-bit unsigned int. In code
- * built for Cygwin as well as for native Windows targets, cast DWORD
- * before printing.
+ * Most Windows targets make DWORD a 32-bit unsigned long, but
+ * in case it doesn't cast DWORD before printing.
*/
write_stderr(_("%s: could not open process token: error code %lu\n"),
progname, (unsigned long) GetLastError());
@@ -1819,10 +1787,6 @@ CreateRestrictedProcess(char *cmd, PROCESS_INFORMATION *processInfo, bool as_ser
return 0;
}
-#ifndef __CYGWIN__
- AddUserToTokenDacl(restrictedToken);
-#endif
-
r = CreateProcessAsUser(restrictedToken, NULL, cmd, NULL, NULL, TRUE, CREATE_SUSPENDED, NULL, NULL, &si, processInfo);
Kernel32Handle = LoadLibrary("KERNEL32.DLL");
Re: [HACKERS] Removing service-related code in pg_ctl for Cygwin
On 01/18/2016 12:38 PM, Alvaro Herrera wrote: Andrew Dunstan wrote: I think we can be a bit more adventurous and remove all the Cygwin-specific code. See attached patch, which builds fine on buildfarm cockatiel. Hopefully you also tested that it builds under MSVC :-) Why would I? This isn't having the slightest effect on MSVC builds. cheers andrew -- 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] Removing service-related code in pg_ctl for Cygwin
On 01/18/2016 03:46 PM, Alvaro Herrera wrote: Andrew Dunstan wrote: On 01/18/2016 12:38 PM, Alvaro Herrera wrote: Andrew Dunstan wrote: I think we can be a bit more adventurous and remove all the Cygwin-specific code. See attached patch, which builds fine on buildfarm cockatiel. Hopefully you also tested that it builds under MSVC :-) Why would I? This isn't having the slightest effect on MSVC builds. You never know ... you might have inadvertently broken some WIN32 block. If you can point out a line where that might be true I'll test it ;-) cheers andrew -- 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] system mingw not recognized
On 01/18/2016 04:11 PM, Igal @ Lucee.org wrote: I posted the error in the docs to pgsql-d...@postgresql.org If it's possible to update it myself via git, or if it should be reported elsewhere -- please advise. 1. Please don't top-post on the PostgreSQL lists. See <http://idallen.com/topposting.html>> 2. You've done all you need to do. We'll take care of it. Thanks for the report. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Buildfarm server move
People, Apologies for the late notice. Tomorrow, January 18th, at 4.00 pm US East Coast time (UT - 5.0) we will be moving the buildfarm server from its current home at CommandPrompt, where we have been ever since we started, to a machine that is part of the standard core infrastructure. In doing so we will be moving to a) a more modern and supported PostgreSQL version, and b) a machine with more disk space so that our current severe pace shortage will be alleviated. In addition, the community would be much better placed to maintain the buildfarm if either JD or I were to fall under a bus. The outage is expected to last about 4 hours or less, and we will sent out notifications when this is complete. Buildfarm owners who want to avoid getting reporting failures should disable their animals during that time. We don't have an avalanche of commits right now either, but it might also be nice if committers were to refrain from adding changes in the hours leading up to this and until we announce that we're back online, for the benefit of those owners who don't see this message in time. Thanks in advance for your help and understanding. And many thanks to CommandPrompt for their constant support over the many years we've been in operation. cheers andrew -- 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] Buildfarm server move
On 01/18/2016 05:20 PM, Andrew Dunstan wrote: People, Apologies for the late notice. Tomorrow, January 18th, at 4.00 pm US East Coast time (UT - 5.0) we will be moving the buildfarm server from its current home at CommandPrompt, where we have been ever since we started, to a machine that is part of the standard core infrastructure. In doing so we will be moving to a) a more modern and supported PostgreSQL version, and b) a machine with more disk space so that our current severe pace shortage will be alleviated. In addition, the community would be much better placed to maintain the buildfarm if either JD or I were to fall under a bus. The outage is expected to last about 4 hours or less, and we will sent out notifications when this is complete. Buildfarm owners who want to avoid getting reporting failures should disable their animals during that time. We don't have an avalanche of commits right now either, but it might also be nice if committers were to refrain from adding changes in the hours leading up to this and until we announce that we're back online, for the benefit of those owners who don't see this message in time. Thanks in advance for your help and understanding. And many thanks to CommandPrompt for their constant support over the many years we've been in operation. Yes, sorry about that. It will be on the 19th. Fat fingers strike again. cheers andrew -- 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] make error - libpqdll.def No such file or directory
On 01/19/2016 01:08 PM, Igal @ Lucee.org wrote:
On 1/17/2016 8:17 PM, Igal @ Lucee.org wrote:
On 1/17/2016 3:24 PM, Igal @ Lucee.org wrote:
When running make I encounter the following error:
gcc.exe: error: libpqdll.def: No such file or directory
/home/Admin/sources/postgresql-9.5.0/src/Makefile.shlib:393: recipe
for target 'libpq.dll' failed
make[3]: *** [libpq.dll] Error 1
make[3]: Leaving directory
'/home/Admin/builds/postgresql-9.5.0/src/interfaces/libpq'
Makefile:17: recipe for target 'all-libpq-recurse' failed
make[2]: *** [all-libpq-recurse] Error 2
make[2]: Leaving directory
'/home/Admin/builds/postgresql-9.5.0/src/interfaces'
Makefile:34: recipe for target 'all-interfaces-recurse' failed
make[1]: *** [all-interfaces-recurse] Error 2
make[1]: Leaving directory '/home/Admin/builds/postgresql-9.5.0/src'
GNUmakefile:11: recipe for target 'all-src-recurse' failed
make: *** [all-src-recurse] Error 2
But /home/Admin/builds/postgresql-9.5.0/src/interfaces/libpq does
contain the file libpqdll.def
The file /home/Admin/sources/postgresql-9.5.0/src/Makefile.shlib
exists as well.
It's hard for me to decipher which file is reporting the error and
which file is not found.
Any feedback would be greatly appreciated, thanks!
So when I try to run `make` I still get that error. Please note that
I am doing a VPATH build (the build in a separate directory from the
downloaded sources), which might play a role here:
x86_64-w64-mingw32-gcc.exe: error: libpqdll.def: No such file or directory
make[3]: *** [libpq.dll] Error 1
make[2]: *** [all-libpq-recurse] Error 2
make[1]: *** [all-interfaces-recurse] Error 2
make: *** [all-src-recurse] Error 2
I found a script that builds postgresql via MinGW-w64, and in it the
author specifically creates symlinks to libpqdll.def
https://github.com/Alexpux/MINGW-packages/blob/master/mingw-w64-postgresql/PKGBUILD#L72
-- excerpt below:
# Make DLL definition file visible during each arch build
ln -s
"${srcdir}/postgresql-$pkgver/src/interfaces/libpq/libpqdll.def"
src/interfaces/libpq/
ln -s
"${srcdir}/postgresql-$pkgver/src/interfaces/ecpg/ecpglib/libecpgdll.def"
src/interfaces/ecpg/ecpglib/
ln -s
"${srcdir}/postgresql-$pkgver/src/interfaces/ecpg/pgtypeslib/libpgtypesdll.def"
src/interfaces/ecpg/pgtypeslib/
ln -s
"${srcdir}/postgresql-$pkgver/src/interfaces/ecpg/compatlib/libecpg_compatdll.def"
src/interfaces/ecpg/compatlib/
Why are the symlinks needed to make the definition files visible?
Is this an issue with VPATH builds? It is not mentioned in the docs
where VPATH builds are discussed (section 15.4
http://www.postgresql.org/docs/9.5/static/install-procedure.html )
jacana does VPATH builds with pretty much this setup all the time. See
for example
<http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=jacana&dt=2016-01-19%2013%3A00%3A09>
cheers
andrew
--
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] make error - libpqdll.def No such file or directory
On 01/19/2016 02:01 PM, Alvaro Herrera wrote: Andrew Dunstan wrote: jacana does VPATH builds with pretty much this setup all the time. See for example <http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=jacana&dt=2016-01-19%2013%3A00%3A09> Yes, it builds a tree *once*, then deletes the result, and the next BF run uses a completely new build directory. So any issues resulting from re-building an existing tree are never seen. Oh. odd. I don't think I've seen that even when developing on Windows (e.g. parallel pg_restore). Maybe I always did that in-tree. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Buildfarm server move complete
The buildfarm server move is complete. Thanks to all who helped, especially Stephen Frost. There might be some small performance regressions which we'll be digging into. Next step: move the mailing lists off pgfoundry. The new lists have been set up I will be working on that migration next week. cheers andrew -- 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] Template for commit messages
On 01/28/2016 09:57 AM, Robert Haas wrote: On Thu, Jan 28, 2016 at 9:52 AM, Tom Lane wrote: Robert Haas writes: On Thu, Jan 28, 2016 at 8:04 AM, Tomas Vondra wrote: Why can't we do both? That is, have a free-form text with the nuances, and then Reviewed-By listing the main reviewers? The first one is for humans, the other one for automated tools. I'm not objecting to or endorsing any specific proposal, just asking what we want to do about this. I think the trick if we do it that way will be to avoid having it seem like too much duplication, but there may be a way to manage that. FWIW, I'm a bit suspicious of the idea that we need to make the commit messages automated-tool-friendly. What tools are there that would need to extract this info, and would we trust them if they didn't understand "nuances"? I'm on board with Bruce's template as being a checklist of points to be sure to cover when composing a commit message. I'm not sure we need fixed-format rules. Well, I think what people are asking for is precisely a fixed format, and I do think there is value in that. It's nice to capture the nuance, but the nuance is going to get flattened out anyway when the release notes are created. If we all agree to use a fixed format, then a bunch of work there that probably has to be done manually can be automated. However, if we all agree to use a fixed format except for you, we might as well just forget the whole thing, because the percentage of commits that are done by you is quite high. Yeah. I have no prejudice in this area, other than one in favor of any rules being fairly precise. As for nuances, I guess they can be specified in the commit message. The one thing I do find annoying from time to time is the limit on subject size. Sometimes it's very difficult to be sufficiently communicative in, say, 50 characters. cheers andrew -- 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] proposal: make NOTIFY list de-duplication optional
On 02/05/2016 08:49 PM, Tom Lane wrote: Yeah, I agree that a GUC for this is quite unappetizing. Agreed. One idea would be to build a hashtable to aid with duplicate detection (perhaps only once the pending-notify list gets long). Another thought is that it's already the case that duplicate detection is something of a "best effort" activity; note for example the comment in AsyncExistsPendingNotify pointing out that we don't collapse duplicates across subtransactions. Would it be acceptable to relax the standards a bit further? For example, if we only checked for duplicates among the last N notification list entries (for N say around 100), we'd probably cover just about all the useful cases, and the runtime would stay linear. The data structure isn't tremendously conducive to that, but it could be done. I like the hashtable idea if it can be made workable. cheers andrew -- 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] proposal: schema PL session variables
On 02/08/2016 03:16 AM, Pavel Stehule wrote: Hi On Russian PgConf I had a talk with Oleg about missing features in PLpgSQL, that can complicates a migrations from Oracle to PostgreSQL. Currently I see only one blocker - missing protected session variables. PL/SQL has package variables with possible only package scope and session life cycle. Currently we cannot to ensure/enforce schema scope visibility - and we cannot to implement this functionality in PL languages other than C. I propose really basic functionality, that can be enhanced in future - step by step. This proposal doesn't contain any controversial feature or syntax, I hope. It is related to PLpgSQL only, but described feature can be used from any PL languages with implemented interface. I think it would make sense to implement the interface in at least one of our other supported PLs. I'm not entirely clear how well this will match up with, say, plperl, but I'd be interested to see. cheers andrew -- 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] Tracing down buildfarm "postmaster does not shut down" failures
On 02/08/2016 02:15 PM, Tom Lane wrote: Of late, by far the majority of the random-noise failures we see in the buildfarm have come from failure to shut down the postmaster in a reasonable timeframe. An example is this current failure on hornet: http://buildfarm.postgresql.org/cgi-bin/show_stage_log.pl?nm=hornet&dt=2016-02-08%2013%3A41%3A55 waiting for server to shut down... failed pg_ctl: server does not shut down === db log file == 2016-02-08 15:14:35.783 UTC [56b8b00d.9e00e2:2] LOG: received fast shutdown request 2016-02-08 15:14:35.783 UTC [56b8b00d.9e00e2:3] LOG: aborting any active transactions 2016-02-08 15:14:35.783 UTC [56b8b00d.7e0028:2] LOG: autovacuum launcher shutting down 2016-02-08 15:14:35.865 UTC [56b8b00d.2e5000a:1] LOG: shutting down The buildfarm script runs pg_ctl stop with -t 120, and counting the dots shows that pg_ctl was honoring that, so apparently the postmaster took more than 2 minutes to shut down. Looking at other recent successful runs, stopdb-C-1 usually takes 30 to 40 or so seconds on this machine. So it's possible that it was just so overloaded that it took three times that much time on this run, but I'm starting to think there may be more to it than that. We've seen variants on this theme on half a dozen machines just in the past week --- and it seems to mostly happen in 9.5 and HEAD, which is fishy. The fact that we got "shutting down" but not "database system is shut down" indicates that the server was in ShutdownXLOG(). Normally the only component of that that takes much time is buffer flushing, but could something else be happening? Or perhaps the checkpoint code has somehow not gotten the word to do its flushing at full speed? What I'd like to do to investigate this is put in a temporary HEAD-only patch that makes ShutdownXLOG() and its subroutines much chattier about how far they've gotten and what time it is, and also makes pg_ctl print out the current time if it gives up waiting. A few failed runs with that in place will at least allow us to confirm or deny whether it's just that the shutdown checkpoint is sometimes really slow, or whether there's a bug lurking. Any objections? Anybody have another idea for data to collect? I think that's an excellent idea. This has been a minor running sore for ages on slow machines. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] [Proposal] Improvement of GiST page layout
Hi, hackers! I want to propose improvement of GiST page layout. GiST is optimized to reduce disk operations on search queries, for example, windows search queries in case of R-tree. I expect that more complicated page layout will help to tradeoff some of CPU efficiency for disk efficiency. GiST tree is a balanced tree structure with two kinds of pages: internal pages and leaf pages. Each tree page contains bunch of tuples with the same structure. Tuples of an internal page reference other pages of the same tree, while a leaf page tuples holds heap TIDs. During execution of search query GiST for each tuple on page invokes key comparison algorithm with two possible outcomes: 'no' and 'may be'. 'May be' answer recursively descends search algorithms to referenced page (in case of internal page) or yields tuple (in case of leaf page). Expected tuples count on page is around of tenth to hundreds. This count is big enough to try to save some cache lines from loading into CPU during enumeration. For B-trees during inspection of a page we effectively apply binary search algorithm, which is not possible in GiST tree. Let's consider R-tree with arbitrary fan-out f. If a given query will find exactly one data tuple, it is easily to show that keys comparison count is minimal if f->e /*round_to_optimal(2.78) == 3*/ (tree have to review f*h keys, h=logf(N), f*logf(N) is minimal when f->e). Smaller keys comparison count means less cache lines are touched. So fan-out reduction means cache pressure reduction (except avg fan-out 2, which seems to be too small) and less time waiting for RAM. I suppose, all that reasoning holds true in a cases when not just one tuple will be found. How do we reduce tree fan-out? Obviously, we can’t fill page with just 3 tuples. But we can install small tree-like structure inside one page. General GiST index has root page. But a page tree should have “root” layer of tuples. Private (or internal, intermediate, auxiliary, I can’t pick precise word) tuples have only keys and fixed-size(F) array of underlying records offsets. Each layer is linked-list. After page have just been allocated there is only “ground” level of regular tuples. Eventually record count reaches F-1 and we create new root layer with two tuples. Each new tuple references half of preexisting records. Placement of new “ground” tuples on page eventually will cause internal tuple to split. If there is not enough space to spilt internal tuple we mark page for whole page-split during next iteration of insertion algorithms of owning tree. That is why tuple-spilt happens on F-1 tuples, not on F: if we have no space for splitting, we just adding reference to last slot. In this algorithm, page split will cause major page defragmentation: we take root layer, halve it and place halves on different pages. When half of a data is gone to other page, restructuration should tend to place records in such a fashion that accessed together tuples lie together. I think, placing whole level together is a good strategy. Let’s look how page grows with fan-out factor F=5. RLS – root layer start, G – ground tuple, Ix – internal tuple of level x. When we added 3 ground tuples it’s just a ground layer RLS=0|G G G Then we place one more tuple and layer splits: RLS=4|G G G G I0 I0 Each I0 tuple now references two G tuples. We keep placing G tuples. RLS=4|G G G G I0 I0 G G And then one of I0 tuples is spitted RLS=4|G G G G I0 I0 G G G I0 And right after one more I0 split causes new layer RLS=12|G G G G I0 I0 G G G I0 G I0 I1 I1 And so on, until we have space on a page. In a regular GiST we ran out of space on page before we insert tuple on page. Now we can run out of space during insertion. But this will not be fatal, we still will be able to place ground level tuple. Inner index structure will use one-extra slot for reference allocation, but next insertion on a page will deal with it. On a pages marked for split we have to find which exactly index tuple has run out of extra-slot during split and fix it. Several years ago I had unsuccessful attempt to implement akin algorithm in a database engine of a proprietary system. I stuck in the debug of deletion algorithm and tree condensation, it is in use in that system. I suppose it was a mistake to defrag page with creeping heuristic, eventually I dropped the idea and just moved on to actually important tasks, there is always deadline rush in business. I’m newbie in Postgres internals. But as I see there is no deletion on GiST page. So I feel itch to try this feature one more time. I expect that implementation of this proposal could speed up insertions into index by 20% and performance of queries by 30% when all index accommodates in shared buffer. In case of buffer starvation, when index is accessed through disk this feature will cause 15% performance degrade since effective page capacity will be smaller. Should this feature be configurable? May be this should be other access method? I need help to
Re: [HACKERS] Tracing down buildfarm "postmaster does not shut down" failures
On 02/08/2016 10:55 PM, Tom Lane wrote: Noah Misch writes: On Mon, Feb 08, 2016 at 02:15:48PM -0500, Tom Lane wrote: We've seen variants on this theme on half a dozen machines just in the past week --- and it seems to mostly happen in 9.5 and HEAD, which is fishy. It has been affecting only the four AIX animals, which do share hardware. (Back in 2015 and once in 2016-01, it did affect axolotl and shearwater.) Certainly your AIX critters have shown this a bunch, but here's another current example: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=axolotl&dt=2016-02-08%2014%3A49%3A23 That's reasonable. If you would like higher-fidelity data, I can run loops of "pg_ctl -w start; make installcheck; pg_ctl -t900 -w stop", and I could run that for HEAD and 9.2 simultaneously. A day of logs from that should show clearly if HEAD is systematically worse than 9.2. That sounds like a fine plan, please do it. So, I wish to raise the timeout for those animals. Using an environment variable was a good idea; it's one less thing for test authors to remember. Since the variable affects a performance-related fudge factor rather than change behavior per se, I'm less skittish than usual about unintended consequences of dynamic scope. (With said unintended consequences in mind, I made "pg_ctl register" ignore PGCTLTIMEOUT rather than embed its value into the service created.) While this isn't necessarily a bad idea in isolation, the current buildfarm scripts explicitly specify a -t value to pg_ctl stop, which I would not expect an environment variable to override. So we need to fix the buildfarm script to allow the timeout to be configurable. I'm not sure if there would be any value-add in having pg_ctl answer to an environment variable once we've done that. The failure on axolotl was for the ECPG tests, which don't use the buildfarm's startup/stop db code. They don't honour TEMP_CONFIG either, which they probably should - then we might get better log traces. cheers andrew -- 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] Tracing down buildfarm "postmaster does not shut down" failures
On 02/09/2016 03:05 PM, Tom Lane wrote: I wrote: In any case, we should proceed with fixing things so that buildfarm owners can specify a higher shutdown timeout for especially slow critters. I looked into doing this as I suggested yesterday, namely modifying the buildfarm scripts, and soon decided that it would be a mess; there are too many cases where "pg_ctl stop" is not invoked directly by the script. I'm now in favor of applying the PGCTLTIMEOUT patch Noah proposed, and *removing* the two existing hacks in run_build.pl that try to force -t 120. The only real argument I can see against that approach is that we'd have to back-patch the PGCTLTIMEOUT patch to all active branches if we want to stop the buildfarm failures. We don't usually back-patch feature additions. On the other hand, this wouldn't be the first time we've back-patched something on grounds of helping the buildfarm, so I find that argument pretty weak. OK. I can put out a new release as required. cheers andrew -- 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] Tracing down buildfarm "postmaster does not shut down" failures
On 02/09/2016 05:53 PM, Tom Lane wrote: Andrew, I wonder if I could prevail on you to make axolotl run "make check" on HEAD in src/interfaces/ecpg/ until it fails, so that we can see if the logging I added tells anything useful about this. Will do. cheers andrew -- 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] Tracing down buildfarm "postmaster does not shut down" failures
On 02/09/2016 06:46 PM, Andrew Dunstan wrote:
On 02/09/2016 05:53 PM, Tom Lane wrote:
Andrew, I wonder if I could prevail on you to make axolotl run "make
check" on HEAD in src/interfaces/ecpg/ until it fails, so that we can
see if the logging I added tells anything useful about this.
Will do.
Incidentally, as I noted earlier, the ecpg tests don't honour
TEMP_CONFIG, and in axolotl's case this could well make a difference, as
it it set up like this:
extra_config =>{
DEFAULT => [
q(log_line_prefix = '%m [%c:%l] '),
"log_connections = 'true'",
"log_disconnections = 'true'",
"log_statement = 'all'",
"fsync = off",
"stats_temp_directory='/home/andrew/bf/root/stats_temp/$branch'"
],
},
So running it's not running with fsync off or using the ramdisk for
stats_temp_directory. Of course, that doesn't explain why we're not
seeing it on branches earlier than 9.5, but it could explain why we're
only seeing it on the ecpg tests.
cheers
andrew
--
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] Tracing down buildfarm "postmaster does not shut down" failures
On 02/09/2016 07:49 PM, Tom Lane wrote: Andrew Dunstan writes: So running it's not running with fsync off or using the ramdisk for stats_temp_directory. Of course, that doesn't explain why we're not seeing it on branches earlier than 9.5, but it could explain why we're only seeing it on the ecpg tests. BTW, the evidence we already have from axolotl's last run is that post-checkpoint shutdown in the ecpg test was pretty quick: LOG: database system is shut down at 2016-02-09 16:31:14.784 EST LOG: lock files all released at 2016-02-09 16:31:14.817 EST However, I'd already noted from some other digging in the buildfarm logs that axolotl's speed seems to vary tremendously. I do not know what else you typically run on that hardware, but putting it under full load might help prove things. Almost nothing. It's a Raspberry Pi 2B that I got mainly to run a buildfarm animal. About the only other thing of note is a very lightly configured Nagios instance. Of course, I could put it under continuous load running a compilation or something. cheers andrew -- 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] Tracing down buildfarm "postmaster does not shut down" failures
On 02/09/2016 08:49 PM, Tom Lane wrote: Andrew Dunstan writes: On 02/09/2016 07:49 PM, Tom Lane wrote: However, I'd already noted from some other digging in the buildfarm logs that axolotl's speed seems to vary tremendously. I do not know what else you typically run on that hardware, but putting it under full load might help prove things. Almost nothing. It's a Raspberry Pi 2B that I got mainly to run a buildfarm animal. About the only other thing of note is a very lightly configured Nagios instance. Huh, that's quite strange. There is one fairly recent report of axolotl failing "make check" because of taking over a minute to shut down: http://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=axolotl&dt=2015-12-14%2020%3A30%3A52 but the runs before and after that show shutdown times of only a second or two. No idea why. anyway, we got a failure pretty quickly: pg_ctl: server does not shut down at 2016-02-09 21:10:11.914 EST pg_regress: could not stop postmaster: exit code was 256 Makefile:81: recipe for target 'check' failed make: *** [check] Error 2 The log traces from the shutdown are below. cheers andrew LOG: received fast shutdown request at 2016-02-09 21:09:11.824 EST LOG: aborting any active transactions LOG: autovacuum launcher shutting down at 2016-02-09 21:09:11.830 EST LOG: shutting down at 2016-02-09 21:09:11.839 EST LOG: checkpoint starting: shutdown immediate LOG: CheckPointGuts starting at 2016-02-09 21:09:11.840 EST LOG: CheckPointCLOG() done at 2016-02-09 21:09:11.840 EST LOG: CheckPointCommitTs() done at 2016-02-09 21:09:11.840 EST LOG: CheckPointSUBTRANS() done at 2016-02-09 21:09:11.841 EST LOG: CheckPointMultiXact() done at 2016-02-09 21:09:11.841 EST LOG: CheckPointPredicate() done at 2016-02-09 21:09:11.841 EST LOG: CheckPointRelationMap() done at 2016-02-09 21:09:11.841 EST LOG: CheckPointReplicationSlots() done at 2016-02-09 21:09:11.841 EST LOG: CheckPointSnapBuild() done at 2016-02-09 21:09:11.841 EST LOG: CheckPointLogicalRewriteHeap() done at 2016-02-09 21:09:11.842 EST LOG: BufferSync(5) beginning to write 172 buffers at 2016-02-09 21:09:11.845 EST LOG: BufferSync(5) done, wrote 172/172 buffers at 2016-02-09 21:09:14.635 EST LOG: CheckPointBuffers() done at 2016-02-09 21:09:14.636 EST LOG: CheckPointReplicationOrigin() done at 2016-02-09 21:09:14.636 EST LOG: CheckPointGuts done at 2016-02-09 21:09:14.636 EST LOG: checkpoint WAL record flushed at 2016-02-09 21:09:14.636 EST LOG: pg_control updated at 2016-02-09 21:09:14.637 EST LOG: smgrpostckpt() done at 2016-02-09 21:09:14.668 EST LOG: RemoveOldXlogFiles() done at 2016-02-09 21:09:14.668 EST LOG: TruncateSUBTRANS() done at 2016-02-09 21:09:14.669 EST LOG: checkpoint complete: wrote 172 buffers (1.0%); 0 transaction log file(s) added, 0 removed, 0 recycled; write=2.794 s, sync=0.000 s, total=2.829 s; sync files=0, longest=0.000 s, average=0.000 s; distance=966 kB, estimate=966 kB LOG: shutdown checkpoint complete at 2016-02-09 21:09:14.669 EST LOG: ShutdownCLOG() complete at 2016-02-09 21:09:14.669 EST LOG: ShutdownCommitTs() complete at 2016-02-09 21:09:14.669 EST LOG: ShutdownSUBTRANS() complete at 2016-02-09 21:09:14.669 EST LOG: database system is shut down at 2016-02-09 21:09:14.669 EST LOG: doing before_shmem_exit 0 at 2016-02-09 21:09:14.670 EST LOG: doing on_shmem_exit 2 at 2016-02-09 21:09:14.670 EST LOG: doing on_shmem_exit 1 at 2016-02-09 21:09:14.670 EST LOG: doing on_shmem_exit 0 at 2016-02-09 21:09:14.670 EST LOG: doing on_proc_exit 1 at 2016-02-09 21:09:14.670 EST LOG: doing on_proc_exit 0 at 2016-02-09 21:09:14.671 EST LOG: calling exit(0) at 2016-02-09 21:09:14.671 EST LOG: checkpointer dead at 2016-02-09 21:09:14.683 EST LOG: all children dead at 2016-02-09 21:10:11.184 EST LOG: doing on_shmem_exit 3 at 2016-02-09 21:10:11.191 EST LOG: doing on_shmem_exit 2 at 2016-02-09 21:10:11.191 EST LOG: doing on_shmem_exit 1 at 2016-02-09 21:10:11.192 EST LOG: doing on_shmem_exit 0 at 2016-02-09 21:10:11.208 EST LOG: doing on_proc_exit 1 at 2016-02-09 21:10:11.209 EST LOG: doing on_proc_exit 0 at 2016-02-09 21:10:11.209 EST LOG: lock files all released at 2016-02-09 21:10:11.211 EST LOG: calling exit(0) at 2016-02-09 21:10:11.211 EST -- 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] Tracing down buildfarm "postmaster does not shut down" failures
On 02/09/2016 10:27 PM, Tom Lane wrote: Noah Misch writes: On Tue, Feb 09, 2016 at 10:02:17PM -0500, Tom Lane wrote: I wonder if it's worth sticking some instrumentation into stats collector shutdown? I wouldn't be surprised if the collector got backlogged during the main phase of testing and took awhile to chew through its message queue before even starting the write of the final stats. But why would the ecpg tests show such an effect when the main regression tests don't? AFAIK the ecpg tests don't exactly stress the server --- note the trivial amount of data written by the shutdown checkpoint, for instance. The main regression tests run with the stats file on the ramdisk. The other weird thing is that it's only sometimes slow. If you look at the last buildfarm result from axolotl, for instance, the tail end of the ecpg log is LOG: ShutdownSUBTRANS() complete at 2016-02-09 16:31:14.784 EST LOG: database system is shut down at 2016-02-09 16:31:14.784 EST LOG: lock files all released at 2016-02-09 16:31:14.817 EST so we only spent ~50ms on stats write that time. That part is puzzling. The idea I was toying with is that previous filesystem activity (making the temp install, the server's never-fsync'd writes, etc) has built up a bunch of dirty kernel buffers, and at some point the kernel goes nuts writing all that data. So the issues we're seeing would come and go depending on the timing of that I/O spike. I'm not sure how to prove such a theory from here. Yeah. It's faintly possible that a kernel upgrade will help. cheers andrew -- 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] Tracing down buildfarm "postmaster does not shut down" failures
On 02/09/2016 11:21 PM, Andrew Dunstan wrote: The idea I was toying with is that previous filesystem activity (making the temp install, the server's never-fsync'd writes, etc) has built up a bunch of dirty kernel buffers, and at some point the kernel goes nuts writing all that data. So the issues we're seeing would come and go depending on the timing of that I/O spike. I'm not sure how to prove such a theory from here. Yeah. It's faintly possible that a kernel upgrade will help. Another data point. I have another RPi2B that is running Debian Wheezy rather than the Fedora remix. I'm running the same test on it we ran yesterday on axolotl. It seems to be running without having the same problems. So maybe the particular kernel port to ARM is what's tripping us up. cheers andrew -- 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] Tracing down buildfarm "postmaster does not shut down" failures
On 02/10/2016 12:53 PM, Tom Lane wrote: Andrew Dunstan writes: Yeah. It's faintly possible that a kernel upgrade will help. Another data point. I have another RPi2B that is running Debian Wheezy rather than the Fedora remix. I'm running the same test on it we ran yesterday on axolotl. It seems to be running without having the same problems. So maybe the particular kernel port to ARM is what's tripping us up. I'd bet against it being a port-specific problem; it sounds more like a filesystem performance-tuning issue, which could easily change from one kernel version to another. What are the kernel version numbers exactly? axolotl: Linux pirouette 3.18.13-501.20150510gitf36e19f.sc20.armv7hl.bcm2709 #1 SMP PREEMPT debian: Linux piratic 3.18.7-v7+ #755 SMP PREEMPT cheers andrew -- 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] postgres_fdw vs. force_parallel_mode on ppc
On 02/15/2016 07:57 PM, Tom Lane wrote: Noah Misch writes: On Mon, Feb 15, 2016 at 07:31:40PM -0500, Robert Haas wrote: Oh, crap. I didn't realize that TEMP_CONFIG didn't affect the contrib test suites. Is there any reason for that, or is it just kinda where we ended up? To my knowledge, it's just the undesirable place we ended up. Yeah. +1 for fixing that, if it's not unreasonably painful. +1 for fixing it everywhere. Historical note: back when TEMP_CONFIG was implemented, the main regression set was just about the only set of tests the buildfarm ran using a temp install. That wasn't even available for contrib and the PLs, IIRC. cheers andrew -- 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] exposing pg_controldata and pg_config as functions
On 02/17/2016 05:14 PM, Tom Lane wrote: Peter Eisentraut writes: On 2/17/16 12:15 PM, Joe Conway wrote: Ok, removed the documentation on the function pg_config() and pushed. I still have my serious doubts about this, especially not even requiring superuser access for this information. Could someone explain why we need this? I thought we'd agreed on requiring superuser access for this function. I concur that letting just anyone see the config data is inappropriate. I'm in favor, and don't really want to rehearse the argument. But I think I can live quite happily with it being superuser only. It's pretty hard to argue that exposing it to a superuser creates much risk, ISTM. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Release 4.17 of the PostgreSQL Buildfarm client
I have just cut release 4.17 of the PostgreSQL Buildfarm client. It is available at <http://www.pgbuildfarm.org/downloads/latest-client.tgz>. Changes of note: * use PGCTLTIMEOUT instead of hardcoded timeout settings * shipped config file is renamed to build-farm.conf.sample to avoid overwriting a default config file * use configured make command in make_bin_installcheck * restrict restart sleeps to Windows platforms * fix trigger_exclude pattern in build-farm.conf * protect git checkout with a lock if using git_use_workdirs For the most part, the thing of significance it the PGCTLTIMEOUT change, which follows up on some changes in core code. The new code defaults this setting to 120, but it can be set in the config file. If you don't need any of the other changes (most people probably won't) then just adding something like PGCTLTIMEOUT => '120', to the build_env stanza of your config file should do the trick, and you can happily wait for the next release. cheers andrew -- 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] EXLCUDE constraints and Hash indexes
>>>>> "Jeff" == Jeff Janes writes: Jeff> From: https://www.postgresql.org/docs/9.4/static/sql-createtable.html Jeff> "The access method must support amgettuple (see Chapter 55); at Jeff> present this means GIN cannot be used. Although it's allowed, there is Jeff> little point in using B-tree or hash indexes with an exclusion Jeff> constraint, because this does nothing that an ordinary unique Jeff> constraint doesn't do better. So in practice the access method will Jeff> always be GiST or SP-GiST." I also recently found a case where using btree exclusion constraints was useful: a unique index on an expression can't be marked deferrable, but the equivalent exclusion constraint can be. -- Andrew (irc:RhodiumToad) -- 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] [GENERAL] C++ port of Postgres
>>>>> "Robert" == Robert Haas writes: Robert> Hmm, so sizeof() has different semantics in C vs. C++? No. '1' has different semantics in C vs C++. (In C, '1' is an int, whereas in C++ it's a char. It so happens that (sizeof '1') is the only case which is valid in both C and C++ where this makes a difference.) -- Andrew (irc:RhodiumToad) -- 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] Re: GiST optimizing memmoves in gistplacetopage for fixed-size updates [PoC]
Thank you for your corrections. Here is the patch with suggestions taken into account, except 6th. >6) I'd rather use alignednewsize here. > +ItemIdSetNormal(tupid, offset + size_diff, newsize); This behavior is accroding to ubiquitous PageAddItem. Best regards, Andrey Borodin, Octonica & Ural Federal University. 2016-08-16 20:23 GMT+05:00 Anastasia Lubennikova : > 09.08.2016 19:45, Andrew Borodin: >> >> Here is new version of the patch, now it includes recommendations from >> Anastasia Lubennikova. >> >> >> I've investigated anamalous index size decrease. Most probable version >> appeared to be true. >> Cube extension, as some others, use Guttman's polynomial time split >> algorithm. It is known to generate "needle-like" MBBs (MBRs) for >> sorted data due to imbalanced splits (splitting 100 tuples as 98 to >> 2). Due to previous throw-to-the-end behavior of GiST this imbalance >> was further amplificated (most of inserts were going to bigger part >> after split). So GiST inserts were extremely slow for sorted data. >> There is no need to do exact sorting to trigger this behavior. >> This behavior can be fused by implementation of small-m restriction in >> split (AFAIR this is described in original R-tree paper from 84), >> which prescribes to do a split in a parts no smaller than m, where m >> is around 20% of a page capacity (in tuples number). After application >> of this patch performance for sorted data is roughly the same as >> performance for randomized data. > > > Thank you for explanation. Now it's clear to me. I did some more testing and > found nothing special. The declared feature is implemented correctly. > It passes all regression tests and improves performance. > > I still have a few minor nitpicks about the patch style. > You can find a style guide on > https://www.postgresql.org/docs/9.6/static/source.html > > 1) remove extra whitespace in README > > 2) add whitespace: if(ntup == 1) > > 3) fix comments in accordance with coding conventions > > /* In case of single tuple update GiST calls overwrite > * In all other cases function gistplacetopage deletes > * old tuples and place updated at the end > * */ > > > +/* Normally here was following assertion > + * Assert(ItemIdHasStorage(ii)); > + * This assertion was introduced in PageIndexTupleDelete > + * But if this function will be used from BRIN index > + * this assertion will fail. Thus, here we do not check that > + * item has the storage. > + * */ > > 4) remove unrelated changes > -data += sizeof(OffsetNumber) * xldata->ntodelete; > +data += sizeof(OffsetNumber) *xldata->ntodelete; > > 5) If the comment is correct, maxalignment is not necessary. > +/* tuples on a page are always maxaligned */ > +oldsize = MAXALIGN(oldsize); > > 6) I'd rather use alignednewsize here. > +ItemIdSetNormal(tupid, offset + size_diff, newsize); > > > After the cleanup you can change status to "Ready for Committer" > without waiting for the response. > >> If data is randomized this effect of Guttman poly-time split is not in >> effect; test from the start of the thread will show no statistically >> confident improvement by the patch. >> To prove performance increase in randomized case I've composed >> modified test https://gist.github.com/x4m/856b2e1a5db745f8265c76b9c195f2e1 >> This test with five runs shows following: >> Before patch >> Insert Time AVG 78 seconds STD 9.5 >> Afer patch >> Insert Time AVG 68 seconds STD 7.7 >> This is marginal but statistically viable performance improvement. >> >> For sorted data performance is improved by a factor of 3. >> Best regards, Andrey Borodin, Octonica & Ural Federal University. >> > > -- > Anastasia Lubennikova > Postgres Professional: http://www.postgrespro.com > The Russian Postgres Company > PageIndexTupleOverwrite v8.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] SP-GiST support for inet datatypes
>>>>> "Tom" == Tom Lane writes: > Emre Hasegeli writes: >> Attached patches add SP-GiST support to the inet datatypes. Tom> I started to look at this patch. The reported speedup is pretty Tom> nice, but ... The builtin gist support for inet seems quite surprisingly slow; ip4r beats it into the ground without difficulty. (I'd be curious how well this sp-gist version stacks up against ip4r.) -- Andrew (irc:RhodiumToad) -- 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] Strange result with LATERAL query
>>>>> "Jeevan" == Jeevan Chalke writes:
Jeevan> Hi,
Jeevan> While playing with LATERAL along with some aggregates in
Jeevan> sub-query, I have observed somewhat unusual behavior.
Simpler example not needing LATERAL:
select array(select sum(x+y) from generate_series(1,3) y group by y)
from generate_series(1,3) x;
?column?
--
{2,4,3}
{2,4,3}
{2,4,3}
(3 rows)
This is broken all the way back, it's not a recent bug.
Something is wrong with the way chgParam is being handled in Agg nodes.
The code in ExecReScanAgg seems to assume that if the lefttree doesn't
have any parameter changes then it suffices to re-project the data from
the existing hashtable; but of course this is nonsense if the parameter
is in an input to an aggregate function.
--
Andrew (irc:RhodiumToad)
--
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] Strange result with LATERAL query
>>>>> "Andrew" == Andrew Gierth writes: >>>>> "Jeevan" == Jeevan Chalke writes: Jeevan> Hi, Jeevan> While playing with LATERAL along with some aggregates in Jeevan> sub-query, I have observed somewhat unusual behavior. Andrew> Simpler example not needing LATERAL: Andrew> select array(select sum(x+y) from generate_series(1,3) y group by y) Andrew> from generate_series(1,3) x; Oh, and another way to verify that it's a bug and not expected behavior is that it goes away with set enable_hashagg=false; -- Andrew (irc:RhodiumToad) -- 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] Strange result with LATERAL query
>>>>> "Tom" == Tom Lane writes: Tom> I'm not sure if it's worth trying to distinguish whether the Param Tom> is inside any aggregate calls or not. The existing code gets the Tom> right answer for Tom> select array(select x+sum(y) from generate_series(1,3) y group by y) Tom> from generate_series(1,3) x; Tom> and we'd be losing some efficiency for cases like that if we fix Tom> it as above. But is it worth the trouble? The loss of efficiency could be significant, since it's forcing a rescan of what could be an expensive plan. -- Andrew (irc:RhodiumToad) -- 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] Strange result with LATERAL query
>>>>> "Andrew" == Andrew Gierth writes:
>>>>> "Tom" == Tom Lane writes:
Tom> I'm not sure if it's worth trying to distinguish whether the Param
Tom> is inside any aggregate calls or not.
How about:
--
Andrew (irc:RhodiumToad)
diff --git a/src/backend/executor/nodeAgg.c b/src/backend/executor/nodeAgg.c
index 1ec2515..4a9fefb 100644
--- a/src/backend/executor/nodeAgg.c
+++ b/src/backend/executor/nodeAgg.c
@@ -3426,10 +3426,11 @@ ExecReScanAgg(AggState *node)
/*
* If we do have the hash table and the subplan does not have any
- * parameter changes, then we can just rescan the existing hash table;
- * no need to build it again.
+ * parameter changes, we might still need to rebuild the hash if any of
+ * the parameter changes affect the input to aggregate functions.
*/
- if (outerPlan->chgParam == NULL)
+ if (outerPlan->chgParam == NULL
+ && !bms_overlap(node->ss.ps.chgParam, aggnode->aggParam))
{
ResetTupleHashIterator(node->hashtable, &node->hashiter);
return;
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index c7a0644..7b5eb86 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -871,6 +871,7 @@ _copyAgg(const Agg *from)
COPY_SCALAR_FIELD(aggstrategy);
COPY_SCALAR_FIELD(aggsplit);
COPY_SCALAR_FIELD(numCols);
+ COPY_BITMAPSET_FIELD(aggParam);
if (from->numCols > 0)
{
COPY_POINTER_FIELD(grpColIdx, from->numCols * sizeof(AttrNumber));
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index 1fab807..5e48edd 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -706,6 +706,7 @@ _outAgg(StringInfo str, const Agg *node)
WRITE_ENUM_FIELD(aggstrategy, AggStrategy);
WRITE_ENUM_FIELD(aggsplit, AggSplit);
WRITE_INT_FIELD(numCols);
+ WRITE_BITMAPSET_FIELD(aggParam);
appendStringInfoString(str, " :grpColIdx");
for (i = 0; i < node->numCols; i++)
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index c83063e..67dcf17 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -2004,6 +2004,7 @@ _readAgg(void)
READ_ENUM_FIELD(aggstrategy, AggStrategy);
READ_ENUM_FIELD(aggsplit, AggSplit);
READ_INT_FIELD(numCols);
+ READ_BITMAPSET_FIELD(aggParam);
READ_ATTRNUMBER_ARRAY(grpColIdx, local_node->numCols);
READ_OID_ARRAY(grpOperators, local_node->numCols);
READ_LONG_FIELD(numGroups);
diff --git a/src/backend/optimizer/plan/subselect.c b/src/backend/optimizer/plan/subselect.c
index a46cc10..2e56484 100644
--- a/src/backend/optimizer/plan/subselect.c
+++ b/src/backend/optimizer/plan/subselect.c
@@ -82,6 +82,7 @@ static Bitmapset *finalize_plan(PlannerInfo *root,
Bitmapset *valid_params,
Bitmapset *scan_params);
static bool finalize_primnode(Node *node, finalize_primnode_context *context);
+static bool finalize_agg_primnode(Node *node, finalize_primnode_context *context);
/*
@@ -2659,8 +2660,30 @@ finalize_plan(PlannerInfo *root, Plan *plan, Bitmapset *valid_params,
&context);
break;
- case T_Hash:
case T_Agg:
+ {
+Agg *agg = (Agg *) plan;
+finalize_primnode_context aggcontext;
+
+/*
+ * We need to know which params are referenced in inputs to
+ * aggregate calls, so that we know whether we need to rebuild
+ * the hashtable in the AGG_HASHED case. Rescan the tlist and
+ * qual for this purpose.
+ */
+
+aggcontext = context;
+aggcontext.paramids = NULL;
+
+finalize_agg_primnode((Node *) agg->plan.targetlist, &aggcontext);
+finalize_agg_primnode((Node *) agg->plan.qual, &aggcontext);
+
+/* remember results for execution */
+agg->aggParam = aggcontext.paramids;
+ }
+ break;
+
+ case T_Hash:
case T_Material:
case T_Sort:
case T_Unique:
@@ -2812,6 +2835,24 @@ finalize_primnode(Node *node, finalize_primnode_context *context)
}
/*
+ * finalize_agg_primnode: add IDs of all PARAM_EXEC params appearing inside
+ * Aggref nodes in the given expression tree to the result set.
+ */
+static bool
+finalize_agg_primnode(Node *node, finalize_primnode_context *context)
+{
+ if (node == NULL)
+ return false;
+ if (IsA(node, Aggref))
+ {
+ finalize_primnode(node, context);
+ return false; /* no more to do here */
+ }
+ return expression_tree_walker(node, finalize_agg_primnode,
+ (void *) context);
+}
+
+/*
* SS_make_initplan_output_param - make a Param for an initPlan's output
*
* The plan is expected to return a scalar value of the given type/collation.
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 369179f..3d5e4df 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -712,6 +712,7 @@ typedef struct Agg
AggStrategy aggstrategy; /* basic strategy, see nodes.h */
AggSp
Re: [HACKERS] Strange result with LATERAL query
>>>>> "Pavel" == Pavel Stehule writes: Pavel> The result should not depend on GUC - hashagg on/off changing Pavel> output - it is error. I don't think anyone's suggesting leaving it unfixed, just whether the fix should introduce unnecessary rescans of the aggregate input. -- Andrew (irc:RhodiumToad) -- 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] Strange result with LATERAL query
>>>>> "Tom" == Tom Lane writes: Tom> Hm, I was just working on inserting something of the sort into Tom> ExecInitAgg. But I guess we could do it in the planner too. Will Tom> run with your approach. Tom> I think it's a bit too stupid as-is, though. We don't need to Tom> recalculate for Params in aggdirectargs, do we? In theory we would need to. But in practice we don't, because we don't allow ordered aggs in AGG_HASHED mode anyway. We could skip filling in aggParam at all if not in AGG_HASHED mode I guess. -- Andrew (irc:RhodiumToad) -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] GiST penalty functions [PoC]
Hi, hackers!
Some time ago I put a patch to commitfest that optimizes slightly GiST
inserts and updates. It's effect in general is quite modest (15% perf
improved), but for sorted data inserts are 3 times quicker. This
effect I attribute to mitigation of deficiencies of old split
algorithm.
Alexander Korotkov advised me to lookup some features of the RR*-tree.
The RR*-tree differs from combination of Gist + cube extension in two
algorithms:
1.Algorithm to choose subtree for insertion
2.Split algorithm
This is the message regarding implementation of first one in GiST. I
think that both of this algorithms will improve querying performance.
Long story short, there are two problems in choose subtree algorithm.
1.GiST chooses subtree via penalty calculation for each entry,
while RR*-tree employs complicated conditional logic: when there are
MBBs (Minimum Bounding Box) which do not require extensions, smallest
of them is chosen; in some cases, entries are chosen not by volume,
but by theirs margin.
2.RR*-tree algorithm jumps through entry comparation non-linearly,
I think this means that GiST penalty computation function will have to
access other entries on a page.
In this message I address only first problem. I want to construct a
penalty function that will choose:
1.Entry with a zero volume and smallest margin, that can
accommodate insertion tuple without extension, if one exists;
2.Entry with smallest volume, that can accommodate insertion tuple
without extension, if one exists;
3.Entry with zero volume increase after insert and smallest margin
increase, if one exists;
4.Entry with minimal volume increase after insert.
Current cube behavior inspects only 4th option by returning as a
penalty (float) MBB’s volume increase. To implement all 4 options, I
use a hack: order of positive floats corresponds exactly to order of
integers with same binary representation (I’m not sure this stands for
every single supported platform). So I use two bits of float as if it
were integer, and all others are used as shifted bits of corresponding
float: option 4 – volume increase, 3 - margin increase, 2 – MBB
volume, 1 – MBB margin. You can check the reinterpretation cast
function pack_float() in the patch.
I’ve tested patch performance with attached test. On my machine patch
slows index construction time from 60 to 76 seconds, reduces size of
the index from 85Mb to 82Mb, reduces time of aggregates computation
from 36 seconds to 29 seconds.
I do not know: should I continue this work in cube, or it’d be better
to fork cube?
Maybe anyone have tried already RR*-tree implementation? Science
papers show very good search performance increase.
Please let me know if you have any ideas, information or interest in this topic.
Best regards, Andrey Borodin, Octonica & Ural Federal University.
diff --git a/contrib/cube/cube.c b/contrib/cube/cube.c
index 3feddef..9c8c63d 100644
--- a/contrib/cube/cube.c
+++ b/contrib/cube/cube.c
@@ -96,6 +96,7 @@ bool cube_contains_v0(NDBOX *a, NDBOX *b);
bool cube_overlap_v0(NDBOX *a, NDBOX *b);
NDBOX *cube_union_v0(NDBOX *a, NDBOX *b);
void rt_cube_size(NDBOX *a, double *sz);
+void rt_cube_edge(NDBOX *a, double *sz);
NDBOX *g_cube_binary_union(NDBOX *r1, NDBOX *r2, int *sizep);
bool g_cube_leaf_consistent(NDBOX *key, NDBOX *query, StrategyNumber
strategy);
bool g_cube_internal_consistent(NDBOX *key, NDBOX *query,
StrategyNumber strategy);
@@ -420,6 +421,13 @@ g_cube_decompress(PG_FUNCTION_ARGS)
PG_RETURN_POINTER(entry);
}
+float pack_float(float actualValue, int realm)
+{
+ // two bits for realm, other for value
+ int realmAjustment = *((int*)&actualValue)/4;
+ int realCode = realm * (INT32_MAX/4) + realmAjustment; // we have 4
realms
+ return *((float*)&realCode);
+}
/*
** The GiST Penalty method for boxes
@@ -428,6 +436,11 @@ g_cube_decompress(PG_FUNCTION_ARGS)
Datum
g_cube_penalty(PG_FUNCTION_ARGS)
{
+ // REALM 0: No extension is required, volume is zerom return edge
+ // REALM 1: No extension is required, return nonzero volume
+ // REALM 2: Volume extension is zero, return nonzero edge extension
+ // REALM 3: Volume extension is nonzero, return it
+
GISTENTRY *origentry = (GISTENTRY *) PG_GETARG_POINTER(0);
GISTENTRY *newentry = (GISTENTRY *) PG_GETARG_POINTER(1);
float *result = (float *) PG_GETARG_POINTER(2);
@@ -440,6 +453,32 @@ g_cube_penalty(PG_FUNCTION_ARGS)
rt_cube_size(ud, &tmp1);
rt_cube_size(DatumGetNDBOX(origentry->key), &tmp2);
*result = (float) (tmp1 - tmp2);
+ if(*result == 0)
+ {
+ double tmp3 = tmp1;
+ rt_cube_edge(ud, &tmp1);
+ rt_cube_edge(DatumGetNDBOX(origentry->key), &tmp2);
+ *result = (float) (tmp1 - tmp2);
+ if(*result == 0)
+ {
+
Re: [HACKERS] GiST penalty functions [PoC]
Hi hackers!
Here is the new patch version.
With the help of Mikhail Bakhterev I've optimized subroutines of the
penalty function. Index build time now is roughly equivalent to build
time before patch (test attached to thread start).
Time of SELECT statement execution is reduced by 40%.
Changes in the patch:
1. Wrong usage of realms is fixed
2. Cube size and margin (edge) functions are optimized to reduce
memory write instructions count (result of these functions were
written on evey cycle of a loop)
3. All private functions are marked as static inline
4. Comments are formatted per project style
I'm going to put this to commitfest queue, because performance of gist
queries is improved significantly and I do not see any serious
drawbacks.
Any ideas about this patch are welcome. Especialy I'm conserned about
portability of pack_float function.
Does every supported Postgres platform conforms to IEEE 754 floating
point specification?
Also I'm not sure about possibility to hit any problems with float
NaNs during float package?
Best regards, Andrey Borodin, Octonica & Ural Federal University.
diff --git a/contrib/cube/cube.c b/contrib/cube/cube.c
index 3feddef..dec314f 100644
--- a/contrib/cube/cube.c
+++ b/contrib/cube/cube.c
@@ -91,20 +91,22 @@ PG_FUNCTION_INFO_V1(cube_enlarge);
/*
** For internal use only
*/
-int32 cube_cmp_v0(NDBOX *a, NDBOX *b);
-bool cube_contains_v0(NDBOX *a, NDBOX *b);
-bool cube_overlap_v0(NDBOX *a, NDBOX *b);
-NDBOX *cube_union_v0(NDBOX *a, NDBOX *b);
-void rt_cube_size(NDBOX *a, double *sz);
-NDBOX *g_cube_binary_union(NDBOX *r1, NDBOX *r2, int *sizep);
-bool g_cube_leaf_consistent(NDBOX *key, NDBOX *query, StrategyNumber
strategy);
-bool g_cube_internal_consistent(NDBOX *key, NDBOX *query,
StrategyNumber strategy);
+static inline int32cube_cmp_v0(NDBOX *a, NDBOX *b);
+static inline bool cube_contains_v0(NDBOX *a, NDBOX *b);
+static inline bool cube_overlap_v0(NDBOX *a, NDBOX *b);
+static inline NDBOX *cube_union_v0(NDBOX *a, NDBOX *b);
+static inline floatpack_float(float actualValue, int realm);
+static inline void rt_cube_size(NDBOX *a, double *sz);
+static inline void rt_cube_edge(NDBOX *a, double *sz);
+static inline NDBOX *g_cube_binary_union(NDBOX *r1, NDBOX *r2, int
*sizep);
+static inline bool g_cube_leaf_consistent(NDBOX *key, NDBOX
*query, StrategyNumber strategy);
+static inline bool g_cube_internal_consistent(NDBOX *key, NDBOX
*query, StrategyNumber strategy);
/*
** Auxiliary funxtions
*/
-static double distance_1D(double a1, double a2, double b1, double b2);
-static bool cube_is_point_internal(NDBOX *cube);
+static inline double distance_1D(double a1, double a2, double b1, double b2);
+static inline bool cube_is_point_internal(NDBOX *cube);
/*
@@ -420,6 +422,15 @@ g_cube_decompress(PG_FUNCTION_ARGS)
PG_RETURN_POINTER(entry);
}
+static inline float
+pack_float(float actualValue, int realm)
+{
+ /* two bits for realm, other for value */
+ /* we have 4 realms */
+ int realmAjustment = *((int*)&actualValue)/4;
+ int realCode = realm * (INT32_MAX/4) + realmAjustment;
+ return *((float*)&realCode);
+}
/*
** The GiST Penalty method for boxes
@@ -428,6 +439,11 @@ g_cube_decompress(PG_FUNCTION_ARGS)
Datum
g_cube_penalty(PG_FUNCTION_ARGS)
{
+ /* REALM 0: No extension is required, volume is zero, return edge
*/
+ /* REALM 1: No extension is required, return nonzero volume
*/
+ /* REALM 2: Volume extension is zero, return nonzero edge extension
*/
+ /* REALM 3: Volume extension is nonzero, return it
*/
+
GISTENTRY *origentry = (GISTENTRY *) PG_GETARG_POINTER(0);
GISTENTRY *newentry = (GISTENTRY *) PG_GETARG_POINTER(1);
float *result = (float *) PG_GETARG_POINTER(2);
@@ -441,9 +457,33 @@ g_cube_penalty(PG_FUNCTION_ARGS)
rt_cube_size(DatumGetNDBOX(origentry->key), &tmp2);
*result = (float) (tmp1 - tmp2);
- /*
-* fprintf(stderr, "penalty\n"); fprintf(stderr, "\t%g\n", *result);
-*/
+ if( *result == 0 )
+ {
+ double tmp3 = tmp1; /* remember entry volume */
+ rt_cube_edge(ud, &tmp1);
+ rt_cube_edge(DatumGetNDBOX(origentry->key), &tmp2);
+ *result = (float) (tmp1 - tmp2);
+ if( *result == 0 )
+ {
+ if( tmp3 != 0 )
+ {
+ *result = pack_float(tmp3, 1); /* REALM 1 */
+ }
+ else
+ {
+ *result = pack_float(tmp1, 0); /*
Re: [HACKERS] GiST penalty functions [PoC]
Thank you for your coments, Tom. > Modern compilers are generally able to make their own decisions about it, and > trying to put your thumb on the scales this heavily is not likely to improve > the code. Well, I have tested that combination of "static inline" affets performance of index build on a scale of 5%. Though I didn't tested with "static" only. AFAIK compiler cannot prove that array of function input and output do not intersect, so it emits lots of writes to output address inside loop body. >That pack_float function is absolutely not acceptable Well, possibly, there are ways to achive penalty optimization without such hacks, but it failed for pg_shpere and in PostGIS code. They reverted plain arithmetic optimization without bit hacks, becuase it didn't worked. This one works. There is other way: advance GiST API. But I'm not sure it is possible to do so without breaking compatibility with many existing extensions. Best regards, Andrey Borodin, Octonica & Ural Federal University. -- 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] GiST penalty functions [PoC]
Here is new version of the patch.
Now function pack_float is commented better.
All inline keywords are removed. I haven't found any performance loss for that.
Remove of static's showed 1%-7% performance loss in SELECT computation
(3 test runs), so I left statics where they are.
Actually, to avoid this kind of hacks, probably, it would be better to
fork GiST to GiSTv2 and implement many API changes there:
1. Bulk load API
2. Richier choose subtree API
3. Allow for key test not just NO\MAYBE answers, but SURE answer to
skip key test for underlying tree
And some other improvements require breaking chanes for extensions.
GiST idea is almost 25, modern spatial indices vary a lot from things
that were there during 90th.
Best regards, Andrey Borodin, Octonica & Ural Federal University.
diff --git a/contrib/cube/cube.c b/contrib/cube/cube.c
index 3feddef..9ce3cd6 100644
--- a/contrib/cube/cube.c
+++ b/contrib/cube/cube.c
@@ -91,14 +91,16 @@ PG_FUNCTION_INFO_V1(cube_enlarge);
/*
** For internal use only
*/
-int32 cube_cmp_v0(NDBOX *a, NDBOX *b);
-bool cube_contains_v0(NDBOX *a, NDBOX *b);
-bool cube_overlap_v0(NDBOX *a, NDBOX *b);
-NDBOX *cube_union_v0(NDBOX *a, NDBOX *b);
-void rt_cube_size(NDBOX *a, double *sz);
-NDBOX *g_cube_binary_union(NDBOX *r1, NDBOX *r2, int *sizep);
-bool g_cube_leaf_consistent(NDBOX *key, NDBOX *query, StrategyNumber
strategy);
-bool g_cube_internal_consistent(NDBOX *key, NDBOX *query,
StrategyNumber strategy);
+static int32 cube_cmp_v0(NDBOX *a, NDBOX *b);
+static boolcube_contains_v0(NDBOX *a, NDBOX *b);
+static boolcube_overlap_v0(NDBOX *a, NDBOX *b);
+static NDBOX *cube_union_v0(NDBOX *a, NDBOX *b);
+static float pack_float(float actualValue, int realm);
+static voidrt_cube_size(NDBOX *a, double *sz);
+static voidrt_cube_edge(NDBOX *a, double *sz);
+static NDBOX *g_cube_binary_union(NDBOX *r1, NDBOX *r2, int *sizep);
+static boolg_cube_leaf_consistent(NDBOX *key, NDBOX *query,
StrategyNumber strategy);
+static boolg_cube_internal_consistent(NDBOX *key, NDBOX *query,
StrategyNumber strategy);
/*
** Auxiliary funxtions
@@ -419,7 +421,27 @@ g_cube_decompress(PG_FUNCTION_ARGS)
}
PG_RETURN_POINTER(entry);
}
-
+/*
+ * Function to pack bit flags inside float type
+ * Resulted value represent can be from four different "realms"
+ * Every value from realm 3 is greater than any value from realms 2,1 and 0.
+ * Every value from realm 2 is less than every value from realm 3 and greater
+ * than any value from realm 1 and 0, and so on. Values from the same realm
+ * loose two bits of precision. This technique is possible due to floating
+ * point numbers specification according to IEEE 754: exponent bits are highest
+ * (excluding sign bits, but here penalty is always positive). If float a is
+ * greater than float b, integer A with same bit representation as a is greater
+ * than integer B with same bits as b.
+ */
+static float
+pack_float(float actualValue, int realm)
+{
+ /* two bits for realm, others for value */
+ /* we have 4 realms */
+ int realmAjustment = *((int*)&actualValue)/4;
+ int realCode = realm * (INT32_MAX/4) + realmAjustment;
+ return *((float*)&realCode);
+}
/*
** The GiST Penalty method for boxes
@@ -428,6 +450,11 @@ g_cube_decompress(PG_FUNCTION_ARGS)
Datum
g_cube_penalty(PG_FUNCTION_ARGS)
{
+ /* REALM 0: No extension is required, volume is zero, return edge
*/
+ /* REALM 1: No extension is required, return nonzero volume
*/
+ /* REALM 2: Volume extension is zero, return nonzero edge extension
*/
+ /* REALM 3: Volume extension is nonzero, return it
*/
+
GISTENTRY *origentry = (GISTENTRY *) PG_GETARG_POINTER(0);
GISTENTRY *newentry = (GISTENTRY *) PG_GETARG_POINTER(1);
float *result = (float *) PG_GETARG_POINTER(2);
@@ -441,9 +468,33 @@ g_cube_penalty(PG_FUNCTION_ARGS)
rt_cube_size(DatumGetNDBOX(origentry->key), &tmp2);
*result = (float) (tmp1 - tmp2);
- /*
-* fprintf(stderr, "penalty\n"); fprintf(stderr, "\t%g\n", *result);
-*/
+ if( *result == 0 )
+ {
+ double tmp3 = tmp1; /* remember entry volume */
+ rt_cube_edge(ud, &tmp1);
+ rt_cube_edge(DatumGetNDBOX(origentry->key), &tmp2);
+ *result = (float) (tmp1 - tmp2);
+ if( *result == 0 )
+ {
+ if( tmp3 != 0 )
+ {
+ *result = pack_float(tmp3, 1); /* REALM 1 */
+ }
+ else
+ {
+ *result = pack_float(tmp1, 0); /* REALM 0 */
+
