Re: [HACKERS] Extensions vs PGXS' MODULE_PATHNAME handling
Tom Lane t...@sss.pgh.pa.us writes: My take here is to way that in this case, the current (9.1) way to deal with the situation is to have multiple extensions when you have multiple shlibs. After all we know that multiple extensions from the same Makefile works, thanks to contrib/spi (I mean extension/spi). But contrib/spi is exactly the case where it *won't* work. We need to somehow figure out that $libdir/autoinc is what to substitute in autoinc-1.0.sql, $libdir/insert_username in insert_username-1.0.sql, etc. Indeed. That's why I'm proposing to have that setup in the control file, which is per extension, rather than in the common Makefile. Also, I've been looking at the pg_available_extensions issue a bit. I don't yet have a proposal for exactly how we ought to redefine it, but I did notice that the existing code is terribly confused by secondary control files: it doesn't realize that they're not primary control files, so you get e.g. hstore and hstore-1.0 as separate listings. I'd think that's it's a good idea if dealt with correctly because now that ALTER EXTENSION UPDATE can deal with more than one target VERSION I expect the view to show each available update here. If possible adding the update chain sequence information as computed in the code would be great. Because we can't ask people to figure that out all by themselves, the best way to check your upgrading setup is fine would be to run SELECT * FROM pg_available_extensions; and read the result. We could possibly work around that by giving secondary control files a different extension, but I'm becoming more and more convinced that it's just a bad idea to have a file naming rule in which it's ambiguous where the extension name stops and the version name starts. Agreed. I did think of another idea besides forbidding dash in extension names: what if we use double dash as the name/version separator, ie the naming conventions are like extension--version.control extension--version.sql extension--oldversion-newversion.sql Yeah, something like that would work, so would maybe using ':' and forbidding one-letter extension names, but I'm not in a position to check that this won't confuse the windows we support too much. I see about no downside to the double dash proposal, that said. Then we'd only have to forbid double dash in extension names, which seems unlikely to be a problem for anybody. (I think we might also have to forbid empty version names to make this bulletproof, but that doesn't bother me much either.) Those look like sanity checks more than anything else, I'd welcome us having them. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Debian readline/libedit breakage
Greg Smith g...@2ndquadrant.com writes: What the RPM packaging does is run this (approximately): Well building the debian package also run make check. My question is if that's enough QA here for us? The other side of things if that we will need to provide for a debian repository with support for at least lenny and squeeze and sid, and i386 and amd64. Maybe some more. We will need some build environments here. Anybody thinking we should somehow include ubuntu in the mix? If yes, which versions? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Debian readline/libedit breakage
On Sun, Feb 13, 2011 at 12:09, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Greg Smith g...@2ndquadrant.com writes: What the RPM packaging does is run this (approximately): Well building the debian package also run make check. My question is if that's enough QA here for us? Don't the RPM building guys (Hi, Devrim!) also run the tests on an installed version of the RPMs? Should be easy enough to automate something like that, no? Though there obviously has to be some point where to stop - should we test both install and upgrade? The other side of things if that we will need to provide for a debian repository with support for at least lenny and squeeze and sid, and i386 and amd64. Maybe some more. We will need some build environments here. I think i386 and amd64 are enough, really. We could add more later if necessary, but i don't think we need to. I assume this can be easily virtualized - e.g. having one VM for each version and just boot it up, update all dependencis, build, and shut down? in fact, shouldn't there be tools around already that do this automated? Anybody thinking we should somehow include ubuntu in the mix? If yes, which versions? Yes, since according to a comment somewhere the same issue will bubble into ubuntu soon. At this point, definitely 8.04 and 10.04, and probably 10.10. If things can be easily automated, it would be great if we could do *all* supported ubuntu, but doing LTS and the latest one or two non-LTS releases. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- 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 TYPE 2: skip already-provable no-work rewrites
On Sun, Feb 13, 2011 at 12:04:20AM -0500, Robert Haas wrote: On Sat, Feb 12, 2011 at 10:45 AM, Noah Misch n...@leadboat.com wrote: That said, I've tried both constructions, and I marginally prefer the end result with AlteredTableInfo.verify. ?I've inlined ATColumnChangeRequiresRewrite into ATPrepAlterColumnType; it would need to either pass back two bools or take an AlteredTableInfo arg to mutate, so this seemed cleaner. I think I like the idea of passing it the AlteredTableInfo. Okay. I've omitted the assertion that my previous version added to ATRewriteTable; it was helpful for other scan-only type changes, but it's excessive for domains alone. ?Otherwise, the differences are cosmetic. So in the case of a constrained domain, it looks like we're going to evaluate the changed columns, but if no error is thrown, we're going to assume they match the original ones and throw out the data? Correct. We can see that a RelabelType changes no values by inspecting ExecEvalRelabelType. Likewise, by inspecting ExecEvalCoerceToDomain, we can know that a CoerceToDomain node may introduce errors but never modified values. Yikes. I didn't like that Assert much, but maybe we need it, because this is scary. Can you elaborate on the fear-inducing aspect? I don't mind re-adding the Assert, but it seems that some positive understanding of the assumption's validity is in order. nm -- 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] Debian readline/libedit breakage
Magnus Hagander mag...@hagander.net writes: I think i386 and amd64 are enough, really. We could add more later if necessary, but i don't think we need to. Ok. I assume this can be easily virtualized - e.g. having one VM for each version and just boot it up, update all dependencis, build, and shut down? in fact, shouldn't there be tools around already that do this automated? Yes, see pbuilder. You prepare a debootstrap environment (that's a tar.gz with a bare minimum debian installation in that you can chroot to to build package) and pbuilder will apt-get build-dep then debuild for you then copy the resulting debs out of the chroot and remove it. You can even build i386 packages from an amd64 OS, I'm doing that nowadays for the emacs-snapshot package for ubuntu. I'm building from the debian package someone else is doing. Anybody thinking we should somehow include ubuntu in the mix? If yes, which versions? Yes, since according to a comment somewhere the same issue will bubble into ubuntu soon. At this point, definitely 8.04 and 10.04, and probably 10.10. If things can be easily automated, it would be great if we could do *all* supported ubuntu, but doing LTS and the latest one or two non-LTS releases. Well, that needs I guess either several ubuntu VM or pbuilder support for ubuntu debootstraps, will check about that. -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] make check for PLs
Here is a patch that implements make check support for the PLs and a make check-world target. I found the lack of this quite annoying while reviewing the load of PL/Python patches. There did not appear to be a reason why it wasn't done before. Support for make check in contrib would also be nice, but it's currently not possible without a way to teach pg_regress to install the respective module's shared library. diff --git a/GNUmakefile.in b/GNUmakefile.in index 8ccbdcc..b9c5f31 100644 --- a/GNUmakefile.in +++ b/GNUmakefile.in @@ -60,6 +60,9 @@ check: all check installcheck installcheck-parallel: $(MAKE) -C src/test $@ +# TODO: add contrib +$(call recurse,check-world,src/test src/pl src/interfaces/ecpg,check) + $(call recurse,installcheck-world,src/test src/pl src/interfaces/ecpg contrib,installcheck) GNUmakefile: GNUmakefile.in $(top_builddir)/config.status @@ -121,4 +124,4 @@ distcheck: dist rm -rf $(distdir) $(dummy) @echo Distribution integrity checks out. -.PHONY: dist distdir distcheck docs install-docs world install-world installcheck-world +.PHONY: dist distdir distcheck docs install-docs world check-world install-world installcheck-world diff --git a/Makefile b/Makefile index b5b2ea5..72e9c83 100644 --- a/Makefile +++ b/Makefile @@ -11,7 +11,7 @@ # GNUmakefile won't exist yet, so we catch that case as well. -all check install installdirs installcheck installcheck-parallel uninstall clean distclean maintainer-clean dist distcheck world install-world installcheck-world: +all check install installdirs installcheck installcheck-parallel uninstall clean distclean maintainer-clean dist distcheck world check-world install-world installcheck-world: @if [ ! -f GNUmakefile ] ; then \ echo You need to run the 'configure' program first. See the file; \ echo 'INSTALL' for installation instructions. ; \ diff --git a/src/Makefile.global.in b/src/Makefile.global.in index d6b7b47..9fad24e 100644 --- a/src/Makefile.global.in +++ b/src/Makefile.global.in @@ -285,9 +285,6 @@ XGETTEXT = @XGETTEXT@ GZIP = gzip BZIP2 = bzip2 -PL_TESTDB = pl_regression -CONTRIB_TESTDB = contrib_regression - # Installation. INSTALL = $(SHELL) $(top_srcdir)/config/install-sh -c @@ -428,6 +425,19 @@ submake-libpgport: ## # +# Testing support + +PL_TESTDB = pl_regression +CONTRIB_TESTDB = contrib_regression + +pg_regress_check = $(top_builddir)/src/test/regress/pg_regress --temp-install=./tmp_check --top-builddir=$(top_builddir) +pg_regress_installcheck = $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --psqldir=$(PSQLDIR) + +pg_regress_clean_files = results/ regression.diffs regression.out tmp_check/ log/ + + +## +# # Customization # # This includes your local customizations if Makefile.custom exists diff --git a/src/makefiles/pgxs.mk b/src/makefiles/pgxs.mk index 412bf5c..23f28e3 100644 --- a/src/makefiles/pgxs.mk +++ b/src/makefiles/pgxs.mk @@ -280,12 +280,11 @@ endif # against installed postmaster installcheck: submake - $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --psqldir=$(PSQLDIR) $(REGRESS_OPTS) $(REGRESS) + $(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS) # in-tree test doesn't work yet (no way to install my shared library) #check: all submake -# $(top_builddir)/src/test/regress/pg_regress --temp-install \ -# --top-builddir=$(top_builddir) $(REGRESS_OPTS) $(REGRESS) +# $(pg_regress_check) $(REGRESS_OPTS) $(REGRESS) check: @echo 'make check' is not supported. @echo Do 'make install', then 'make installcheck' instead. diff --git a/src/pl/plperl/.gitignore b/src/pl/plperl/.gitignore index c04f42b..1a79873 100644 --- a/src/pl/plperl/.gitignore +++ b/src/pl/plperl/.gitignore @@ -4,4 +4,6 @@ /plperl_opmask.h # Generated subdirectories +/log/ /results/ +/tmp_check/ diff --git a/src/pl/plperl/GNUmakefile b/src/pl/plperl/GNUmakefile index 85cf9a8..01e585e 100644 --- a/src/pl/plperl/GNUmakefile +++ b/src/pl/plperl/GNUmakefile @@ -76,8 +76,11 @@ installdirs: installdirs-lib uninstall: uninstall-lib +check: submake + $(pg_regress_check) $(REGRESS_OPTS) $(REGRESS) + installcheck: submake - $(top_builddir)/src/test/regress/pg_regress --inputdir=$(srcdir) --psqldir=$(PSQLDIR) $(REGRESS_OPTS) $(REGRESS) + $(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS) .PHONY: submake submake: @@ -85,8 +88,7 @@ submake: clean distclean maintainer-clean: clean-lib rm -f SPI.c Util.c $(OBJS) perlchunks.h plperl_opmask.h - rm -rf results - rm -f regression.diffs regression.out + rm -rf $(pg_regress_clean_files) else # can't build diff --git a/src/pl/plpython/.gitignore b/src/pl/plpython/.gitignore index 19b6c5b..5dcb3ff 100644 --- a/src/pl/plpython/.gitignore +++ b/src/pl/plpython/.gitignore @@ -1,2 +1,4 @@ # Generated subdirectories +/log/ /results/ +/tmp_check/ diff --git
[HACKERS] psql -l doesn't process psqlrc
psql -l doesn't process psqlrc. Historically, this was probably not
useful, hence no one cared. But with the linestyle option it's useful.
So I propose the attached tweak.
diff --git i/src/bin/psql/startup.c w/src/bin/psql/startup.c
index 4f3815a..10713e9 100644
--- i/src/bin/psql/startup.c
+++ w/src/bin/psql/startup.c
@@ -224,8 +224,12 @@ main(int argc, char *argv[])
if (options.action == ACT_LIST_DB)
{
- int success = listAllDbs(false);
+ int success;
+ if (!options.no_psqlrc)
+ process_psqlrc(argv[0]);
+
+ success = listAllDbs(false);
PQfinish(pset.db);
exit(success ? EXIT_SUCCESS : EXIT_FAILURE);
}
--
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] Scheduled maintenance affecting gitmaster
On Tue, Feb 8, 2011 at 19:52, Magnus Hagander mag...@hagander.net wrote: Hi! The PostgreSQL Infrastructure Team will be performing scheduled maintenance on the server that is hosting gitmaster.postgresql.org the coming sunday, Feb 13th. While we expect this to only cause a very short downtime for the service, one can never be entirely sure with remote maintenance.. For this reason, please avoid using the service during this time. Note that only the git *master* server is affected, not the git.postgresql.org anonymous mirror is on another host and will not be affected. We will post a note here when the maintenance has been completed. Date: 2011-02-13 Start: 12:00 UTC End: ~13:00 UTC, but may drag out Affected services: gitmaster.postgresql.org and misc. internal services Unfortunately, one of the worst-case scenarios appears to have happened - a machine did not come back up after a reboot. We are working with the hosting company to figure out what went wrong and get it back up as soon as possible, but obviously this will not happen within the window we were hoping for. We have a backup option which is to bring the VM up on a separate machine, but given the amount of extra work and possible further issues with this, we are going to hold out for a while to see if we can get the current machine back into working state, given that it's only internal services. We'll get back to you with more information as soon as we have it. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- 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] Add support for logging the current role
On Fri, Feb 11, 2011 at 11:23 AM, Stephen Frost sfr...@snowman.net wrote: Updated patch attached, full git log below. The documentation for csvlog_fields should probably use literal around the default value. The sentence that begins For details on what these fields are should hyperlink the referenced sections of the documentation. The function prototype you added to elog.c is misformatted - the type should be on the line preceding the function name only for the definition, not for prototypes. The code for log_line_prefix = 'u' is indented wrong. Also, an else clause that has only an if hanging off of it can be turned into an else if for better readability. This part kind of concerns me: + * This is more of a 'safety valve' than anything else, + * since GUC processing really should happen before we do any error logging. + * We might even want to change this eventually to just not log CSV format logs + * if this ever happens, to avoid a discrepency in the CSV log file which would + * make it difficult to load into PG. I'm not really convinced that making the CSV log format variable is a good thing. One of the reasons we added that format in the first place is to make sure that we could generate log output in an easily parseable format, and this seems like a big step backwards for not much, especially if we can't even guarantee that we're not going to inject random differently-formatted lines during startup. Stylistically, build_default_csvlog_list and assign_csvlog_fields ought to be loops driven off an array, rather than hand-coded. I think this was discussed before, and I hate to remention it, but would it make sense to reuse the single-character codes from log_line_prefix rather than inventing new codes for the same fields? It would be awfully nice if we could inject something into the csvlog output format that would let client programs know which fields are present. This would be especially useful for people writing tools that are intended to work on ANY PG installation, rather than just, say, their own. I'm not sure if there's a clean way to do that, though. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add support for logging the current role
* Robert Haas (robertmh...@gmail.com) wrote: The documentation for csvlog_fields should probably use literal around the default value. The sentence that begins For details on what these fields are should hyperlink the referenced sections of the documentation. The function prototype you added to elog.c is misformatted - the type should be on the line preceding the function name only for the definition, not for prototypes. The code for log_line_prefix = 'u' is indented wrong. Also, an else clause that has only an if hanging off of it can be turned into an else if for better readability. Will fix. This part kind of concerns me: + * This is more of a 'safety valve' than anything else, + * since GUC processing really should happen before we do any error logging. + * We might even want to change this eventually to just not log CSV format logs + * if this ever happens, to avoid a discrepency in the CSV log file which would + * make it difficult to load into PG. I'm not really convinced that making the CSV log format variable is a good thing. One of the reasons we added that format in the first place is to make sure that we could generate log output in an easily parseable format, and this seems like a big step backwards for not much, especially if we can't even guarantee that we're not going to inject random differently-formatted lines during startup. I couldn't make it actually produce incorrect lines. I was worried about the possibility, but I don't think it's actually possible because postgresql.conf needs to be parsed and GUC handling has to work before we will even start trying to do CSV logging. I'll rework the comment. Stylistically, build_default_csvlog_list and assign_csvlog_fields ought to be loops driven off an array, rather than hand-coded. Sure, will fix. I think this was discussed before, and I hate to remention it, but would it make sense to reuse the single-character codes from log_line_prefix rather than inventing new codes for the same fields? As I recall, Tom didn't like that idea and neither did I- there's only so many letters.. It also sucks wrt being self-documenting. I'd much rather support multi-line GUCs.. It would be awfully nice if we could inject something into the csvlog output format that would let client programs know which fields are present. This would be especially useful for people writing tools that are intended to work on ANY PG installation, rather than just, say, their own. I'm not sure if there's a clean way to do that, though. This would be called a 'header' in most typical CSV scenarios. Unfortunately, last I checked (maybe it's changed?), COPY w/ HEADER just throws the header away instead of doing anything useful with it. If it actually used the header to build the column list, then adding a header would be sufficient, provided all the necessary fields are in the table. If I wanted something to throw away the first record of a file before loading it, I'd use tail. I'll see about adding an option to have it output a header. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Extensions vs PGXS' MODULE_PATHNAME handling
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@sss.pgh.pa.us writes: But contrib/spi is exactly the case where it *won't* work. We need to somehow figure out that $libdir/autoinc is what to substitute in autoinc-1.0.sql, $libdir/insert_username in insert_username-1.0.sql, etc. Indeed. That's why I'm proposing to have that setup in the control file, which is per extension, rather than in the common Makefile. How's that help? In a makefile building more than one extension, you'd still need a way to decide which extension the current script file is associated with. Or are you suggesting substituting for MODULE_PATHNAME during CREATE EXTENSION, and not during make at all? That would work I guess. I'm hesitant to have any substitutions that happen unconditionally, but we could add a control parameter like module_pathname = '$libdir/hstore' and then things would be pretty clean. I think we should still change the file naming conventions to use double dashes, though, since there's more than one reason to want that. Will work on that next. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Debian readline/libedit breakage
On Sun, Feb 13, 2011 at 12:56:03PM +0100, Dimitri Fontaine wrote: Magnus Hagander mag...@hagander.net writes: Yes, since according to a comment somewhere the same issue will bubble into ubuntu soon. At this point, definitely 8.04 and 10.04, and probably 10.10. If things can be easily automated, it would be great if we could do *all* supported ubuntu, but doing LTS and the latest one or two non-LTS releases. Well, that needs I guess either several ubuntu VM or pbuilder support for ubuntu debootstraps, will check about that. As pbuilder just runs debootstrap on --create and (Debian) debootstrap supports the Ubuntu releases, this is not an issue. Michael -- 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] odd output of pg_basebackup
On Mon, Feb 7, 2011 at 13:43, Fujii Masao masao.fu...@gmail.com wrote: Hi, When I ran pg_basebackup with -x, -P and -v options, I encountered the following odd output. $ pg_basebackup -D hoge -x -P -v xlog start point: 0/220 33708/17322 kB (100%) 1/1 tablespaces ( )02) 02) is a part of the previously reported message, it looks odd that the subsequent progress report doesn't reset that. We should reset the output buffer with each progress report? Yes. I thought I had fixed that already.. Hmm. I'll stick it on my list.. -- Magnus Hagander Me: http://www.hagander.net/ Work: http://www.redpill-linpro.com/ -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
Tom Lane t...@sss.pgh.pa.us writes: Or are you suggesting substituting for MODULE_PATHNAME during CREATE EXTENSION, and not during make at all? That would work I guess. That's my idea, sorry not having made it clear enough. We have $libdir which is expanded server-side AFAIUI, I though we would have $shlib expanded the same way and taken from some backend variable like with creating_extension. I'm hesitant to have any substitutions that happen unconditionally, but we could add a control parameter like module_pathname = '$libdir/hstore' and then things would be pretty clean. Ok. Maybe the simpler would be to make the current control variable a static backend variable so that EXT_CONTROL(module_pathname) is easy to find out from anywhere (I see you got rid of some direct usage of static variables with recordDependencyOnCurrentExtension() already). I think we should still change the file naming conventions to use double dashes, though, since there's more than one reason to want that. Will work on that next. Great! Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] pl/python custom exceptions for SPI
On 11-02-12 05:58 AM, Jan Urbański wrote: On 11/02/11 10:53, Jan Urbański wrote: On 10/02/11 22:26, Steve Singer wrote: Here's an updated patch with documentation. It's an incremental patch on top of the latest explicit-subxacts version. This looks fine. I've attached a one word documentation change to go o top of the patch. I'll let Peter decide if he likes those assert's or not. I don't have a good enough sense of if we often use asserts in that fashion elsewhere. Cheers, Jan diff --git a/doc/src/sgml/plpython.sgml b/doc/src/sgml/plpython.sgml index aee54bf..4a90430 100644 *** a/doc/src/sgml/plpython.sgml --- b/doc/src/sgml/plpython.sgml *** $$ LANGUAGE plpythonu; *** 966,972 /programlisting /para para ! The actual class of the exception being raised corresponds to exact condition that caused the error (refer to xref linkend=errcodes-table for a list of possible conditions). The literalplpy.spiexceptions/literal module defines an exception class for --- 966,972 /programlisting /para para ! The actual class of the exception being raised corresponds to the exact condition that caused the error (refer to xref linkend=errcodes-table for a list of possible conditions). The literalplpy.spiexceptions/literal module defines an exception class for -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@sss.pgh.pa.us writes: I'm hesitant to have any substitutions that happen unconditionally, but we could add a control parameter like module_pathname = '$libdir/hstore' and then things would be pretty clean. Ok. Maybe the simpler would be to make the current control variable a static backend variable so that EXT_CONTROL(module_pathname) is easy to find out from anywhere (I see you got rid of some direct usage of static variables with recordDependencyOnCurrentExtension() already). I think it's better to keep it working as a textual substitution. That poses the least risk of breaking scripts that work today --- who's to say that somebody might not be relying on the substitution happening someplace else than CREATE FUNCTION's shlib string? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Debian readline/libedit breakage
Michael Banck mba...@debian.org writes: As pbuilder just runs debootstrap on --create and (Debian) debootstrap supports the Ubuntu releases, this is not an issue. Great. It seems that a single amd64 build VM would allow us to build all those binary packages for i386 and amd64, for several debian and ubuntu releases. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] SQL/MED - file_fdw
Noah Misch n...@leadboat.com wrote: On Sat, Feb 12, 2011 at 03:42:17PM -0600, Kevin Grittner wrote: Do you see any reason that COPY FROM should be significantly *faster* with the patch? No. Up to, say, 0.5% wouldn't be too surprising, but 8.4% is surprising. What is the uncertainty of that figure? With a few more samples, it's not that high. It's hard to dodge around the maintenance tasks on this machine to get good numbers, so I can't really just set something up to run overnight to get numbers in which I can have complete confidence, but (without putting statistical probabilities around it) I feel very safe in saying there isn't a performance *degradation* with the patch. I got four restores of of the 90GB data with the patch and four without. I made sure it was during windows without any maintenance running, did a fresh initdb for each run, and made sure that the disk areas were the same for each run. The times for each version were pretty tightly clustered except for each having one (slow) outlier. If you ignore the outlier for each, there is *no overlap* between the two sets -- the slowest of the non-outlier patched times is faster than the fastest non-patched time. With the patch, compared to without -- best time is 9.8% faster, average time without the outliers is 6.9% faster, average time including outliers is 4.3% faster, outlier is 0.8% faster. Even with just four samples each, since I was careful to minimize distorting factors, that seems like plenty to have confidence that there is no performance *degradation* from the patch. If we want to claim some particular performance *gain* from it, I would need to arrange a dedicated machine and script maybe 100 runs each way to be willing to offer a number for public consumption. Unpatched: real17m24.171s real16m52.892s real16m40.624s real16m41.700s Patched: real15m56.249s real15m47.001s real15m3.018s real17m16.157s Since you said that a cursory test, or no test at all, should be good enough given the low risk of performance regression, I didn't book a machine and script a large test run, but if anyone feels that's justified, I can arrange something. -Kevin -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
Tom Lane t...@sss.pgh.pa.us writes: I think it's better to keep it working as a textual substitution. That poses the least risk of breaking scripts that work today --- who's to say that somebody might not be relying on the substitution happening someplace else than CREATE FUNCTION's shlib string? Fair enough, I suppose. So +1 from me, FWIW. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@sss.pgh.pa.us writes: I think it's better to keep it working as a textual substitution. That poses the least risk of breaking scripts that work today --- who's to say that somebody might not be relying on the substitution happening someplace else than CREATE FUNCTION's shlib string? Fair enough, I suppose. So +1 from me, FWIW. OK, so with that, attached is an example of the complete conversion diff for a contrib module (hstore in particular). Although git status reports hstore--1.0.sql as being a rename of hstore.sql.in, git diff doesn't seem to be exceedingly bright about presenting it that way :-(. But actually the change in that script other than renaming is just removing the set search_path command and adjusting the header comment. I've checked that regression tests pass and create extension hstore from unpackaged successfully upgrades from a 9.0 dump. I don't have the ability to check that it works on Windows too, but since we're not hacking pgxs.mk I doubt that there's anything to do to the Windows build process. Barring objections, I'll press on with fixing the rest of them. regards, tom lane diff --git a/contrib/hstore/.gitignore b/contrib/hstore/.gitignore index d7af95330c380d468c35f781f34de30ea05709a5..19b6c5ba425ca92d1bb371bf43d9cdae372f8c1a 100644 *** a/contrib/hstore/.gitignore --- b/contrib/hstore/.gitignore *** *** 1,3 - /hstore.sql # Generated subdirectories /results/ --- 1,2 diff --git a/contrib/hstore/Makefile b/contrib/hstore/Makefile index 1d533fdd60280b1e62610dd7b98cdfb4151de1b4..5badbdb714b60cd786cffa86526a405bccfd1ea0 100644 *** a/contrib/hstore/Makefile --- b/contrib/hstore/Makefile *** MODULE_big = hstore *** 4,11 OBJS = hstore_io.o hstore_op.o hstore_gist.o hstore_gin.o hstore_compat.o \ crc32.o ! DATA_built = hstore.sql ! DATA = uninstall_hstore.sql REGRESS = hstore ifdef USE_PGXS --- 4,11 OBJS = hstore_io.o hstore_op.o hstore_gist.o hstore_gin.o hstore_compat.o \ crc32.o ! EXTENSION = hstore ! DATA = hstore--1.0.sql hstore--unpackaged--1.0.sql REGRESS = hstore ifdef USE_PGXS diff --git a/contrib/hstore/expected/hstore.out b/contrib/hstore/expected/hstore.out index 354fff20fe2b24127ac9ec1ae9a20f72d628e256..083faf8d9c433ba9f34a95f65fed64c0079a6561 100644 *** a/contrib/hstore/expected/hstore.out --- b/contrib/hstore/expected/hstore.out *** *** 1,12 ! -- ! -- first, define the datatype. Turn off echoing so that expected file ! -- does not depend on contents of hstore.sql. ! -- ! SET client_min_messages = warning; ! \set ECHO none ! psql:hstore.sql:228: WARNING: = is deprecated as an operator name DETAIL: This name may be disallowed altogether in future versions of PostgreSQL. - RESET client_min_messages; set escape_string_warning=off; --hstore; select ''::hstore; --- 1,6 ! CREATE EXTENSION hstore; ! WARNING: = is deprecated as an operator name DETAIL: This name may be disallowed altogether in future versions of PostgreSQL. set escape_string_warning=off; --hstore; select ''::hstore; diff --git a/contrib/hstore/hstore--1.0.sql b/contrib/hstore/hstore--1.0.sql index ...d77b14286bdce8af49bdad9620e00c5c4ce827fe . *** a/contrib/hstore/hstore--1.0.sql --- b/contrib/hstore/hstore--1.0.sql *** *** 0 --- 1,527 + /* contrib/hstore/hstore--1.0.sql */ + + CREATE TYPE hstore; + + CREATE OR REPLACE FUNCTION hstore_in(cstring) + RETURNS hstore + AS 'MODULE_PATHNAME' + LANGUAGE C STRICT IMMUTABLE; + + CREATE OR REPLACE FUNCTION hstore_out(hstore) + RETURNS cstring + AS 'MODULE_PATHNAME' + LANGUAGE C STRICT IMMUTABLE; + + CREATE OR REPLACE FUNCTION hstore_recv(internal) + RETURNS hstore + AS 'MODULE_PATHNAME' + LANGUAGE C STRICT IMMUTABLE; + + CREATE OR REPLACE FUNCTION hstore_send(hstore) + RETURNS bytea + AS 'MODULE_PATHNAME' + LANGUAGE C STRICT IMMUTABLE; + + CREATE TYPE hstore ( + INTERNALLENGTH = -1, + INPUT = hstore_in, + OUTPUT = hstore_out, + RECEIVE = hstore_recv, + SEND = hstore_send, + STORAGE = extended + ); + + CREATE OR REPLACE FUNCTION hstore_version_diag(hstore) + RETURNS integer + AS 'MODULE_PATHNAME','hstore_version_diag' + LANGUAGE C STRICT IMMUTABLE; + + CREATE OR REPLACE FUNCTION fetchval(hstore,text) + RETURNS text + AS 'MODULE_PATHNAME','hstore_fetchval' + LANGUAGE C STRICT IMMUTABLE; + + CREATE OPERATOR - ( + LEFTARG = hstore, + RIGHTARG = text, + PROCEDURE = fetchval + ); + + CREATE OR REPLACE FUNCTION slice_array(hstore,text[]) + RETURNS text[] + AS 'MODULE_PATHNAME','hstore_slice_to_array' + LANGUAGE C STRICT IMMUTABLE; + + CREATE OPERATOR - ( + LEFTARG = hstore, + RIGHTARG = text[], + PROCEDURE = slice_array + ); + + CREATE OR REPLACE FUNCTION slice(hstore,text[]) + RETURNS hstore + AS 'MODULE_PATHNAME','hstore_slice_to_hstore' + LANGUAGE C STRICT IMMUTABLE; + +
Re: [HACKERS] ALTER TYPE 2: skip already-provable no-work rewrites
On Sun, Feb 13, 2011 at 6:50 AM, Noah Misch n...@leadboat.com wrote: Yikes. I didn't like that Assert much, but maybe we need it, because this is scary. Can you elaborate on the fear-inducing aspect? I don't mind re-adding the Assert, but it seems that some positive understanding of the assumption's validity is in order. Well, it's pretty obvious that if somehow we were to go down that code path and not get back a value that was identical to the one we had before, we'd have a corrupted table. It seems that just about any refactoring of tablecmds.c might create such a possibility. Assertions are a good idea when the reasons why something is true are far-removed (in the code) from the places where they need to be true. I'm somewhat uncomfortable also with the dependency of this code on very subtle semantic differences between if (newrel) and if (tab-newvals != NIL). I think this would blow up and die if for any reason newrel were non-NULL but tab-newvals were NIL. Actually, doesn't that happen if we're adding or dropping an OID column? I think it might be cleaner to have tab-verify_constraints rather than tab-verify. Then ATRewriteTables() could test if (tab-verify_constraints || tab-new_notnull), and you wouldn't need the bit that sets at-verify if at-new_notnull is already set. That part makes the semantics of tab-verify depend on which point in processing we're at, which I have found to be a recipe for confusion in other parts of the source base (planner, I'm looking at you). Correct me if I'm wrong here, but we could handle the domains-without-contraints part here with about three additional lines of code, right? It's only the domains-with-constraints part that requires the rest of this. I'm half-tempted to put that part off to 9.2, in the hopes of getting a more substantial solution that can also handle things like text - xml which we don't have time to re-engineer right now. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions vs PGXS' MODULE_PATHNAME handling
Tom Lane t...@sss.pgh.pa.us writes:
Tom Lane t...@sss.pgh.pa.us writes:
I think it's better to keep it working as a textual substitution.
Thinking about this some more, it has the advantage that the effects of
the control file settings are kept within the script file processing and
pg_extension catalog. The only backend impact is the dependency
tracking.
OK, so with that, attached is an example of the complete conversion diff
for a contrib module (hstore in particular). Although git status
I see you're not using the @extschema@ placeholder in the upgrade
script. It is intentional? It's been common wisdom and practice to
edit the SQL file of any contrib or third party module to have it
installed in your preferred schema…
reports hstore--1.0.sql as being a rename of hstore.sql.in, git diff
doesn't seem to be exceedingly bright about presenting it that way :-(.
But actually the change in that script other than renaming is just
removing the set search_path command and adjusting the header comment.
And we don't have to rely on hstore.sql.in file anymore as the change is
done by the backend side of things. That's a very good point for the
windows build system I think.
Barring objections, I'll press on with fixing the rest of them.
I think you'd be interested into this reworked SQL query. It should be
providing exactly the script file you need as an upgrade from unpackaged.
I took the time to finish this query (filter out array types, some
replacement in operator classes and families descriptions) because I
think it would be nice to offer it in the docs. It could even be
proposed as a function :)
I hope you'll find it useful, but it could well be you finished the
searchreplace of all contribs already (ah, emacs keyboard macros).
CREATE EXTENSION hstore;
CREATE SCHEMA empty_place;
SET search_path TO empty_place;
WITH objs AS (
SELECT classid, 'ALTER EXTENSION ' || E.extname || ' ADD '
|| replace(pg_describe_object(classid, objid, 0),
N.nspname, '@extschema@')
|| ';' as sql
FROM pg_depend D
JOIN pg_extension E ON D.refobjid = E.oid
AND D.refclassid = E.tableoid
JOIN pg_namespace N ON E.extnamespace = N.oid
WHERE CASE WHEN classid = 'pg_catalog.pg_type'::regclass
THEN (SELECT typarray FROM pg_type WHERE oid=objid) != 0
ELSE true
END
AND deptype = 'e' AND E.extname = 'hstore'
)
SELECT
CASE WHEN classid IN ('pg_catalog.pg_opclass'::regclass,
'pg_catalog.pg_opfamily'::regclass)
THEN replace(sql, 'for access method', 'using')
ELSE sql
END
FROM objs;
Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--
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] Extensions vs PGXS' MODULE_PATHNAME handling
Dimitri Fontaine dimi...@2ndquadrant.fr writes: Tom Lane t...@sss.pgh.pa.us writes: OK, so with that, attached is an example of the complete conversion diff for a contrib module (hstore in particular). Although git status I see you're not using the @extschema@ placeholder in the upgrade script. It is intentional? Yes, it should be unnecessary given the search_path setup done by execute_extension_script(). Also, I think that a relocatable extension's script should not be subject to @extschema@ substitution, no matter what. I think you'd be interested into this reworked SQL query. It should be providing exactly the script file you need as an upgrade from unpackaged. This seems overly complicated. I have a version of it that I'll publish as soon as I've tested it on all the contrib modules ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Change pg_last_xlog_receive_location not to move backwards
On Sun, Jan 30, 2011 at 11:12 PM, Fujii Masao masao.fu...@gmail.com wrote: On Sun, Jan 30, 2011 at 10:44 AM, Jeff Janes jeff.ja...@gmail.com wrote: I do not understand what doing so gets us. Say we previously received 2/3 of a WAL file, and replayed most of it. So now the shared buffers have data that has been synced to that WAL file already, and some of those dirty shared buffers have been written to disk and some have not. At this point, we need the data in the first 2/3 of the WAL file in order to reach a consistent state. But now we lose the connection to the master, and then we restore it. Now we request the entire file from the start rather than from where it left off. Either of two things happens. Most likely, the newly received WAL file matches the file it is overwriting, in which case there was no point in asking for it. Less likely, the master is feeding us gibberish. By requesting the full WAL file, we check the header and detect that the master is feeding us gibberish. Unfortunately, we only detect that fact *after* we have replaced a critical part of our own (previously good) copy of the WAL file with said gibberish. The standby is now in an unrecoverable state. Right. To avoid this problem completely, IMO, walreceiver should validate the received WAL data before writing it. Or, walreceiver should write the WAL to the transient file, and the startup process should rename it to the correct name after replaying it. We should do something like the above? I don't think we should overwrite local WAL that has already been applied, unless we already know for sure that it is bad (and suspect re-streaming might make it right.) I think the better approach would be to check the existence, and the segment header, of the WAL segment we already (think we) have in pg_xlog. And then request re-streaming of that file from the beginning only if it is either missing or has the wrong header in the local copy. That might make the code a lot more complex, though. At least I don't see a simple way to implement it. With a bit of malicious engineering, I have created this situation. I don't know how likely it is that something like that could happen accidentally, say with a corrupted file system. I have been unable to engineer a situation where checking the header actually does any good. It has either done nothing, or done harm. OK, I seem to have to consider again why the code which retreats the replication starting location exists. At first, I added the code to prevent a half-baked WAL file. For example, please imagine the case where you start the standby server with no WAL files in pg_xlog. In this case, if replication starts from the middle of WAL file, the received WAL file is obviously broken (i.e., with no WAL data in the first half of file). This broken WAL file might cause trouble when we restart the standby and even when we just promote it (because the last applied WAL file is re-fetched at the end of recovery). OTOH, we can start replication from the middle of WAL file if we can *ensure* that the first half of WAL file already exists. At least, when the standby reconnects to the master, we might be able to ensure that and start from the middle. OK, thanks for the explanation. Is there a race condition here? That is, it seems that with your patch this part of the code could get executed after streaming is restarted, but before the streaming ever actually received and wrote anything. So it would be checking the old header, not the newly about-to-be received header. As far as I read the code again, there is no such a race condition. When the received WAL is read just after streaming is restarted, that part of the code seems to always be executed. Maybe I'm missing something. Could you explain about the problematic scenario? OK, I think you are right. I was thinking that if apply is well behind receive when the connection is lost, that a new connection could immediately pass the havedata test an so proceed to the header check perhaps before anything was received. But now I see that we never try to re-connect until after apply has caught up with receive, so that race condition cannot happen. Sorry for the false alarm. By the way, the patch no longer applies cleanly to HEAD, as of commit d309acf201ab2c5 (Feb 11th). But it looks like a trivial conflict (Two different patches both correct the same doc typo). Cheers, Jeff -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
Tom Lane t...@sss.pgh.pa.us writes: Yes, it should be unnecessary given the search_path setup done by execute_extension_script(). Also, I think that a relocatable extension's script should not be subject to @extschema@ substitution, no matter what. Oh I'm just realizing that my reasoning predates the search_path strong guarantees at CREATE EXTENSION time. I think you'd be interested into this reworked SQL query. It should be providing exactly the script file you need as an upgrade from unpackaged. This seems overly complicated. I have a version of it that I'll publish as soon as I've tested it on all the contrib modules ... Nice. I confess I worked out mine from my last patch where I still have the INTERNAL dependencies setup etc, so maybe that makes it more complex that it needs to be. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] [Mingw-users] mingw64
On 02/12/2011 05:10 AM, Ralf Wildenhues wrote: Hello, and sorry for the delay, * Peter Rosin wrote on Sat, Jan 29, 2011 at 02:26:24PM CET: Or is plain 'ar' used somewhere instead of 'x86_64-w64-mingw32-ar'? Automake outputs 'AR = ar' in Makefile.in for rules creating old libraries iff neither AC_PROG_LIBTOOL nor another method to define AR correctly is used in configure.ac. So this issue concerns packages using Automake but not using Libtool. I figured with AM_PROG_AR eventually being needed anyway that would fix this in one go ... A good workaround, as already mentioned, is to use this in configure.ac: AC_CHECK_TOOL([AR], [ar], [false]) This was sorted out some time ago. 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] SQL/MED - file_fdw
On 02/12/2011 05:33 PM, Noah Misch wrote: On Sat, Feb 12, 2011 at 03:42:17PM -0600, Kevin Grittner wrote: In two hours of testing with a 90GB production database, the copy patch on top of HEAD ran 0.6% faster than HEAD for pg_dumpall (generating identical output files), but feeding that in to and empty cluster with psql ran 8.4% faster with the patch than without! I'm going to repeat that latter with more attention to whether everything made it in OK. (That's not as trivial to check as the dump phase.) Do you see any reason that COPY FROM should be significantly *faster* with the patch? No. Up to, say, 0.5% wouldn't be too surprising, but 8.4% is surprising. What is the uncertainty of that figure? We have seen in the past that changes that might be expected to slow things down slightly can have the opposite effect. For example, see http://people.planetpostgresql.org/andrew/index.php?/archives/37-Puzzling-results.html where Tom commented: Yeah, IME it's not unusual for microbenchmark results to move a percent or three in response to any code change at all, even unrelated ones. I suppose it's from effects like critical loops breaking across cache lines differently than before. 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] Scheduled maintenance affecting gitmaster
Magnus Hagander mag...@hagander.net writes: Unfortunately, one of the worst-case scenarios appears to have happened - a machine did not come back up after a reboot. ... We'll get back to you with more information as soon as we have it. I didn't see any followup to this? gitmaster seems to be responding as of now, is it safe to push? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add support for logging the current role
On 02/13/2011 08:26 AM, Stephen Frost wrote: This would be called a 'header' in most typical CSV scenarios. Unfortunately, last I checked (maybe it's changed?), COPY w/ HEADER just throws the header away instead of doing anything useful with it. If it actually used the header to build the column list, then adding a header would be sufficient, provided all the necessary fields are in the table. See the discussion back around the the 8.1 release (IIRC) when we added the HEADER option. If I wanted something to throw away the first record of a file before loading it, I'd use tail. The whole point of us having direct CSV import is to minimise the requirement for preprocessing. That said, I think there's probably a good case for an option to use the header line as a column list. I know of at least one application I have written that could benefit from it. But that's work for 9.2 or later. 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] Extensions vs PGXS' MODULE_PATHNAME handling
On Feb 13, 2011, at 11:34 AM, Tom Lane wrote: OK, so with that, attached is an example of the complete conversion diff for a contrib module (hstore in particular). Although git status reports hstore--1.0.sql as being a rename of hstore.sql.in, git diff doesn't seem to be exceedingly bright about presenting it that way :-(. But actually the change in that script other than renaming is just removing the set search_path command and adjusting the header comment. I sure would like it if the install script with no version in it corresponded to the latest version. Otherwise, one must rename the file every time one does a release. And as you're noting, you lose Git history that way. Best, David -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
David E. Wheeler da...@kineticode.com writes: I sure would like it if the install script with no version in it corresponded to the latest version. Otherwise, one must rename the file every time one does a release. And as you're noting, you lose Git history that way. (1) git does know it's a rename, it's just not default for git diff to show it that way. (2) I think that the normal use-case would not involve removing the old file, so this is moot anyhow. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions vs PGXS' MODULE_PATHNAME handling
On Feb 13, 2011, at 4:46 PM, Tom Lane wrote: I sure would like it if the install script with no version in it corresponded to the latest version. Otherwise, one must rename the file every time one does a release. And as you're noting, you lose Git history that way. (1) git does know it's a rename, it's just not default for git diff to show it that way. I see, looks like one can `git diff --follow` to see it that way: http://stackoverflow.com/questions/2314652/ (2) I think that the normal use-case would not involve removing the old file, so this is moot anyhow. Oh. So one normally will ship, for an extension foo, only foo.sql and any necssary upgrade scripts? Best, David -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
David E. Wheeler da...@kineticode.com writes: On Feb 13, 2011, at 4:46 PM, Tom Lane wrote: (2) I think that the normal use-case would not involve removing the old file, so this is moot anyhow. Oh. So one normally will ship, for an extension foo, only foo.sql and any necssary upgrade scripts? I think after a couple of releases you'd be shipping something like foo--1.0.sql foo--1.1.sql foo--1.0--1.1.sql foo--2.0.sql foo--1.1--2.0.sql and it'll soon get to be a mess if your SCM doesn't clearly distinguish which is which. Also, as I mentioned before, once you've branched off foo--1.1.sql it's probably a mistake to be changing foo--1.0.sql anymore anyway. I suppose if you really wanted foo.sql to always be the head version, you could do something like cp foo.sql foo--$VERSION.sql as part of the build process in the Makefile. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Extensions vs PGXS' MODULE_PATHNAME handling
On Feb 13, 2011, at 4:59 PM, Tom Lane wrote: I think after a couple of releases you'd be shipping something like foo--1.0.sql foo--1.1.sql foo--1.0--1.1.sql foo--2.0.sql foo--1.1--2.0.sql and it'll soon get to be a mess if your SCM doesn't clearly distinguish which is which. Also, as I mentioned before, once you've branched off foo--1.1.sql it's probably a mistake to be changing foo--1.0.sql anymore anyway. I suppose if you really wanted foo.sql to always be the head version, you could do something like cp foo.sql foo--$VERSION.sql as part of the build process in the Makefile. That would be okay. Is $EXTVERSION still defined in the Makefile? ($VERSION is the PostgreSQL version, of course). Best, David -- 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] Extensions vs PGXS' MODULE_PATHNAME handling
David E. Wheeler da...@kineticode.com writes: On Feb 13, 2011, at 4:59 PM, Tom Lane wrote: I suppose if you really wanted foo.sql to always be the head version, you could do something like cp foo.sql foo--$VERSION.sql as part of the build process in the Makefile. That would be okay. Is $EXTVERSION still defined in the Makefile? ($VERSION is the PostgreSQL version, of course). I haven't set the contrib makefiles up that way, but of course you could do it if you wanted to in your own makefiles. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Add support for logging the current role
Robert, * Robert Haas (robertmh...@gmail.com) wrote: On Fri, Feb 11, 2011 at 11:23 AM, Stephen Frost sfr...@snowman.net wrote: Updated patch attached, full git log below. Thanks again for the review. Updated patch attached. The documentation for csvlog_fields should probably use literal around the default value. Fixed. The sentence that begins For details on what these fields are should hyperlink the referenced sections of the documentation. Fixed. The function prototype you added to elog.c is misformatted - the type should be on the line preceding the function name only for the definition, not for prototypes. Fixed. The code for log_line_prefix = 'u' is indented wrong. Also, an else clause that has only an if hanging off of it can be turned into an else if for better readability. Fixed. This part kind of concerns me: + * This is more of a 'safety valve' than anything else, + * since GUC processing really should happen before we do any error logging. + * We might even want to change this eventually to just not log CSV format logs + * if this ever happens, to avoid a discrepency in the CSV log file which would + * make it difficult to load into PG. I'm not really convinced that making the CSV log format variable is a good thing. One of the reasons we added that format in the first place is to make sure that we could generate log output in an easily parseable format, and this seems like a big step backwards for not much, especially if we can't even guarantee that we're not going to inject random differently-formatted lines during startup. Comment, function, and whole issue removed. Stylistically, build_default_csvlog_list and assign_csvlog_fields ought to be loops driven off an array, rather than hand-coded. Done, added CSVFieldNames and modiffied assign_csvlog_fileds to use it (build_default_csvlog_list was removed). I think this was discussed before, and I hate to remention it, but would it make sense to reuse the single-character codes from log_line_prefix rather than inventing new codes for the same fields? I'd rather ditch the log_line_prefix idea of single-letter codes since it's never going to scale. Perhaps making log_line_prefix with work %csvlog_name instead of just the %single-letter codes might work. I don't see a solution which doesn't involve changing log_line_prefix though, in any case. It would be awfully nice if we could inject something into the csvlog output format that would let client programs know which fields are present. This would be especially useful for people writing tools that are intended to work on ANY PG installation, rather than just, say, their own. I'm not sure if there's a clean way to do that, though. Added csvlog_header GUC to allow the user to ask for the header to be printed at the top of each log file. If and when an option is added to PG's COPY to respect the header, this should resolve that issue. Also updated to HEAD. Full git log below, patch attached. Thanks, Stephen commit 592c2564ffde77fc29ff28fdedd2c9f2dafd Merge: 33639eb cebbaa1 Author: Stephen Frost sfr...@snowman.net Date: Sun Feb 13 21:11:44 2011 -0500 Merge branch 'master' of git://git.postgresql.org/git/postgresql into log_csv_options commit 33639ebfe67b0dd58a0a89161e9f0d5237830ed4 Author: Stephen Frost sfr...@snowman.net Date: Sun Feb 13 21:08:08 2011 -0500 Add csvlog_header GUC, other cleanup This patch adds a csvlog_header option which will start each CSV log file with a header which matches the GUC (and hence the format of the CSV log file generated). Numerous other whitespace clean-ups, removed build_default_csvlog_list(), since it wasn't actually necessary or useful. Added an array which lists the text strings of the various CSVLOG options to simplify assign_csvlog_fields(). commit 6bd2b9f1d2bc3b166a3e5598ee590e25159c61a5 Author: Stephen Frost sfr...@snowman.net Date: Fri Feb 11 11:16:17 2011 -0500 Rename log_csv_fields GUC to csvlog_fields This patch renames the log_csv_fileds GUC to csvlog_fields, to better match the other csvlog_* options. Also cleaned up the CSV generation code a bit by moving the comma-adding code out of the switch() statement. commit a281ca611e6181339e92b488c815e0cb8c1298d2 Merge: d81 183d3cf Author: Stephen Frost sfr...@snowman.net Date: Fri Feb 11 08:37:27 2011 -0500 Merge branch 'master' of git://git.postgresql.org/git/postgresql into log_csv_options commit d81c425a4c320540769084ceeb7d23bc3662 Author: Stephen Frost sfr...@snowman.net Date: Sun Feb 6 14:02:05 2011 -0500 Change log_csv_options listing to a table This patch changes the listing of field options available to log_csv_options into a table, which will hopefully both look better and be clearer. commit f9851cdfaeb931f01c015f5651b72d16957c7114 Merge: 3e71e33 5ed45ac Author: Stephen
Re: [HACKERS] Add support for logging the current role
Andrew, * Andrew Dunstan (and...@dunslane.net) wrote: See the discussion back around the the 8.1 release (IIRC) when we added the HEADER option. I recall seeing it and not agreeing with it then. :) If I wanted something to throw away the first record of a file before loading it, I'd use tail. The whole point of us having direct CSV import is to minimise the requirement for preprocessing. Having a 'throw away 1 line' option is just silly to me, but documenting it as header is worse. Water under the bridge at this point though. That said, I think there's probably a good case for an option to use the header line as a column list. I know of at least one application I have written that could benefit from it. But that's work for 9.2 or later. I agree it's work for 9.2. I could probably help with this if people agree that it should be done. Thanks, Stephen signature.asc Description: Digital signature
Re: pg_ctl failover Re: [HACKERS] Latches, signals, and waiting
Thanks for the review! On Thu, Feb 10, 2011 at 11:25 PM, Magnus Hagander mag...@hagander.net wrote: I see that the docs part of the patch removes the mentioning of reporting servers - is that intentional, or a mistake? Seems that usecase still remains, no? It was intentional, but I agree with you. I re-added the mention to the reporting servers. On Thu, Feb 10, 2011 at 11:30 PM, Magnus Hagander mag...@hagander.net wrote: Also, the patch no longer applies, since it conflicts with faa0550572583f51dba25611ab0f1d1c31de559b. Since you (Fujii-san) wrote both of them, feel like rebasing it properly for current master? Yeah, I rebased the patch to the current git master and attached it. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center pg_ctl_promote_v4.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] Sync Rep for 2011CF1
On Fri, Feb 11, 2011 at 4:06 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
I committed the patch with those changes, and some minor comment tweaks and
other kibitzing.
+* 'd' means a standby reply wrapped in a COPY BOTH packet.
+*/
Typo: s/COPY BOTH/CopyData
+ msgtype = pq_getmsgbyte(input_message);
+ if (msgtype != 'r')
+ ereport(COMMERROR,
+ (errcode(ERRCODE_PROTOCOL_VIOLATION),
+errmsg(unexpected message type %c, msgtype)));
I think that proc_exit(0) needs to be called in error case.
+ static StringInfoData input_message;
+ StandbyReplyMessage reply;
+ char msgtype;
+
+ initStringInfo(input_message);
Doesn't the repeat of initStringInfo() cause the memory leak?
@@ -518,6 +584,7 @@ WalSndLoop(void)
{
if (!XLogSend(output_message, caughtup))
break;
+ ProcessRepliesIfAny();
Why is ProcessRepliesIfAny() required there?
We added new columns write_location, flush_location and
apply_location. So, for the sake of consistency, the column
name sent_location should be changed to send_location?
Regards,.
--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center
--
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] Sync Rep for 2011CF1
On Fri, Feb 11, 2011 at 4:06 AM, Heikki Linnakangas heikki.linnakan...@enterprisedb.com wrote: I added a XLogWalRcvSendReply() call into XLogWalRcvFlush() so that it also sends a status update every time the WAL is flushed. If the walreceiver is busy receiving and flushing, that would happen once per WAL segment, which seems sensible. This change can make the callback function WalRcvDie() call ereport(ERROR) via XLogWalRcvFlush(). This looks unsafe. Regards, -- Fujii Masao NIPPON TELEGRAPH AND TELEPHONE CORPORATION NTT Open Source Software Center -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Extension versus module
Appendix F (contrib.sgml and its subsidiary files) is pretty consistent about using module to refer to a contrib, uh, module. I considered doing a search-and-replace to change this to extension, but I'm not convinced that's a good idea. I think extension means a specific kind of SQL object that we just invented, and it's not exactly the same concept as one of those subdirectories under contrib/. One pretty obvious example is that contrib/spi calls itself a module, and it's definitely not an extension --- it contains five extensions, none of them named spi. Another problem is that we'd like to speak of upgrading a module from pre-9.1 to 9.1, and in only one of those two states is it strictly correct to call it an extension. But in some sense it's still the same entity. So I'm not sure whether to change the text at all. Comments? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Scheduled maintenance affecting gitmaster
On 02/14/2011 01:27 AM, Tom Lane wrote: Magnus Hagandermag...@hagander.net writes: Unfortunately, one of the worst-case scenarios appears to have happened - a machine did not come back up after a reboot. ... We'll get back to you with more information as soon as we have it. I didn't see any followup to this? yeah - the hosting company managed to reboot the box for us which brought it back to life in the middle of the night (with both magnus and me asleep). gitmaster seems to be responding as of now, is it safe to push? yes it is - however we will need to schedule another maintenance window soon to finish the stuff we actually wanted to do. Stefan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI bug?
hi, I wrote: it seems likely that such a cycle might be related to this new code not properly allowing for some aspect of tuple cleanup. I found a couple places where cleanup could let these fall through the cracks long enough to get stale and still be around when a tuple ID is re-used, causing problems. Please try the attached patch and see if it fixes the problem for you. If it does, then there's no need to try to track the other things I was asking about. thanks. unfortunately the problem still happens with the patch. YAMAMOTO Takashi Thanks! -Kevin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] SSI bug?
hi,
all of the following answers are with the patch you provided in
other mail applied.
YAMAMOTO Takashi y...@mwd.biglobe.ne.jp wrote:
i have seen this actually happen. i've confirmed the creation of
the loop with the attached patch. it's easily reproducable with
my application. i can provide the full source code of my
application if you want. (but it isn't easy to run unless you are
familiar with the recent version of NetBSD)
i haven't found a smaller reproducible test case yet.
I've never used NetBSD, so maybe a few details will help point me in
the right direction faster than the source code.
Has your application ever triggered any of the assertions in the
code? (In particular, it would be interesting if it ever hit the
one right above where you patched.)
the assertion right above is sometimes triggered. sometimes not.
How long was the loop?
see below.
Did you notice whether the loop involved multiple tuples within a
single page?
if i understand correctly, yes.
the following is a snippet of my debug code (dump targets when
triggerCheckTargetForConflictsIn loops 1000 times) and its output.
the same locktag_field3 value means the same page, right?
+ for (t = target, i = 0; t != NULL; i++) {
+ elog(WARNING, [%u] target %p tag % PRIx32 :% PRIx32 :% PRIx32
+ :% PRIx16 :% PRIx16 prior %p next %p, i, t,
+ t-tag.locktag_field1,
+ t-tag.locktag_field2,
+ t-tag.locktag_field3,
+ t-tag.locktag_field4,
+ t-tag.locktag_field5,
+ t-priorVersionOfRow,
+ t-nextVersionOfRow);
+ t = t-priorVersionOfRow;
+ if (t == target) {
+ elog(WARNING, found a loop);
+ break;
+ }
+ }
WARNING: [0] target 0xbb51f238 tag 4000:4017:53b:6c:0 prior 0xbb51f350 next
0xbb51f350
WARNING: [1] target 0xbb51f350 tag 4000:4017:53b:69:0 prior 0xbb51f238 next
0xbb51f238
WARNING: found a loop
another sample:
WARNING: [0] target 0xbb51f530 tag 4000:4017:565:ae:0 prior 0xbb51f1e8 next
0xbb51f300
WARNING: [1] target 0xbb51f1e8 tag 4000:4017:565:ad:0 prior 0xbb51f580 next
0xbb51f530
WARNING: [2] target 0xbb51f580 tag 4000:4017:565:ac:0 prior 0xbb51f300 next
0xbb51f1e8
WARNING: [3] target 0xbb51f300 tag 4000:4017:565:ab:0 prior 0xbb51f530 next
0xbb51f580
WARNING: found a loop
the table seems mostly hot-updated, if it matters.
hoge=# select * from pg_stat_user_tables where relid=16407;
-[ RECORD 1 ]-+
relid | 16407
schemaname| pgfs
relname | file
seq_scan | 0
seq_tup_read | 0
idx_scan | 53681
idx_tup_fetch | 52253
n_tup_ins | 569
n_tup_upd | 12054
n_tup_del | 476
n_tup_hot_upd | 12041
n_live_tup| 93
n_dead_tup| 559
last_vacuum |
last_autovacuum |
last_analyze |
last_autoanalyze |
vacuum_count | 0
autovacuum_count | 0
analyze_count | 4922528128875102208
autoanalyze_count | 7598807461784802080
(values in the last two columns seems bogus.
i don't know if it's related or not.)
Did this coincide with an autovacuum of the table?
no.
(assuming that autovacuum=off in postgresql.conf is enough to exclude
the possibility.)
These last two are of interest because it seems likely that such a
cycle might be related to this new code not properly allowing for
some aspect of tuple cleanup.
Thanks for finding this and reporting it, and thanks in advance for
any further detail you can provide.
thanks for looking.
YAMAMOTO Takashi
-Kevin
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] MSVC build scripts not up to speed for extensions
Buildfarm results remind me that I forgot to think about updating the MSVC perl scripts to match the recent changes in pgxs.mk. I think the relevant points are: 1. Install .control files matching the entries in EXTENSION. 2. If EXTENSION is defined, the default value of MODULEDIR should be taken as extension not contrib. Too tired to look at fixing this myself right now ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
