Re: [HACKERS] Building PL/Perl with ActiveState Perl 5.22 and MSVC
On Sat, Mar 26, 2016 at 03:43:21PM +0300, Victor Wagner wrote:
> It turns out that while ActiveState seems to drop support of embedding
> their perl into msvc-compiled appications, there are just few minor
> issues which prevent PL-perl to compile.
>
> 1. ActiveState Perl doesn't ship MSVC-build import library perl522.lib
> for their perl522.dll. Instead they ship MINGW-build library
> libperl522.a.
>
> Visual Studio 2012 is able to link against this library. It is only
> matter of modifing search expresions in Mkvcbuild.pm to be able to find
> this import library. Not sure if it stands true for all eariler
> versions of Visual Studio, supported by Postgresql.
>
> 2. There is macro PERL_STATIC_INLINME in perl's lib/CORE/config.h file,
> which produces compilation errors.
> #define PERL_STATIC_INLINE static __inline__ /**/
I've seen the same problems, and I converted your description into the
attached patch. With ActivePerl-5.24.1.2402-MSWin32-x64-401627.exe binaries,
"vcregress plcheck" passes. I plan to back-patch this. If some site has
worked around this with "copy libperl522.a perl522.lib", that site's build
will fail due to having two matching libraries. The build failure and fix
will be clear enough, so that seems acceptable.
> 3. Fixing two issues above was enough to make build complete for 64-bit
> windows target. With 32-bit build I'v got linking errors
> LINK2026: module unsafe for safeseh image
>
> for all modules which are linked with perl DLL. I suspect that it is
> not a problem with DLL itself, it is rather related to way MINGW32
> generates its import libraries. To fix this problem XML element
> false
> should be added inside element of the two vcxproj files which
> link with perl - plperl.vcxproj and hstore_plperl.vcxproj.
Official 10.1 x86 binaries[1] contain a SAFESEH build of plperl.dll, linked
with a perl524.dll. I wonder how. Do they use a perl524.dll from a source
other than ActivePerl? Before adopting your proposal, I'd like to understand
why the official binaries haven't needed it. Also, I'd instead use
"", which omits /SAFESEH entirely instead of
passing /SAFESEH:NO. That way, only the binaries linked to Perl (or other
old-fashioned DLLs) lose their safe exception handler table.
Even if I do this, "vcregress plcheck" fails with "loadable library and perl
binaries are mismatched (got handshake key 0B080080, needed 0AF00080)".
That's building with ActivePerl-5.22.4.2205-MSWin32-x86-64int-403863.exe and
VS2015. When I have more information, I'll send it to the thread[2] about
that failure mode.
Thanks,
nm
[1]
https://get.enterprisedb.com/postgresql/postgresql-10.1-1-windows-binaries.zip
[2]
https://postgr.es/m/flat/CAE9k0P%3DhMFew%3DVxNGTeOJJr32%2BUDy-o2qWXrThHg524EBqvnZQ%40mail.gmail.com
diff --git a/src/pl/plperl/plperl.h b/src/pl/plperl/plperl.h
index c4e06d0..aac95f8 100644
--- a/src/pl/plperl/plperl.h
+++ b/src/pl/plperl/plperl.h
@@ -42,6 +42,14 @@
#undef vsnprintf
#endif
+/*
+ * ActivePerl 5.18 and later are MinGW-built, and their headers use GCC's
+ * __inline__. Translate to something MSVC recognizes.
+ */
+#ifdef _MSC_VER
+#define __inline__ inline
+#endif
+
/*
* Get the basic Perl API. We use PERL_NO_GET_CONTEXT mode so that our code
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index 686c736..4c2e12e 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -615,9 +615,10 @@ sub mkvcbuild
}
}
$plperl->AddReference($postgres);
- my $perl_path = $solution->{options}->{perl} .
'\lib\CORE\perl*.lib';
+ my $perl_path = $solution->{options}->{perl} .
'\lib\CORE\*perl*';
+ # ActivePerl 5.16 provided perl516.lib; 5.18 provided
libperl518.a
my @perl_libs =
- grep { /perl\d+.lib$/ } glob($perl_path);
+ grep { /perl\d+\.lib$|libperl\d+\.a$/ } glob($perl_path);
if (@perl_libs == 1)
{
$plperl->AddLibrary($perl_libs[0]);
--
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] [bug fix] postgres.exe crashes with access violation on Windows while starting up
On Sat, Oct 28, 2017 at 03:43:02PM -0700, Michael Paquier wrote: > couldn't we envisage to just use > malloc? pgwin32_message_to_UTF16() is called in two places in elog.c, > and there is a full control on the error code paths. Switching to malloc is feasible, but it wouldn't enable PostgreSQL to handle non-ASCII messages any earlier. Messages should be ASCII-only until the init_locale(LC_CTYPE) call initializes MessageEncoding. (Before that call, pgwin32_message_to_UTF16() assumes the message is UTF8-encoded. I've expanded the comments slightly. We easily comply with that restriction today.) On Fri, Nov 03, 2017 at 11:10:14AM +, Michael Paquier wrote: > I am > switching the patch as ready for committer, I definitely agree that > you are taking the write approach here. Committed both patches. -- 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] possible encoding issues with libxml2 functions
On Sun, Nov 05, 2017 at 06:10:04PM +0100, Pavel Stehule wrote: > Hi > > 2017-11-05 4:07 GMT+01:00 Noah Misch : > > > On Tue, Oct 17, 2017 at 06:06:40AM +0200, Pavel Stehule wrote: > > > Please, if you can, try it write. I am little bit lost :) > > > > I'm attaching the patch I desired. Please review. This will probably miss > > this week's minor releases. If there's significant support, I could > > instead > > push before the wrap. > > > > I have not any objection to this solution. It fixes my regress tests too. > > I checked it and it is working. Pushed, but the buildfarm shows I didn't get the test quite right for the non-xml, non-UTF8 case. Fixing. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Race to build pg_isolation_regress in "make -j check-world"
I've been enjoying the speed of parallel check-world, but I get spurious failures from makefile race conditions. Commit c66b438 fixed the simple ones. More tricky is this problem of multiple "make" processes entering src/test/regress concurrently, which causes failures like these: gcc: error: pg_regress.o: No such file or directory make[4]: *** [pg_isolation_regress] Error 1 /bin/sh: ../../../src/test/isolation/pg_isolation_regress: Permission denied make -C test_extensions check make[2]: *** [check] Error 126 make[2]: Leaving directory `/home/nm/src/pg/backbranch/10/src/test/isolation' /bin/sh: ../../../../src/test/isolation/pg_isolation_regress: Text file busy make[3]: *** [isolationcheck] Error 126 make[3]: Leaving directory `/home/nm/src/pg/backbranch/10/src/test/modules/snapshot_too_old' This is reproducible since commit 2038bf4 or earlier; "make -j check-world" had worse problems before that era. A workaround is to issue "make -j; make -j -C src/test/isolation" before the check-world. This problem doesn't affect src/test/regress/pg_regress. Every top-level "make" or "make install", including temp-install, builds pg_regress. I tried fixing this by building src/test/isolation at the same times we run install-temp. Naturally, that didn't help installcheck-world. It also caused multiple "make" processes to enter src/port concurrently. I could fix both check-world and installcheck-world with the attached hack of building src/test/isolation during every top-level build or install. The problem of multiple "make" processes in a directory (especially src/port) shows up elsewhere. In a cleaned tree, "make -j -C src/bin" or "make -j installcheck-world" will do it. For more-prominent use cases, src/Makefile prevents this with ".NOTPARALLEL:" and building first the directories that are frequent submake targets. Perhaps we could fix the general problem with directory locking; targets that call "$(MAKE) -C FOO" would first sleep until FOO's lock is available. That could be tricky to make robust. For now, I propose back-patching the attached, sad hack. Better ideas? Thanks, nm diff --git a/src/Makefile b/src/Makefile index 380da92..febbced 100644 --- a/src/Makefile +++ b/src/Makefile @@ -28,6 +28,7 @@ SUBDIRS = \ pl \ makefiles \ test/regress \ + test/isolation \ test/perl # There are too many interdependencies between the subdirectories, so diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile index 8eb4969..efbdc40 100644 --- a/src/test/isolation/Makefile +++ b/src/test/isolation/Makefile @@ -15,6 +15,13 @@ OBJS = specparse.o isolationtester.o $(WIN32RES) all: isolationtester$(X) pg_isolation_regress$(X) +# Though we don't install these binaries, build them during installation +# (including temp-install). Otherwise, "make -j check-world" and "make -j +# installcheck-world" would spawn multiple, concurrent builds in this +# directory. Later builds would overwrite files while earlier builds are +# reading them, causing occasional failures. +install: | all + submake-regress: $(MAKE) -C $(top_builddir)/src/test/regress pg_regress.o -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Account for catalog snapshot in PGXACT->xmin updates.
On Sat, Nov 04, 2017 at 12:23:36PM -0400, Tom Lane wrote: > Noah Misch writes: > > I plan to use the attached patch after the minor release tags land. If > > there's significant support, I could instead push before the wrap. > > This looks fine to me --- I think you should push now. Done. -- 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] possible encoding issues with libxml2 functions
On Tue, Oct 17, 2017 at 06:06:40AM +0200, Pavel Stehule wrote:
> Please, if you can, try it write. I am little bit lost :)
I'm attaching the patch I desired. Please review. This will probably miss
this week's minor releases. If there's significant support, I could instead
push before the wrap.
diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c
index 24229c2..3e3aed8 100644
--- a/src/backend/utils/adt/xml.c
+++ b/src/backend/utils/adt/xml.c
@@ -3845,6 +3845,7 @@ xpath_internal(text *xpath_expr_text, xmltype *data,
ArrayType *namespaces,
int32 xpath_len;
xmlChar*string;
xmlChar*xpath_expr;
+ size_t xmldecl_len = 0;
int i;
int ndim;
Datum *ns_names_uris;
@@ -3900,6 +3901,16 @@ xpath_internal(text *xpath_expr_text, xmltype *data,
ArrayType *namespaces,
string = pg_xmlCharStrndup(datastr, len);
xpath_expr = pg_xmlCharStrndup(VARDATA_ANY(xpath_expr_text), xpath_len);
+ /*
+* In a UTF8 database, skip any xml declaration, which might assert
+* another encoding. Ignore parse_xml_decl() failure, letting
+* xmlCtxtReadMemory() report parse errors. Documentation disclaims
+* xpath() support for non-ASCII data in non-UTF8 databases, so leave
+* those scenarios bug-compatible with historical behavior.
+*/
+ if (GetDatabaseEncoding() == PG_UTF8)
+ parse_xml_decl(string, &xmldecl_len, NULL, NULL, NULL);
+
xmlerrcxt = pg_xml_init(PG_XML_STRICTNESS_ALL);
PG_TRY();
@@ -3914,7 +3925,8 @@ xpath_internal(text *xpath_expr_text, xmltype *data,
ArrayType *namespaces,
if (ctxt == NULL || xmlerrcxt->err_occurred)
xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY,
"could not allocate parser
context");
- doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, NULL,
0);
+ doc = xmlCtxtReadMemory(ctxt, (char *) string + xmldecl_len,
+ len -
xmldecl_len, NULL, NULL, 0);
if (doc == NULL || xmlerrcxt->err_occurred)
xml_ereport(xmlerrcxt, ERROR,
ERRCODE_INVALID_XML_DOCUMENT,
"could not parse XML document");
diff --git a/src/test/regress/expected/xml.out
b/src/test/regress/expected/xml.out
index bcc585d..f7a8c38 100644
--- a/src/test/regress/expected/xml.out
+++ b/src/test/regress/expected/xml.out
@@ -670,6 +670,37 @@ SELECT xpath('/nosuchtag', '');
{}
(1 row)
+-- Round-trip non-ASCII data through xpath().
+DO $$
+DECLARE
+ xml_declaration text := '';
+ degree_symbol text;
+ res xml[];
+BEGIN
+ -- Per the documentation, xpath() doesn't work on non-ASCII data when
+ -- the server encoding is not UTF8. The EXCEPTION block below,
+ -- currently dead code, will be relevant if we remove this limitation.
+ IF current_setting('server_encoding') <> 'UTF8' THEN
+RAISE LOG 'skip: encoding % unsupported for xml',
+ current_setting('server_encoding');
+RETURN;
+ END IF;
+
+ degree_symbol := convert_from('\xc2b0', 'UTF8');
+ res := xpath('text()', (xml_declaration ||
+'' || degree_symbol || '')::xml);
+ IF degree_symbol <> res[1]::text THEN
+RAISE 'expected % (%), got % (%)',
+ degree_symbol, convert_to(degree_symbol, 'UTF8'),
+ res[1], convert_to(res[1]::text, 'UTF8');
+ END IF;
+EXCEPTION
+ -- character with byte sequence 0xc2 0xb0 in encoding "UTF8" has no
equivalent in encoding "LATIN8"
+ WHEN untranslatable_character THEN RAISE LOG 'skip: %', SQLERRM;
+ -- default conversion function for encoding "UTF8" to "MULE_INTERNAL" does
not exist
+ WHEN undefined_function THEN RAISE LOG 'skip: %', SQLERRM;
+END
+$$;
-- Test xmlexists and xpath_exists
SELECT xmlexists('//town[text() = ''Toronto'']' PASSING BY REF
'Bidford-on-AvonCwmbranBristol');
xmlexists
diff --git a/src/test/regress/expected/xml_1.out
b/src/test/regress/expected/xml_1.out
index d3bd8c9..1a9efa2 100644
--- a/src/test/regress/expected/xml_1.out
+++ b/src/test/regress/expected/xml_1.out
@@ -576,6 +576,41 @@ LINE 1: SELECT xpath('/nosuchtag', '');
^
DETAIL: This functionality requires the server to be built with libxml
support.
HINT: You need to rebuild PostgreSQL using --with-libxml.
+-- Round-trip non-ASCII data through xpath().
+DO $$
+DECLARE
+ xml_declaration text := '';
+ degree_symbol text;
+ res xml[];
+BEGIN
+ -- Per the documentation, xpath() doesn't work on non-ASCII data when
+ -- the server encoding is not UTF8. The EXCEPTION block below,
+ -- currently dead code, will be relevant if we remove this limitation.
+ IF current_setting('server_encoding') <> 'UTF8' THEN
+RAISE LOG 'skip: encoding % unsupported for xml',
+ current_setting('s
[HACKERS] Re: [COMMITTERS] pgsql: Account for catalog snapshot in PGXACT->xmin updates.
On Mon, Aug 21, 2017 at 07:41:52AM +0100, Simon Riggs wrote:
> On 19 August 2017 at 20:54, Noah Misch wrote:
> > On Tue, Dec 06, 2016 at 01:59:18PM -0500, Robert Haas wrote:
> >> On Tue, Dec 6, 2016 at 1:36 PM, Tom Lane wrote:
> >> > Robert Haas writes:
> >> >> On Tue, Nov 15, 2016 at 3:55 PM, Tom Lane wrote:
> >> >>> Account for catalog snapshot in PGXACT->xmin updates.
> >> >
> >> >> It seems to me that this makes it possible for
> >> >> ThereAreNoPriorRegisteredSnapshots() to fail spuriously. The only
> >> >> core code that calls that function is in copy.c, and apparently we
> >> >> never reach that point with the catalog snapshot set. But that seems
> >> >> fragile.
> >
> > I recently noticed this by way of the copy2 regression test failing, in 9.4
> > and later, under REPEATABLE READ and SERIALIZABLE. That failure started
> > with
> > $SUBJECT.
> > Fair summary. ThereAreNoPriorRegisteredSnapshots() has functioned that way
> > since an early submission[1], and I don't see that we ever discussed it
> > explicitly. Adding Simon for his recollection.
>
> The intention, IIRC, was to allow the current snapshot (i.e. 1) but no
> earlier (i.e. prior) snapshots (so not >=2)
>
> The name was chosen so it was (hopefully) clear what it did --
> ThereAreNoPriorRegisteredSnapshots
I now understand the function name, so I added a comment but didn't rename it.
I plan to use the attached patch after the minor release tags land. If
there's significant support, I could instead push before the wrap.
commit d68eed7 (HEAD, ThereAreNoPriorRegisteredSnapshots-CatalogSnapshot)
Author: Noah Misch
AuthorDate: Sat Nov 4 00:08:04 2017 -0700
Commit: Noah Misch
CommitDate: Sat Nov 4 00:33:37 2017 -0700
Ignore CatalogSnapshot when checking COPY FREEZE prerequisites.
This restores the ability, essentially lost in commit
ffaa44cb559db332baeee7d25dedd74a61974203, to use COPY FREEZE under
REPEATABLE READ isolation. Back-patch to 9.4, like that commit.
Discussion:
https://postgr.es/m/ca+tgmoahwdm-7fperbxzu9uz99lpmumepsxltw9tmrogzwn...@mail.gmail.com
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index 8006df3..1bdd492 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2394,13 +2394,25 @@ CopyFrom(CopyState cstate)
/*
* Optimize if new relfilenode was created in this subxact or one of its
* committed children and we won't see those rows later as part of an
-* earlier scan or command. This ensures that if this subtransaction
-* aborts then the frozen rows won't be visible after xact cleanup. Note
+* earlier scan or command. The subxact test ensures that if this
subxact
+* aborts then the frozen rows won't be visible after xact cleanup.
Note
* that the stronger test of exactly which subtransaction created it is
-* crucial for correctness of this optimization.
+* crucial for correctness of this optimization. The test for an earlier
+* scan or command tolerates false negatives. FREEZE causes other
sessions
+* to see rows they would not see under MVCC, and a false negative
merely
+* spreads that anomaly to the current session.
*/
if (cstate->freeze)
{
+ /*
+* Tolerate one registration for the benefit of
FirstXactSnapshot.
+* Scan-bearing queries generally create at least two
registrations,
+* though relying on that is fragile, as is ignoring
ActiveSnapshot.
+* Clear CatalogSnapshot to avoid counting its registration.
We'll
+* still detect ongoing catalog scans, each of which separately
+* registers the snapshot it uses.
+*/
+ InvalidateCatalogSnapshot();
if (!ThereAreNoPriorRegisteredSnapshots() ||
!ThereAreNoReadyPortals())
ereport(ERROR,
(errcode(ERRCODE_INVALID_TRANSACTION_STATE),
diff --git a/src/backend/utils/time/snapmgr.c b/src/backend/utils/time/snapmgr.c
index 294ab70..addf87d 100644
--- a/src/backend/utils/time/snapmgr.c
+++ b/src/backend/utils/time/snapmgr.c
@@ -1645,6 +1645,14 @@ DeleteAllExportedSnapshotFiles(void)
FreeDir(s_dir);
}
+/*
+ * ThereAreNoPriorRegisteredSnapshots
+ * Is the registered snapshot count less than or equal to one?
+ *
+ * Don't use this to settle important decisions. While zero registrations and
+ * no ActiveSnapshot would confirm a certain idleness, the system makes no
+ * guarantees about the significance of one registered snapshot.
+ */
bool
ThereAreNoPriorRegisteredSnapshots(void)
{
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: heap/SLRU verification, relfrozenxid cut-off, and freeze-the-dead bug (Was: amcheck (B-Tree integrity checking tool))
On Mon, Oct 16, 2017 at 12:57:39PM -0700, Peter Geoghegan wrote: > On Fri, Oct 13, 2017 at 7:09 PM, Noah Misch wrote: > > The checker should > > consider circumstances potentially carried from past versions via > > pg_upgrade. > > Right. False positives are simply unacceptable. False positives are bugs, but they're not exceptionally-odious bugs. > > Fortunately, if you get some details wrong, it's cheap to recover from > > checker > > bugs. > > Ideally, amcheck will become a formal statement of the contracts > provided by major subsystems, such as the heapam, the various SLRUs, > and so on. While I agree that having bugs there is much less severe > than having bugs in backend code, I would like the tool to reach a > point where it actually *defines* correctness (by community > consensus). That presupposes construction of two pieces of software, the server and the checker, such that every disagreement is a bug in the server. But checkers get bugs just like servers get bugs. Checkers do provide a sort of double-entry bookkeeping. When a reproducible test case prompts a checker complaint, we'll know *some* code is wrong. That's an admirable contribution. > If a bug in amcheck reflects a bug in our high level > thinking about correctness, then that actually is a serious problem. My notion of data file correctness is roughly this: A data file is correct if the server's reads and mutations thereof will not cause it to deviate from documented behavior. Where the documentation isn't specific, fall back on SQL standards. Where no documentation or SQL standard addresses a particular behavior, we should debate the matter and document the decision. I'm essentially saying that the server is innocent until proven guilty. It would be cool to have a self-contained specification of PostgreSQL data files, but where the server disagrees with the spec without causing problem behaviors, we'd ultimately update the spec to fit the server. Thanks, 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] possible encoding issues with libxml2 functions
On Sun, Aug 20, 2017 at 10:37:10PM +0200, Pavel Stehule wrote: > > We have xpath-bugfix.patch and xpath-parsing-error-fix.patch. Both are > > equivalent under supported use cases (xpath in UTF8 databases). Among > > non-supported use cases, they each make different things better and > > different > > things worse. We should prefer to back-patch the version harming fewer > > applications. I expect non-ASCII data is more common than xml declarations > > with "encoding" attribute, so xpath-bugfix.patch will harm fewer > > applications. > > > > Having said that, I now see a third option. Condition this thread's > > patch's > > effects on GetDatabaseEncoding()==PG_UTF8. That way, we fix supported > > cases, > > and we remain bug-compatible in unsupported cases. I think that's better > > than > > the other options discussed so far. If you agree, please send a patch > > based > > on xpath-bugfix.patch with the GetDatabaseEncoding()==PG_UTF8 change and > > the > > two edits I described earlier. > > > > I am sorry - too long day today. Do you think some like > > diff --git a/src/backend/utils/adt/xml.c b/src/backend/utils/adt/xml.c > index 24229c2dff..9fd6f3509f 100644 > --- a/src/backend/utils/adt/xml.c > +++ b/src/backend/utils/adt/xml.c > @@ -3914,7 +3914,14 @@ xpath_internal(text *xpath_expr_text, xmltype *data, > ArrayType *namespaces, > if (ctxt == NULL || xmlerrcxt->err_occurred) > xml_ereport(xmlerrcxt, ERROR, ERRCODE_OUT_OF_MEMORY, > "could not allocate parser context"); > - doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, NULL, 0); > + > + /* > +* Passed XML is always in server encoding. When server encoding > +* is UTF8, we can pass this information to libxml2 to ignore > +* possible invalid encoding declaration in XML document. > +*/ > + doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, > + GetDatabaseEncoding() == PG_UTF8 ? "UTF-8" : NULL, 0); > if (doc == NULL || xmlerrcxt->err_occurred) > xml_ereport(xmlerrcxt, ERROR, ERRCODE_INVALID_XML_DOCUMENT, > "could not parse XML document"); No, that doesn't match my description above. I don't see a way to clarify the description. Feel free to try again. Alternately, if you wait, I will eventually construct the patch I described. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: heap/SLRU verification, relfrozenxid cut-off, and freeze-the-dead bug (Was: amcheck (B-Tree integrity checking tool))
On Mon, Oct 09, 2017 at 05:19:11PM -0700, Peter Geoghegan wrote: > On Sun, Oct 16, 2016 at 6:46 PM, Noah Misch wrote: > > - Verify agreement between CLOG, MULTIXACT, and hint bits. > > This is where it gets complicated, I think. This is what I really want > to talk about. > > The recent commit 20b65522 pretty clearly established that a tuple > could technically escape freezing (having HEAP_XMIN_FROZEN set) > despite actually being before a relfrozenxid cut-off. The idea was > that as long as we reliably set hint bits on heap-only tuples through > WAL-logging, it doesn't matter that their CLOG could be truncated, > because nobody will ever need to interrogate the CLOG anyway (to coin > a phrase, the heap-only tuple nevertheless still had its xmax > "logically frozen"). If nothing else, this leaves me with a very > squishy set of invariant conditions to work with when I go to verify > agreement with CLOG, MULTIXACT, and hint bits. > > So, the question is: What is the exact set of invariant conditions > that I can check when I go to verify agreement between a heap relation > and the various SLRUs? In particular, at what precise XID-wise point > do CLOG and MULTIXACT stop reliably storing transaction status? And, > is there a different point for the xmax of tuples that happen to be > heap-only tuples? > > Another important concern here following 20b65522 is: Why is it safe > to assume that nobody will ever call TransactionIdDidCommit() for > "logically frozen" heap-only tuples that are not at the end of their > HOT chain, and in so doing get a wrong answer? I can't find a rule > that implies that there is no dangerous ambiguity that's written down > anywhere. I *can* find a comment that suggests that it's wrong, though > -- the "N.B." comment at the top of heap_prepare_freeze_tuple(). > (Granted, that comment doesn't seem to acknowledge the fact that the > caller does WAL-log as part of freezing; there has been some churn in > this area.) All good questions; I don't know offhand. Discovering those answers is perhaps the chief labor required of such a project. The checker should consider circumstances potentially carried from past versions via pg_upgrade. Fortunately, if you get some details wrong, it's cheap to recover from checker bugs. -- 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] Still another race condition in recovery TAP tests
On Fri, Oct 06, 2017 at 05:57:24PM +0800, Craig Ringer wrote: > On 6 October 2017 at 14:03, Noah Misch wrote: > > On Fri, Sep 08, 2017 at 10:32:03PM -0400, Tom Lane wrote: > >> (I do kinda wonder why we rolled our own RecursiveCopy; surely there's > >> a better implementation in CPAN?) > > > > Fewer people will test as we grow the list of modules they must first > > install. > > Meh, I don't buy that. At worst, all we have to do is provide a script > that fetches them, from distro repos if possible, and failing that > from CPAN. > > With cpanminus, that's pretty darn simple too. If the tree had such a script and it were reliable, then yes, it would matter little whether the script procured one module or five. -- 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] pgsql: Remove ICU tests from default run
On Sat, Mar 25, 2017 at 04:30:45AM +, Peter Eisentraut wrote: > Remove ICU tests from default run > > These tests require the test database to be in UTF8 encoding. Until > there is a better solution, take them out of the default test set and > treat them like the existing collate.linux.utf8 test, meaning it has to > be selected manually. I'm thinking the regression suite should create an ENCODING=UTF8, LOCALE=C database in addition to the traditional one. This isn't the first or the second use case for such a database. -- 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] Still another race condition in recovery TAP tests
On Fri, Sep 08, 2017 at 10:32:03PM -0400, Tom Lane wrote: > (I do kinda wonder why we rolled our own RecursiveCopy; surely there's > a better implementation in CPAN?) Fewer people will test as we grow the list of modules they must first install. Bundling a copy is tempting, but most CPAN modules use the Perl license. We're in that hole already[1], but I perceived a consensus to stop digging. Code used only for testing is less concerning, but I'd still not bundle it. [1] https://www.postgresql.org/message-id/flat/54EC6D80.5090507%40vmware.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] shm_mq_set_sender() crash
On Thu, Sep 15, 2016 at 06:21:30PM -0400, Robert Haas wrote: > On Thu, Sep 15, 2016 at 5:22 PM, Tom Lane wrote: > > Robert Haas writes: > >> Of course, it's also possible that the ParallelWorkerNumber code is > >> entirely correct and something overwrote the null bytes that were > >> supposed to be found at that location. It would be very useful to see > >> (a) the value of ParallelWorkerNumber and (b) the contents of > >> vmq->mq_sender, and in particular whether that's actually a valid > >> pointer to a PGPROC in the ProcArray. But unless we can reproduce > >> this I don't see how to manage that. > > > > Is it worth replacing that Assert with a test-and-elog that would > > print those values? > > > > Given that we've seen only one such instance in the buildfarm, this > > might've been just a cosmic ray bit-flip. So one part of me says > > not to worry too much until we see it again. OTOH, if it is real > > but rare, missing an opportunity to diagnose would be bad. > > I wonder if we could persuade somebody to run pgbench on a Windows > machine with a similar environment, at least some concurrency, and > force_parallel_mode=on. Assuming this is a generic > initialize-the-parallel-stuff bug and not something specific to a > particular query, that might well trigger it a lot quicker than > waiting for it to recur in the BF. For the sake of this thread in the archives, the cause was almost surely a Cygwin signal handling bug: https://postgr.es/m/20170803034740.ga2641...@rfd.leadboat.com https://marc.info/?t=15018329641 https://marc.info/?t=150291861700011 -- 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: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?
On Sun, Oct 01, 2017 at 09:56:11AM -0400, Peter Eisentraut wrote: > On 9/30/17 03:01, Noah Misch wrote: > > This PostgreSQL 10 open item is past due for your status update. On the > > worst > > week to be violating open item policies. Kindly send a status update within > > 24 hours, and include a date for your subsequent status update. Refer to > > the > > policy on open item ownership: > > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > I had previously replied that I think nothing should be changed. What > process should be applied in that case? If you determine community decision-making is not against that conclusion, edit the list to move the item from "Open Issues" to "Non-bugs". -- 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: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?
On Sat, Sep 30, 2017 at 11:25:43AM -0400, Tom Lane wrote: > Noah Misch writes: > > On Mon, Sep 25, 2017 at 09:36:44AM -0700, Peter Geoghegan wrote: > >>> I think it's inevitable that a certain number of users are going to > >>> have to cope with ICU version changes breaking stuff. > > >> Wasn't the main point of adopting ICU that that doesn't happen when it > >> isn't essential? > > > I wouldn't describe it that way. I agree that few, if any, ICU upgrades > > will > > remove country or language codes. Overall, though, almost every ICU upgrade > > will be difficult. Each ICU release, even a minor release like 58.2, > > changes > > the sorting rules in some tiny way. You then see "Rebuild all objects > > affected by this collation" messages. > > Sure, but dealing with that is mechanical: reindex the necessary indexes > and you're done. In the general case, one must revalidate CHECK constraints, re-partition tables, revalidate range values, and reindex. > In the libc world, > when you upgrade libc's locale definitions, you have no idea what the > consequences are. Yep. It's strictly better than the libc case. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?
On Mon, Sep 25, 2017 at 09:36:44AM -0700, Peter Geoghegan wrote: > On Mon, Sep 25, 2017 at 9:06 AM, Robert Haas wrote: > >> The big concern I have here is that this feels a lot like something that > >> we'll regret at leisure, if it's not right in the first release. I'd > >> much rather be restrictive in v10 and then loosen the rules later, than > >> be lax in v10 and then have to argue about whether to break backwards > >> compatibility in order to gain saner behavior. > > > > I think it's inevitable that a certain number of users are going to > > have to cope with ICU version changes breaking stuff. > > Wasn't the main point of adopting ICU that that doesn't happen when it > isn't essential? There will be some risk with pg_dump across ICU > versions, due rare to political changes. But, on pg_upgrade, the old > collations will continue to work, even if they would not have appeared > at initdb time on that ICU version, because ICU has all kinds of > fallbacks. I wouldn't describe it that way. I agree that few, if any, ICU upgrades will remove country or language codes. Overall, though, almost every ICU upgrade will be difficult. Each ICU release, even a minor release like 58.2, changes the sorting rules in some tiny way. You then see "Rebuild all objects affected by this collation" messages. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?
On Mon, Sep 25, 2017 at 08:26:21AM +, Noah Misch wrote: > On Tue, Sep 19, 2017 at 07:01:47PM -0700, Peter Geoghegan wrote: > > On Tue, Sep 19, 2017 at 5:52 PM, Peter Eisentraut > > wrote: > > > On 9/18/17 18:46, Peter Geoghegan wrote: > > >> As I pointed out a couple of times already [1], we don't currently > > >> sanitize ICU's BCP 47 language tags within CREATE COLLATION. > > > > > > There is no requirement that the locale strings for ICU need to be BCP > > > 47. ICU locale names like 'de@collation=phonebook' are also acceptable. > > > > Right. But, we only document that BCP 47 is supported by Postgres. > > Maybe we can use get_icu_language_tag()/uloc_toLanguageTag() to ensure > > that we end up with BCP 47, even when the user happens to specify the > > legacy syntax. Should we be "canonicalizing" legacy ICU locale strings > > as BCP 47, too? > > > > > The reason they are not validated is that, as you know, ICU accepts any > > > locale string as valid. You appear to have found a way to do some > > > validation, but I would like to see that code. > > > > As I mentioned, I'm simply calling > > get_icu_language_tag()/uloc_toLanguageTag() to do that sanitization. > > The code to get the extra sanitization is completely trivial. > > > > I didn't post the patch that generates the errors in my opening e-mail > > because I'm not confident it's correct just yet. And, I think that I > > see a bigger problem: we pass a string that is almost certainly a BCP > > 47 string to ucol_open() from within pg_newlocale_from_collation(). We > > do so despite the fact that ucol_open() apparently doesn't accept BCP > > 47 syntax locale strings until ICU 54 [1]. Seems entirely possible > > that this accounts for the problems you saw on ICU 4.2, back when we > > were still creating keyword variants (I guess that the keyword > > variants seem very "BCP 47-ish" to me). > > > > I really think we need to add some kind of debug mode that makes ICU > > optionally spit out a locale display name at key points. We need this > > to gain confidence that the behavior that ICU provides actually > > matches what is expected across ICU different versions for different > > locale formats. We talked about this as a user-facing feature before, > > which can wait till v11; I just want this to debug problems like this > > one. > > > > [1] > > https://ssl.icu-project.org/apiref/icu4c/ucol_8h.html#a4721e4c0a519bb0139a874e191223590 > > [Action required within three days. This is a generic notification.] > > The above-described topic is currently a PostgreSQL 10 open item. Peter, > since you committed the patch believed to have created it, you own this open > item. If some other commit is more relevant or if this does not belong as a > v10 open item, please let us know. Otherwise, please observe the policy on > open item ownership[1] and send a status update within three calendar days of > this message. Include a date for your subsequent status update. Testers may > discover new open items at any time, and I want to plan to get them all fixed > well in advance of shipping v10. Consequently, I will appreciate your efforts > toward speedy resolution. Thanks. > > [1] > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com This PostgreSQL 10 open item is past due for your status update. On the worst week to be violating open item policies. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Shaky coding for vacuuming partitioned relations
On Fri, Sep 22, 2017 at 03:13:10PM -0400, Tom Lane wrote: > Somebody inserted this into vacuum.c's get_rel_oids(): > > tuple = SearchSysCache1(RELOID, ObjectIdGetDatum(relid)); > if (!HeapTupleIsValid(tuple)) > elog(ERROR, "cache lookup failed for relation %u", relid); > > apparently without having read the very verbose comment two lines above, > which points out that we're not taking any lock on the target relation. > So, if that relation is concurrently being dropped, you're likely to > get "cache lookup failed for relation " rather than anything more > user-friendly. > > A minimum-change fix would be to replace the elog() with an ereport > that produces the same "relation does not exist" error you'd have > gotten from RangeVarGetRelid, had the concurrent DROP TABLE committed > a few microseconds earlier. But that feels like its's band-aiding > around the problem. > > What I'm wondering about is changing the RangeVarGetRelid call to take > ShareUpdateExclusiveLock rather than no lock. That would protect the > syscache lookup, and it would also make the find_all_inheritors call > a lot more meaningful. > > If we're doing a VACUUM, the ShareUpdateExclusiveLock would be dropped > as soon as we close the caller's transaction, and then we'd acquire > the same or stronger lock inside vacuum_rel(). So that seems fine. > If we're doing an ANALYZE, then the lock would continue to be held > and analyze_rel would merely be acquiring it an extra time, so we'd > actually be removing a race-condition failure scenario for ANALYZE. > This would mean a few more cycles in lock management, but since this > only applies to a manual VACUUM or ANALYZE that specifies a table > name, I'm not too concerned about that. > > Thoughts? This thread now has two open items, both of them pertaining to VACUUM error messages involving partitioning. The pair is probably best treated as a single open item. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Robert, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] pgbench - minor fix for meta command only scripts
On Tue, Sep 12, 2017 at 03:27:13AM +0200, Fabien COELHO wrote: > >Shouldn't we use pg_usleep to ensure portability? it is defined for > >front-end code. But it returns void, so the error check will have to be > >changed. > > Attached v3 with pg_usleep called instead. > > >I didn't see the problem before the commit I originally indicated , so I > >don't think it has to be back-patched to before v10. > > Hmmm you've got a point, although I'm not sure how it could work without > sleeping explicitely. Maybe the path was calling select with an empty wait > list plus timeout, and select is kind enough to just sleep on an empty list, > or some other miracle. ISTM clearer to explicitely sleep in that case. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Heikki, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: CREATE COLLATION does not sanitize ICU's BCP 47 language tags. Should it?
On Tue, Sep 19, 2017 at 07:01:47PM -0700, Peter Geoghegan wrote: > On Tue, Sep 19, 2017 at 5:52 PM, Peter Eisentraut > wrote: > > On 9/18/17 18:46, Peter Geoghegan wrote: > >> As I pointed out a couple of times already [1], we don't currently > >> sanitize ICU's BCP 47 language tags within CREATE COLLATION. > > > > There is no requirement that the locale strings for ICU need to be BCP > > 47. ICU locale names like 'de@collation=phonebook' are also acceptable. > > Right. But, we only document that BCP 47 is supported by Postgres. > Maybe we can use get_icu_language_tag()/uloc_toLanguageTag() to ensure > that we end up with BCP 47, even when the user happens to specify the > legacy syntax. Should we be "canonicalizing" legacy ICU locale strings > as BCP 47, too? > > > The reason they are not validated is that, as you know, ICU accepts any > > locale string as valid. You appear to have found a way to do some > > validation, but I would like to see that code. > > As I mentioned, I'm simply calling > get_icu_language_tag()/uloc_toLanguageTag() to do that sanitization. > The code to get the extra sanitization is completely trivial. > > I didn't post the patch that generates the errors in my opening e-mail > because I'm not confident it's correct just yet. And, I think that I > see a bigger problem: we pass a string that is almost certainly a BCP > 47 string to ucol_open() from within pg_newlocale_from_collation(). We > do so despite the fact that ucol_open() apparently doesn't accept BCP > 47 syntax locale strings until ICU 54 [1]. Seems entirely possible > that this accounts for the problems you saw on ICU 4.2, back when we > were still creating keyword variants (I guess that the keyword > variants seem very "BCP 47-ish" to me). > > I really think we need to add some kind of debug mode that makes ICU > optionally spit out a locale display name at key points. We need this > to gain confidence that the behavior that ICU provides actually > matches what is expected across ICU different versions for different > locale formats. We talked about this as a user-facing feature before, > which can wait till v11; I just want this to debug problems like this > one. > > [1] > https://ssl.icu-project.org/apiref/icu4c/ucol_8h.html#a4721e4c0a519bb0139a874e191223590 [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Peter, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: "inconsistent page found" with checksum and wal_consistency_checking enabled
On Wed, Sep 20, 2017 at 01:52:15PM +0900, Michael Paquier wrote: > On Wed, Sep 20, 2017 at 5:23 AM, Ashwin Agrawal wrote: > > Currently, page checksum is not masked by Page masking routines used by > > wal_consistency_checking facility. So, when running `make installcheck` with > > data checksum enabled and wal_consistency_checking='all', it easily and > > consistently FATALs with "inconsistent page found". > > Indeed. This had better be fixed before PG10 is out. I am adding an open item. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Robert, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] !USE_WIDE_UPPER_LOWER compile errors in v10+
On Thu, Sep 21, 2017 at 05:38:13PM -0400, Tom Lane wrote: > I wrote: > > Noah Misch writes: > >> Perhaps it is time to require HAVE_WCSTOMBS and HAVE_TOWLOWER, removing > >> USE_WIDE_UPPER_LOWER? Every buildfarm fossil has both. > > > +1 ... if nothing else, there's the problem that untested code is likely > > to be broken. You just proved it *is* broken, of course, but my point > > is that even if we repaired the immediate damage we could have little > > confidence in it staying fixed. That would be easy enough to ensure by adding ac_cv_func_towlower=no to some buildfarm animal. But the real-world need for said code is clearly dead and gone. Good riddance. > Meanwhile, I see that Peter has posted a fix for the immediate problem. > I propose that Peter should apply his fix in HEAD and v10, and then > I'll rip out the !USE_WIDE_UPPER_LOWER code paths in HEAD only. That works for me. -- 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] ICU locales and text/char(n) SortSupport on Windows
On Sat, Sep 16, 2017 at 03:33:53PM -0700, Peter Geoghegan wrote: > In summary, we're currently attaching the use of SortSupport to the > wrong thing. We're treating this UTF-16 business as something that > implies a broad OS/platform restriction, when in fact it should be > treated as implying a restriction for one particular collation > provider only (a collation provider that happens to be built into > Windows, but isn't really special to us). > > Attached patch shows what I'm getting at. This is untested, since I > don't use Windows. Proceed with caution. This is currently a v10 open item, but I think it doesn't qualify for that treatment. It's merely an opportunity for optimization, albeit an attractively-simple one. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] !USE_WIDE_UPPER_LOWER compile errors in v10+
On Sat, Sep 16, 2017 at 03:33:53PM -0700, Peter Geoghegan wrote: > On a related note, am I the only one that finds it questionable that > str_tolower() has an "#ifdef USE_WIDE_UPPER_LOWER" block that itself > contains an "#ifdef USE_ICU" block? It seems like those two things > might get conflated on some platforms. We don't want lower() to ever > not use the ICU infrastructure when an ICU collation is used, and yet > it's not obvious that that's impossible. I understand that the code in > regc_pg_locale.c kind of insists on using USE_WIDE_UPPER_LOWER > facilities, and that that was always accepted as legacy that ICU had > to live with. Maybe a static assertion is all that we need here (ICU > builds must also be USE_WIDE_UPPER_LOWER builds). I checked !USE_WIDE_UPPER_LOWER by configuring v10 as follows: ./configure -C --prefix=$HOME/sw/nopath/pg10 --enable-debug \ --enable-cassert --enable-depend --enable-tap-tests --with-libxml \ --with-gssapi --with-openssl ac_cv_func_towlower=no The result fails to compile: $ make formatting.c: In function ‘str_tolower’: formatting.c:1623:10: error: ‘mylocale’ undeclared (first use in this function) if (mylocale) ^ ... snipped other errors ... A --with-icu build fails similarly. PG9.6 builds and passes "make check". Perhaps it is time to require HAVE_WCSTOMBS and HAVE_TOWLOWER, removing USE_WIDE_UPPER_LOWER? Every buildfarm fossil has both. Solaris 2.5.1 (out of support for 12 years) might be the most interesting OS affected: https://www.gnu.org/software/gnulib/manual/html_node/towlower.html https://www.gnu.org/software/gnulib/manual/html_node/wcstombs.html The above-described topic is currently a PostgreSQL 10 open item. Peter (Eisentraut), since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Back-branch release notes up for review
On Thu, Aug 31, 2017 at 02:53:45AM +, Noah Misch wrote:
> On Sat, Aug 26, 2017 at 03:31:12PM -0400, Tom Lane wrote:
> > +
> > +
> > +
> > + Show foreign tables
> > + in information_schema.table_privileges
> > + view (Peter Eisentraut)
> > +
> > +
> > +
> > + All other relevant information_schema views include
> > + foreign tables, but this one ignored them.
> > +
> > +
> > +
> > + Since this view definition is installed by initdb,
> > + merely upgrading will not fix the problem. If you need to fix this
> > + in an existing installation, you can, as a superuser, do this
> > + in psql:
> > +
> > +BEGIN;
> > +DROP SCHEMA information_schema CASCADE;
> > +\i SHAREDIR/information_schema.sql
> > +COMMIT;
> > +
> > + (Run pg_config --sharedir if you're uncertain
> > + where SHAREDIR is.) This must be repeated in each
> > + database to be fixed.
> > +
> > +
>
> "DROP SCHEMA information_schema CASCADE;" will drop objects outside
> information_schema that depend on objects inside information_schema. For
> example, this will drop user-defined views if the view query refers to
> information_schema. That's improper in a release-noted update procedure.
> This could instead offer a CREATE OR REPLACE VIEW or just hand-wave about the
> repaired definition being available in information_schema.sql.
I lean toward the former, attached. Conveniently, every released branch has
the same definition for this view.
diff --git a/doc/src/sgml/release-9.2.sgml b/doc/src/sgml/release-9.2.sgml
index faa7ae4..6fa21e3 100644
--- a/doc/src/sgml/release-9.2.sgml
+++ b/doc/src/sgml/release-9.2.sgml
@@ -58,14 +58,44 @@
in an existing installation, you can, as a superuser, do this
in psql:
-BEGIN;
-DROP SCHEMA information_schema CASCADE;
-\i SHAREDIR/information_schema.sql
-COMMIT;
+SET search_path TO information_schema;
+CREATE OR REPLACE VIEW table_privileges AS
+SELECT CAST(u_grantor.rolname AS sql_identifier) AS grantor,
+ CAST(grantee.rolname AS sql_identifier) AS grantee,
+ CAST(current_database() AS sql_identifier) AS table_catalog,
+ CAST(nc.nspname AS sql_identifier) AS table_schema,
+ CAST(c.relname AS sql_identifier) AS table_name,
+ CAST(c.prtype AS character_data) AS privilege_type,
+ CAST(
+ CASE WHEN
+ -- object owner always has grant options
+ pg_has_role(grantee.oid, c.relowner, 'USAGE')
+ OR c.grantable
+ THEN 'YES' ELSE 'NO' END AS yes_or_no) AS is_grantable,
+ CAST(CASE WHEN c.prtype = 'SELECT' THEN 'YES' ELSE 'NO' END AS
yes_or_no) AS with_hierarchy
+
+FROM (
+SELECT oid, relname, relnamespace, relkind, relowner,
(aclexplode(coalesce(relacl, acldefault('r', relowner.* FROM pg_class
+ ) AS c (oid, relname, relnamespace, relkind, relowner, grantor,
grantee, prtype, grantable),
+ pg_namespace nc,
+ pg_authid u_grantor,
+ (
+ SELECT oid, rolname FROM pg_authid
+ UNION ALL
+ SELECT 0::oid, 'PUBLIC'
+ ) AS grantee (oid, rolname)
+
+WHERE c.relnamespace = nc.oid
+ AND c.relkind IN ('r', 'v', 'f')
+ AND c.grantee = grantee.oid
+ AND c.grantor = u_grantor.oid
+ AND c.prtype IN ('INSERT', 'SELECT', 'UPDATE', 'DELETE', 'TRUNCATE',
'REFERENCES', 'TRIGGER')
+ AND (pg_has_role(u_grantor.oid, 'USAGE')
+ OR pg_has_role(grantee.oid, 'USAGE')
+ OR grantee.rolname = 'PUBLIC');
- (Run pg_config --sharedir if you're uncertain
- where SHAREDIR is.) This must be repeated in each
- database to be fixed.
+ This must be repeated in each database to be fixed,
+ including template0.
diff --git a/doc/src/sgml/release-9.3.sgml b/doc/src/sgml/release-9.3.sgml
index f3b00a7..91fbb34 100644
--- a/doc/src/sgml/release-9.3.sgml
+++ b/doc/src/sgml/release-9.3.sgml
@@ -52,14 +52,44 @@
in an existing installation, you can, as a superuser, do this
in psql:
-BEGIN;
-DROP SCHEMA information_schema CASCADE;
-\i SHAREDIR/information_schema.sql
-COMMIT;
+SET search_path TO information_schema;
+CREATE OR REPLACE VIEW table_privileges AS
+SELECT CAST(u_grantor.rolname AS sql_identifier) AS grantor,
+ CAST(grantee.rolname AS sql_identifier) AS grantee,
+ CAST(current_database() AS sql_identifier) AS table_catalog,
+
Re: [HACKERS] SCRAM in the PG 10 release notes
On Thu, Sep 14, 2017 at 09:57:36AM +0300, Heikki Linnakangas wrote: > On 09/12/2017 04:09 AM, Noah Misch wrote: > >On Wed, May 10, 2017 at 10:50:51PM -0400, Bruce Momjian wrote: > >>On Mon, May 1, 2017 at 08:12:51AM -0400, Robert Haas wrote: > >>>On Tue, Apr 25, 2017 at 10:16 PM, Bruce Momjian wrote: > >>>>Well, we could add "MD5 users are encouraged to switch to > >>>>SCRAM-SHA-256". Now whether we want to list this as something on the > >>>>SCRAM-SHA-256 description, or mention it as an incompatibility, or > >>>>under Migration. I am not clear that MD5 is in such terrible shape that > >>>>this is warranted. > >>> > >>>I think it's warranted. The continuing use of MD5 has been a headache > >>>for some EnterpriseDB customers who have compliance requirements which > >>>they must meet. It's not that they themselves necessarily know or > >>>care whether MD5 is secure, although in some cases they do; it's that > >>>if they use it, they will be breaking laws or regulations to which > >>>their business or agency is subject. I imagine customers of other > >>>PostgreSQL companies have similar issues. But leaving that aside, the > >>>advantage of SCRAM isn't merely that it uses a better algorithm to > >>>hash the password. It has other advantages also, like not being > >>>vulnerable to replay attacks. If you're doing password > >>>authentication, you should really be using SCRAM, and encouraging > >>>people to move to SCRAM after upgrading is a good idea. > >>> > >>>That having been said, SCRAM is a wire protocol break. You will not > >>>be able to upgrade to SCRAM unless and until the drivers you use to > >>>connect to the database add support for it. The only such driver > >>>that's part of libpq; other drivers that have reimplemented the > >>>PostgreSQL wire protocol will have to be updated with SCRAM support > >>>before it will be possible to use SCRAM with those drivers. I think > >>>this should be mentioned in the release notes, too. I also think it > >>>would be great if somebody would put together a wiki page listing all > >>>the popular drivers and (1) whether they use libpq or reimplement the > >>>wire protocol, and (2) if the latter, the status of any efforts to > >>>implement SCRAM, and (3) if those efforts have been completed, the > >>>version from which they support SCRAM. Then, I think we should reach > >>>out to all of the maintainers of those driver authors who aren't > >>>moving to support SCRAM and encourage them to do so. > >> > >>I have added this as an open item because we will have to wait to see > >>where we are with driver support as the release gets closer. > > > >With the release near, I'm promoting this to the regular open issues section. > > Thanks. > > I updated the list of drivers on the wiki > (https://wiki.postgresql.org/wiki/List_of_drivers), adding a column for > whether the driver supports SCRAM authentication. Currently, the only > non-libpq driver that has implemented SCRAM is the JDBC driver. I submitted > a patch for the Go driver, but it hasn't been committed yet. > > As for a recommendation in the release notes, maybe something like > "Installations using MD5 authentication are encouraged to switch to > SCRAM-SHA-256, unless using older client programs or drivers that don't > support it yet." That sounds reasonable. This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Use MINVALUE/MAXVALUE instead of UNBOUNDED for range partition b
On Wed, Sep 13, 2017 at 12:06:40PM -0400, Robert Haas wrote: > On Wed, Sep 13, 2017 at 10:54 AM, Dean Rasheed > wrote: > > Oracle, MySQL and DB2 all use MINVALUE/MAXVALUE. Actually, Oracle and > > MySQL only use MAXVALUE, not MINVALUE, because they don't allow gaps > > between partitions and the first partition implicitly starts at > > MINVALUE, so the bounds that we currently support are a strict > > superset of those supported by Oracle and MySQL. > > > > Both Oracle and MySQL allow finite values after MAXVALUE (usually > > listed as "0" in code examples, e.g. see [1]). Oracle explicitly > > documents the fact that values after MAXVALUE are irrelevant in [1]. > > I'm not sure if MySQL explicitly documents that, but it does behave > > the same. > > > > Also, both Oracle and MySQL store what the user entered (they do not > > canonicalise), as can be seen by looking at ALL_TAB_PARTITIONS in > > Oracle, or "show create table" in MySQL. > > OK, thanks. I still don't really like allowing this, but I can see > that compatibility with other systems has some value here, and if > nobody else is rejecting these cases, maybe we shouldn't either. So > I'll hold my nose and change my vote to canonicalizing rather than > rejecting outright. I vote for rejecting it. DDL compatibility is less valuable than other compatibility. The hypothetical affected application can change its DDL to placate PostgreSQL and use that modified DDL for all other databases, too. -- 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] SCRAM in the PG 10 release notes
On Wed, May 10, 2017 at 10:50:51PM -0400, Bruce Momjian wrote: > On Mon, May 1, 2017 at 08:12:51AM -0400, Robert Haas wrote: > > On Tue, Apr 25, 2017 at 10:16 PM, Bruce Momjian wrote: > > > Well, we could add "MD5 users are encouraged to switch to > > > SCRAM-SHA-256". Now whether we want to list this as something on the > > > SCRAM-SHA-256 description, or mention it as an incompatibility, or > > > under Migration. I am not clear that MD5 is in such terrible shape that > > > this is warranted. > > > > I think it's warranted. The continuing use of MD5 has been a headache > > for some EnterpriseDB customers who have compliance requirements which > > they must meet. It's not that they themselves necessarily know or > > care whether MD5 is secure, although in some cases they do; it's that > > if they use it, they will be breaking laws or regulations to which > > their business or agency is subject. I imagine customers of other > > PostgreSQL companies have similar issues. But leaving that aside, the > > advantage of SCRAM isn't merely that it uses a better algorithm to > > hash the password. It has other advantages also, like not being > > vulnerable to replay attacks. If you're doing password > > authentication, you should really be using SCRAM, and encouraging > > people to move to SCRAM after upgrading is a good idea. > > > > That having been said, SCRAM is a wire protocol break. You will not > > be able to upgrade to SCRAM unless and until the drivers you use to > > connect to the database add support for it. The only such driver > > that's part of libpq; other drivers that have reimplemented the > > PostgreSQL wire protocol will have to be updated with SCRAM support > > before it will be possible to use SCRAM with those drivers. I think > > this should be mentioned in the release notes, too. I also think it > > would be great if somebody would put together a wiki page listing all > > the popular drivers and (1) whether they use libpq or reimplement the > > wire protocol, and (2) if the latter, the status of any efforts to > > implement SCRAM, and (3) if those efforts have been completed, the > > version from which they support SCRAM. Then, I think we should reach > > out to all of the maintainers of those driver authors who aren't > > moving to support SCRAM and encourage them to do so. > > I have added this as an open item because we will have to wait to see > where we are with driver support as the release gets closer. With the release near, I'm promoting this to the regular open issues section. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Heikki, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: DROP SUBSCRIPTION hangs if sub is disabled in the same transaction
On Thu, Sep 07, 2017 at 04:53:12AM +, Noah Misch wrote: > On Wed, Sep 06, 2017 at 03:28:47PM +0900, Masahiko Sawada wrote: > > On Mon, Sep 4, 2017 at 11:43 PM, Arseny Sher wrote: > > > Arseny Sher writes: > > > > > >> Attached patch fixes this by stopping workers before RO drop, as > > >> already done in case when we drop replication slot. > > > > > > Sorry, here is the patch. > > > > > > > I could reproduce this issue, it's a bug. Added this to the open item. > > The cause of this is commit 7e174fa7 which make subscription ddls kill > > the apply worker only when transaction commit. I didn't look the patch > > deeply yet but I'm not sure the approach that kills apply worker > > before commit would be good. > > [Action required within three days. This is a generic notification.] > > The above-described topic is currently a PostgreSQL 10 open item. Peter, > since you committed the patch believed to have created it, you own this open > item. If some other commit is more relevant or if this does not belong as a > v10 open item, please let us know. Otherwise, please observe the policy on > open item ownership[1] and send a status update within three calendar days of > this message. Include a date for your subsequent status update. Testers may > discover new open items at any time, and I want to plan to get them all fixed > well in advance of shipping v10. Consequently, I will appreciate your efforts > toward speedy resolution. Thanks. > > [1] > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: DROP SUBSCRIPTION hangs if sub is disabled in the same transaction
On Wed, Sep 06, 2017 at 03:28:47PM +0900, Masahiko Sawada wrote: > On Mon, Sep 4, 2017 at 11:43 PM, Arseny Sher wrote: > > Arseny Sher writes: > > > >> Attached patch fixes this by stopping workers before RO drop, as > >> already done in case when we drop replication slot. > > > > Sorry, here is the patch. > > > > I could reproduce this issue, it's a bug. Added this to the open item. > The cause of this is commit 7e174fa7 which make subscription ddls kill > the apply worker only when transaction commit. I didn't look the patch > deeply yet but I'm not sure the approach that kills apply worker > before commit would be good. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Peter, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Tuple-routing for certain partitioned tables not working as expected
On Tue, Sep 05, 2017 at 08:35:13PM +0900, Etsuro Fujita wrote: > On 2017/08/30 17:20, Etsuro Fujita wrote: > >On 2017/08/30 9:13, Amit Langote wrote: > >>On 2017/08/29 20:18, Etsuro Fujita wrote: > >>>On 2017/08/25 22:26, Robert Haas wrote: > On Wed, Aug 23, 2017 at 4:55 AM, Etsuro Fujita > wrote: > >Agreed, but I'd vote for fixing this in v10 as proposed; I agree > >that just > >ripping the CheckValidResultRel checks out entirely is not a good > >idea, > >but > >that seems OK to me at least as a fix just for v10. > > I'm still not on-board with having this be the one case where we don't > do CheckValidResultRel. If we want to still call it but pass down > some additional information that can selectively skip certain checks, > I could probably live with that. > >>> > >>>Another idea would be to not do CheckValidResultRel for partitions in > >>>ExecSetupPartitionTupleRouting; instead, do that the first time the > >>>partition is chosen by ExecFindPartition, and if successfully checked, > >>>initialize the partition's ResultRelInfo and other stuff. (We could > >>>skip > >>>this after the first time by checking whether we already have a valid > >>>ResultRelInfo for the chosen partition.) That could allow us to not > >>>only > >>>call CheckValidResultRel the way it is, but avoid initializing useless > >>>partitions for which tuples aren't routed at all. > >> > >>I too have thought about the idea of lazy initialization of the partition > >>ResultRelInfos. I think it would be a good idea, but definitely > >>something > >>for PG 11. > > > >Agreed. Here is a patch to skip the CheckValidResultRel checks for a > >target table if it's a foreign partition to perform tuple-routing for, as > >proposed by Robert. > > I'll add this to the open items list for v10. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Robert, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Parallel worker error
On Thu, Aug 31, 2017 at 03:11:10PM -0400, Robert Haas wrote: > On Wed, Aug 30, 2017 at 11:19 AM, Robert Haas wrote: > > But since that's an established design fl^H^Hprinciple, maybe that > > means we should go with the approach of teaching SerializeGUCState() > > to ignore role altogether and instead have ParallelWorkerMain call > > SetCurrentRoleId using information passed via the FixedParallelState > > (not sure of the precise details here). > > Could I get some opinions on the virtues of this approach, vs. any of > the other suggestions at or near > http://postgr.es/m/ca+tgmoasp90e33-mu2ypgs73ttj37m5hv-xqhjc7tpqx9wx...@mail.gmail.com > ? It seems good to me, better than the other options in that mail. This does rely on "role" being on the only vulnerable GUC. Looking at callers of GUC_check_errmsg(), I don't expect trouble in any other GUC. (I suspect one can hit the "permission denied to set role" during parallel initialization after a concurrent ALTER ROLE removes a membership.) [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Robert, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Re: [PATCH] Fix drop replication slot blocking instead of returning error
On Tue, Aug 29, 2017 at 12:04:33PM +0200, Alvaro Herrera wrote: > Noah Misch wrote: > > On Thu, Aug 24, 2017 at 03:38:20PM +0200, Simone Gotti wrote: > > > I noticed that in postgres 10beta3, calling pg_drop_replication_slot on an > > > active slot will block until it's released instead of returning an error > > > like > > > done in pg 9.6. Since this is a change in the previous behavior and the > > > docs > > > wasn't changed I made a patch to restore the previous behavior. > > > > after git commit 9915de6c1cb calls to pg_drop_replication_slot or the > > > replication protocol DROP_REPLICATION_SLOT command will block when a > > > slot is in use instead of returning an error as before (as the doc > > > states). > > > > > > This patch will set nowait to true to restore the previous > > > behavior. > > > The above-described topic is currently a PostgreSQL 10 open item. > > Looking at it now -- next report tomorrow. This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] More replication race conditions
On Tue, Aug 29, 2017 at 08:44:42PM +0900, Michael Paquier wrote: > On Mon, Aug 28, 2017 at 8:25 AM, Michael Paquier > wrote: > > Today's run has finished with the same failure: > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=dangomushi&dt=2017-08-27%2018%3A00%3A13 > > Attached is a patch to make this code path wait that the transaction > > has been replayed. We could use as well synchronous_commit = apply, > > but I prefer the solution of this patch with a wait query. > > I have added an open item for this issue for PG10. The 2PC tests in > src/test/recovery are new in PG10, introduced by 3082098. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Álvaro, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] More replication race conditions
On Sun, Aug 27, 2017 at 02:32:49AM +, Noah Misch wrote:
> On Fri, Aug 25, 2017 at 12:09:00PM +0200, Petr Jelinek wrote:
> > On 24/08/17 19:54, Tom Lane wrote:
> > > sungazer just failed with
> > >
> > > pg_recvlogical exited with code '256', stdout '' and stderr
> > > 'pg_recvlogical: could not send replication command "START_REPLICATION
> > > SLOT "test_slot" LOGICAL 0/0 ("include-xids" '0', "skip-empty-xacts"
> > > '1')": ERROR: replication slot "test_slot" is active for PID 8913148
> > > pg_recvlogical: disconnected
> > > ' at
> > > /home/nm/farm/gcc64/HEAD/pgsql.build/src/test/recovery/../../../src/test/perl/PostgresNode.pm
> > > line 1657.
> > >
> > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sungazer&dt=2017-08-24%2015%3A16%3A10
> > >
> > > Looks like we're still not there on preventing replication startup
> > > race conditions.
> >
> > Hmm, that looks like "by design" behavior. Slot acquiring will throw
> > error if the slot is already used by somebody else (slots use their own
> > locking mechanism that does not wait). In this case it seems the
> > walsender which was using slot for previous previous step didn't finish
> > releasing the slot by the time we start new command. We can work around
> > this by changing the test to wait perhaps.
>
> [Action required within three days. This is a generic notification.]
>
> The above-described topic is currently a PostgreSQL 10 open item. Simon,
> since you committed the patch believed to have created it, you own this open
> item. If some other commit is more relevant or if this does not belong as a
> v10 open item, please let us know. Otherwise, please observe the policy on
> open item ownership[1] and send a status update within three calendar days of
> this message. Include a date for your subsequent status update. Testers may
> discover new open items at any time, and I want to plan to get them all fixed
> well in advance of shipping v10. Consequently, I will appreciate your efforts
> toward speedy resolution. Thanks.
>
> [1]
> https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com
This PostgreSQL 10 open item is past due for your status update. Kindly send
a status update within 24 hours, and include a date for your subsequent status
update. Refer to the policy on open item ownership:
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Back-branch release notes up for review
On Sat, Aug 26, 2017 at 03:31:12PM -0400, Tom Lane wrote: > + > + > + > + Show foreign tables > + in information_schema.table_privileges > + view (Peter Eisentraut) > + > + > + > + All other relevant information_schema views include > + foreign tables, but this one ignored them. > + > + > + > + Since this view definition is installed by initdb, > + merely upgrading will not fix the problem. If you need to fix this > + in an existing installation, you can, as a superuser, do this > + in psql: > + > +BEGIN; > +DROP SCHEMA information_schema CASCADE; > +\i SHAREDIR/information_schema.sql > +COMMIT; > + > + (Run pg_config --sharedir if you're uncertain > + where SHAREDIR is.) This must be repeated in each > + database to be fixed. > + > + "DROP SCHEMA information_schema CASCADE;" will drop objects outside information_schema that depend on objects inside information_schema. For example, this will drop user-defined views if the view query refers to information_schema. That's improper in a release-noted update procedure. This could instead offer a CREATE OR REPLACE VIEW or just hand-wave about the repaired definition being available in information_schema.sql. I regret not reading this before today. -- 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] More replication race conditions
On Fri, Aug 25, 2017 at 12:09:00PM +0200, Petr Jelinek wrote:
> On 24/08/17 19:54, Tom Lane wrote:
> > sungazer just failed with
> >
> > pg_recvlogical exited with code '256', stdout '' and stderr
> > 'pg_recvlogical: could not send replication command "START_REPLICATION SLOT
> > "test_slot" LOGICAL 0/0 ("include-xids" '0', "skip-empty-xacts" '1')":
> > ERROR: replication slot "test_slot" is active for PID 8913148
> > pg_recvlogical: disconnected
> > ' at
> > /home/nm/farm/gcc64/HEAD/pgsql.build/src/test/recovery/../../../src/test/perl/PostgresNode.pm
> > line 1657.
> >
> > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=sungazer&dt=2017-08-24%2015%3A16%3A10
> >
> > Looks like we're still not there on preventing replication startup
> > race conditions.
>
> Hmm, that looks like "by design" behavior. Slot acquiring will throw
> error if the slot is already used by somebody else (slots use their own
> locking mechanism that does not wait). In this case it seems the
> walsender which was using slot for previous previous step didn't finish
> releasing the slot by the time we start new command. We can work around
> this by changing the test to wait perhaps.
[Action required within three days. This is a generic notification.]
The above-described topic is currently a PostgreSQL 10 open item. Simon,
since you committed the patch believed to have created it, you own this open
item. If some other commit is more relevant or if this does not belong as a
v10 open item, please let us know. Otherwise, please observe the policy on
open item ownership[1] and send a status update within three calendar days of
this message. Include a date for your subsequent status update. Testers may
discover new open items at any time, and I want to plan to get them all fixed
well in advance of shipping v10. Consequently, I will appreciate your efforts
toward speedy resolution. Thanks.
[1]
https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [PATCH] Fix drop replication slot blocking instead of returning error
On Thu, Aug 24, 2017 at 03:38:20PM +0200, Simone Gotti wrote: > I noticed that in postgres 10beta3, calling pg_drop_replication_slot on an > active slot will block until it's released instead of returning an error > like > done in pg 9.6. Since this is a change in the previous behavior and the docs > wasn't changed I made a patch to restore the previous behavior. > > Thanks, > > Simone. > > -- > > after git commit 9915de6c1cb calls to pg_drop_replication_slot or the > replication protocol DROP_REPLICATION_SLOT command will block when a > slot is in use instead of returning an error as before (as the doc > states). > > This patch will set nowait to true to restore the previous > behavior. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Álvaro, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] possible encoding issues with libxml2 functions
On Sun, Aug 20, 2017 at 10:54:57AM +0200, Pavel Stehule wrote:
> 2017-08-20 9:21 GMT+02:00 Noah Misch :
> > On Mon, Aug 07, 2017 at 05:10:14PM -0700, Noah Misch wrote:
> > > On Wed, Apr 05, 2017 at 10:53:39PM +0200, Pavel Stehule wrote:
> > > > One possible fix - and similar technique is used more times in xml.c
> > > > code
> > > > .. xmlroot
> > >
> > > > + /* remove header */
> > > > + parse_xml_decl(string, &header_len, NULL, NULL, NULL);
> > >
> > > > - doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, NULL,
> > > > 0);
> > > > + doc = xmlCtxtReadMemory(ctxt, (char *) string + header_len, len
> > > > -
> >
> > > I like this parse_xml_decl() approach better. Would you update
> > > your patch to use it and to add the test case I give above?
> >
> > Again, would you make those two edits?
>
> Now, I am not sure so it is correct fix. We will fix case, when server is
> in UTF8, but maybe we will break some cases when server is not in UTF8
> (although it is broken already).
That's right.
> I am think so correct solution is encoding to UTF8 and passing encoding
> parameter. It will works safely in all cases - and we don't need cut
> header. We should not to cut header if server encoding is not in UTF8 and
> we don't pass encoding as parameter. When we pass encoding as parameter,
> then cutting header is not necessary.
xpath-bugfix.patch affected only xml values containing an xml declaration with
"encoding" attribute. In UTF8 databases, this latest proposal
(xpath-parsing-error-fix.patch) is equivalent to xpath-bugfix.patch. In
non-UTF8 databases, xpath-parsing-error-fix.patch affects all xml values
containing non-ASCII data. In a LATIN1 database, the following works today
but breaks under your latest proposal:
SELECT xpath('text()', ('' || convert_from('\xc2b0', 'LATIN1') ||
'')::xml);
It's acceptable to break that, since the documentation explicitly disclaims
support for it. xpath-bugfix.patch breaks different use cases, which are
likewise acceptable to break. See my 2017-08-08 review for details.
We have xpath-bugfix.patch and xpath-parsing-error-fix.patch. Both are
equivalent under supported use cases (xpath in UTF8 databases). Among
non-supported use cases, they each make different things better and different
things worse. We should prefer to back-patch the version harming fewer
applications. I expect non-ASCII data is more common than xml declarations
with "encoding" attribute, so xpath-bugfix.patch will harm fewer applications.
Having said that, I now see a third option. Condition this thread's patch's
effects on GetDatabaseEncoding()==PG_UTF8. That way, we fix supported cases,
and we remain bug-compatible in unsupported cases. I think that's better than
the other options discussed so far. If you agree, please send a patch based
on xpath-bugfix.patch with the GetDatabaseEncoding()==PG_UTF8 change and the
two edits I described earlier.
Thanks,
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] possible encoding issues with libxml2 functions
On Sun, Aug 20, 2017 at 08:46:03AM +0200, Pavel Stehule wrote: > 2017-08-20 4:17 GMT+02:00 Noah Misch : > > On Sat, Aug 19, 2017 at 10:53:19PM +0200, Pavel Stehule wrote: > > > I am sending some POC - it does support XPATH and XMLTABLE for not UTF8 > > > server encoding. > > > > > > In this case, all strings should be converted to UTF8 before call libXML2 > > > functions, and result should be converted back from UTF8. > > > > Adding support for xpath in non-UTF8 databases is a v11 feature proposal. > > Please start a new thread for this, and add it to the open CommitFest. > > > > In this thread, would you provide the version of your patch that I > > described > > in my 2017-08-08 post to this thread? That's a back-patchable bug fix. > > > There are three issues: > > 1. processing 1byte encoding XMLs documents with encoding declaration - > should be fixed by ecoding_for_xmlCtxtReadMemory.patch. This patch is very > short and safe - can be apply immediately (there is regress tests) We should fix that problem, yes. encoding_for_xmlCtxtReadMemory.patch is not the right fix; see below. > 2 encoding issues in XPath specification (and namespaces) - because > multibytes chars are not usually used in tag names, this issue hit minimum > users. > > 3. encoding issues in XPath and XMLTABLE results - this is bad issue - the > function XMLTABLE will not be functional on non UTF8 databases. Fortunately > - there are less users with this encoding, but probably should be apply as > fix in 10/11 Postgres. (2) and (3) are long-documented (since commit 14180f9, 2009-06) limitations in xpath functions. That's why I would treat an improvement as a new feature and not back-patch it. It is tempting to fix v10 so XMLTABLE is born without this limitation, but it is too late in the release cycle. Reviewing encoding_for_xmlCtxtReadMemory.patch: On Sat, Aug 19, 2017 at 09:13:50AM +0200, Pavel Stehule wrote: > - doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, NULL, > 0); > + doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, > + > pg_encoding_to_char(GetDatabaseEncoding()), 0); This assumes libxml2 understands every encoding name that pg_encoding_to_char() can return, but it does not. xpath-bugfix.patch was better. Here are the relevant parts of my review of that patch. On Mon, Aug 07, 2017 at 05:10:14PM -0700, Noah Misch wrote: > On Wed, Apr 05, 2017 at 10:53:39PM +0200, Pavel Stehule wrote: > > One possible fix - and similar technique is used more times in xml.c code > > .. xmlroot > > > + /* remove header */ > > + parse_xml_decl(string, &header_len, NULL, NULL, NULL); > > > - doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, NULL, 0); > > + doc = xmlCtxtReadMemory(ctxt, (char *) string + header_len, len - > I like this parse_xml_decl() approach better. Would you update > your patch to use it and to add the test case I give above? Again, would you make those two edits? Thanks, 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] possible encoding issues with libxml2 functions
On Fri, Aug 18, 2017 at 11:43:19PM +0200, Pavel Stehule wrote: > yes, probably libXML2 try to do check from utf8 encoding to header > specified encoding. Yes. That has been the topic of this thread. > a) all values created by xml_in iterface are in database encoding - input > string is stored without any change. xml_parse is called only due > validation. > > b) inside xml_parse, the input is converted to UTF8, and document is read > by xmlCtxtReadDoc with explicitly specified "UTF-8" encoding or > by xmlParseBalancedChunkMemory with explicitly specified encoding "UTF8" > and removed decl section. > > So for "xml_parse" based functions (xml_in, texttoxml, xml_is_document, > wellformated_xml) the database encoding is not important > > c) xml_recv function does validation by xml_parse and translation to > database encoding. > > Now I don't see a difference between @b and @c - so my hypotheses about > necessity to use recv interface was wrong. Yes. You posted, on 2017-04-05, a test case not requiring the recv interface. On Sat, Aug 19, 2017 at 09:13:50AM +0200, Pavel Stehule wrote: > I didn't find any info how to enable libXML2 XPath functions for other > encoding than UTF8 :( ?? http://xmlsoft.org/encoding.html is the relevant authority. To summarize, we should send only UTF8 to libxml2. On Sat, Aug 19, 2017 at 10:53:19PM +0200, Pavel Stehule wrote: > I am sending some POC - it does support XPATH and XMLTABLE for not UTF8 > server encoding. > > In this case, all strings should be converted to UTF8 before call libXML2 > functions, and result should be converted back from UTF8. Adding support for xpath in non-UTF8 databases is a v11 feature proposal. Please start a new thread for this, and add it to the open CommitFest. In this thread, would you provide the version of your patch that I described in my 2017-08-08 post to this thread? That's a back-patchable bug fix. > I found some previous experiments > https://marc.info/?l=pgsql-bugs&m=123407176408688 https://wiki.postgresql.org/wiki/Todo#XML links to other background on this feature proposal. See Tom Lane's review of a previous patch. Ensure your patch does not have the problems he found during that review. Do that before starting a thread for this feature. Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [COMMITTERS] pgsql: Account for catalog snapshot in PGXACT->xmin updates.
On Tue, Dec 06, 2016 at 01:59:18PM -0500, Robert Haas wrote:
> On Tue, Dec 6, 2016 at 1:36 PM, Tom Lane wrote:
> > Robert Haas writes:
> >> On Tue, Nov 15, 2016 at 3:55 PM, Tom Lane wrote:
> >>> Account for catalog snapshot in PGXACT->xmin updates.
> >
> >> It seems to me that this makes it possible for
> >> ThereAreNoPriorRegisteredSnapshots() to fail spuriously. The only
> >> core code that calls that function is in copy.c, and apparently we
> >> never reach that point with the catalog snapshot set. But that seems
> >> fragile.
I recently noticed this by way of the copy2 regression test failing, in 9.4
and later, under REPEATABLE READ and SERIALIZABLE. That failure started with
$SUBJECT. Minimal example:
CREATE TABLE vistest (a text);
BEGIN ISOLATION LEVEL REPEATABLE READ;
TRUNCATE vistest;
COPY vistest FROM stdin CSV FREEZE;
x
\.
Before $SUBJECT, the registered snapshot count was one. Since $SUBJECT, the
catalog snapshot raises the count to two.
> > Hm. If there were some documentation explaining what
> > ThereAreNoPriorRegisteredSnapshots is supposed to do, it would be possible
> > for us to have a considered argument as to whether this patch broke it or
> > not. As things stand, it is not obvious whether it ought to be ignoring
> > the catalog snapshot or not; one could construct plausible arguments
The rows COPY FREEZE writes will be visible (until deleted) to every possible
catalog snapshot, so whether CatalogSnapshot==NULL doesn't matter. It may be
useful to error out if a catalog scan is in-flight, but the registration for
CatalogSnapshot doesn't confirm or refute that.
> > either way. It's not even very obvious why both 0 and 1 registered
> > snapshots should be considered okay; that looks a lot more like a hack
> > than reasoned-out behavior. So I'm satisfied if COPY FREEZE does what
Fair summary. ThereAreNoPriorRegisteredSnapshots() has functioned that way
since an early submission[1], and I don't see that we ever discussed it
explicitly. Adding Simon for his recollection. I think the goal was to raise
an error if any scan of the COPY-target table might use an older CommandId;
this prevents a way of seeing tuples that the scan would not see after COPY
without FREEZE. Allowing one registered snapshot was a satisfactory heuristic
at the time. It rejected running plain queries, which have been registering
two snapshots. It did not reject otherwise-idle REPEATABLE READ transactions,
which register FirstXactSnapshot once; that has been okay, because any use of
FirstXactSnapshot in a query pushes or registers a copy. I think the code was
wrong to consider registered snapshots and ignore ActiveSnapshot, though. (We
must ignore COPY's own ActiveSnapshot, but it should be the only one.) I
don't blame $SUBJECT for changing this landscape; COPY should not attribute
meaning to a particular nonzero registered snapshot count.
> > it's supposed to do, which it still appears to do.
> >
> > IOW, I'm not interested in touching this unless someone first provides
> > adequate documentation for the pre-existing kluge here. There is no
> > API spec at all on the function itself, and the comment in front of the
> > one call site is too muddle-headed to provide any insight.
>
> COPY FREEZE is designed to be usable only in situations where the new
> tuples won't be visible earlier than would otherwise be the case.
> Thus, copy.c has a check that the relfilenode was created by our
> (sub)transaction, which guards against the possibility that other
> transactions might see the data early, and also a check that there are
> no other registered snapshots or ready portals, which guarantees that,
> for example, a cursor belonging to the current subtransaction but with
> a lower command-ID doesn't see the rows early. I am fuzzy why we need
> to check for both snapshots and portals, but the overall intent seems
The snapshot check helps in this case:
CREATE TABLE vistest (a text);
BEGIN ISOLATION LEVEL READ COMMITTED;
CREATE FUNCTION copy_vistest() RETURNS void LANGUAGE plpgsql
AS $$BEGIN COPY vistest FROM '/dev/null' CSV FREEZE; END$$;
CREATE FUNCTION count_vistest() RETURNS int LANGUAGE plpgsql STABLE
AS $$BEGIN RETURN (SELECT count(*) FROM vistest); END$$;
TRUNCATE vistest;
SELECT copy_vistest(), count_vistest();
ROLLBACK;
Other sessions still see rows they ideally would not see[2], the same kind of
anomaly the portal and snapshot tests exist to prevent. It's odd that we
protect visibility solely within the COPY transaction, the transaction most
likely to know what it's doing. I'm therefore skeptical of having these tests
at all, but I hesitate to remove them in back branches. Even in master, we
may want them later if we add visibility protection for other transactions.
I'm tempted to add this hack, ...
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -2445,2 +2445,3 @@ CopyFrom(CopyState cstate)
{
+ InvalidateCatalogSnapshot();
[HACKERS] Re: Stats for triggers on partitioned tables not shown in EXPLAIN ANALYZE
On Wed, Aug 16, 2017 at 05:14:25PM +0900, Amit Langote wrote: > On 2017/08/15 21:20, Etsuro Fujita wrote: > > I noticed that runtime stats for BEFORE ROW INSERT triggers on leaf > > partitions of partitioned tables aren't reported in EXPLAIN ANALYZE. Here > > is an example: > > So here is a > > patch for fixing both issues. > Adding this to the PG 10 open items list. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Robert, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Re: ICU collation variant keywords and pg_collation entries (Was: [BUGS] Crash report for some ICU-52 (debian8) COLLATE and work_mem values)
On Thu, Aug 17, 2017 at 09:22:07PM -0400, Peter Eisentraut wrote: > On 8/14/17 12:23, Peter Eisentraut wrote: > > On 8/13/17 15:39, Noah Misch wrote: > >> This PostgreSQL 10 open item is past due for your status update. Kindly > >> send > >> a status update within 24 hours, and include a date for your subsequent > >> status > >> update. Refer to the policy on open item ownership: > >> https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > > > I think there are up to three separate issues in play: > > > > - what to do about some preloaded collations disappearing between versions > > > > - whether to preload keyword variants > > > > - whether to canonicalize some things during CREATE COLLATION > > > > I responded to all these subplots now, but the discussion is ongoing. I > > will set the next check-in to Thursday. > > I haven't read anything since that has provided any more clarity about > what needs changing here. I will entertain concrete proposals about the > specific points above (considering any other issues under discussion to > be PG11 material), but in the absence of that, I don't plan any work on > this right now. I think you're contending that, as formulated, this is not a valid v10 open item. Are you? -- 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] One-shot expanded output in psql using \gx
On Tue, Aug 15, 2017 at 10:24:34PM +0200, Tobias Bussmann wrote: > I've tested the new \gx against 10beta and current git HEAD. Actually one of > my favourite features of PostgreSQL 10! However in my environment it was > behaving strangely. After some debugging I found that \gx does not work if > you have \set FETCH_COUNT n before. Please find attached a patch that fixes > this incl. new regression test. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Stephen, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] [postgresql 10 beta3] unrecognized node type: 90
On Fri, Aug 11, 2017 at 11:59:14AM -0400, Tom Lane wrote: > "Adam, Etienne (Nokia-TECH/Issy Les Moulineaux)" > writes: > > ERROR: XX000: unrecognized node type: 90 > > LOCATION: ExecReScan, execAmi.c:284 > > (gdb) p (NodeTag) 90 > $1 = T_GatherMergeState > > So, apparently somebody wrote ExecReScanGatherMerge, but never bothered > to plug it into ExecReScan. From which we may draw depressing conclusions > about how much it's been tested. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Robert, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] [BUGS] Replication to Postgres 10 on Windows is broken
On Fri, Aug 11, 2017 at 08:56:22PM -0700, Noah Misch wrote: > On Thu, Aug 10, 2017 at 09:59:40PM -0400, Tom Lane wrote: > > Noah Misch writes: > > > On Tue, Aug 08, 2017 at 07:25:37PM -0400, Peter Eisentraut wrote: > > >> I don't think I can usefully contribute to this. Could someone else > > >> take it? > > This PostgreSQL 10 open item is past due for your status update. Kindly send > a status update within 24 hours, and include a date for your subsequent status > update. Refer to the policy on open item ownership: > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com IMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long past due for your status update. Please reacquaint yourself with the policy on open item ownership[1] and then reply immediately. If I do not hear from you by 2017-08-14 20:00 UTC, I will transfer this item to release management team ownership without further notice. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: ICU collation variant keywords and pg_collation entries (Was: [BUGS] Crash report for some ICU-52 (debian8) COLLATE and work_mem values)
On Thu, Aug 10, 2017 at 04:51:16AM +, Noah Misch wrote: > On Mon, Aug 07, 2017 at 06:23:56PM -0400, Tom Lane wrote: > > Peter Eisentraut writes: > > > On 8/6/17 20:07, Peter Geoghegan wrote: > > >> I've looked into this. I'll give an example of what keyword variants > > >> there are for Greek, and then discuss what I think each is. > > > > > I'm not sure why we want to get into editorializing this. We query ICU > > > for the names of distinct collations and use that. It's more than most > > > people need, sure, but it doesn't cost us anything. > > > > Yes, *it does*. The cost will be borne by users who get screwed at update > > time, not by developers, but that doesn't make it insignificant. > > [Action required within three days. This is a generic notification.] > > The above-described topic is currently a PostgreSQL 10 open item. Peter, > since you committed the patch believed to have created it, you own this open > item. If some other commit is more relevant or if this does not belong as a > v10 open item, please let us know. Otherwise, please observe the policy on > open item ownership[1] and send a status update within three calendar days of > this message. Include a date for your subsequent status update. Testers may > discover new open items at any time, and I want to plan to get them all fixed > well in advance of shipping v10. Consequently, I will appreciate your efforts > toward speedy resolution. Thanks. > > [1] > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] More race conditions in logical replication
On Sat, Aug 12, 2017 at 05:38:29PM +0900, Michael Paquier wrote: > On Thu, Aug 10, 2017 at 4:22 PM, Michael Paquier > wrote: > > On Tue, Aug 8, 2017 at 8:11 PM, Alvaro Herrera > > wrote: > >> Here's a patch. It turned to be a bit larger than I initially expected. > > > > Álvaro, 030273b7 did not get things completely right. A couple of wait > > events have been added in the docs for sections Client, IPC and > > Activity but it forgot to update the counters for morerows . Attached > > is a patch to fix all that. > > As the documentation format is weird because of this commit, I am > adding an open item dedicated to that. Look for example at > WalSenderWaitForWAL in > https://www.postgresql.org/docs/devel/static/monitoring-stats.html. > While looking again, I have noticed that one line for the section of > LWLock is weird, so attached is an updated patch. Committed. These counts broke three times in the v10 release cycle. It's too bad this defect doesn't cause an error when building the docs. -- 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] [BUGS] Replication to Postgres 10 on Windows is broken
On Thu, Aug 10, 2017 at 09:59:40PM -0400, Tom Lane wrote: > Noah Misch writes: > > On Tue, Aug 08, 2017 at 07:25:37PM -0400, Peter Eisentraut wrote: > >> I don't think I can usefully contribute to this. Could someone else > >> take it? This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > If nobody volunteers, you could always resolve this by reverting 1e8a850 and > > successors. > > I think you're blaming the victim. Our current theory about the cause > of this is that on Windows, WaitLatchOrSocket cannot be used to wait for > completion of a nonblocking connect() call. That seems pretty broken > independently of whether libpqwalreceiver needs the capability. Yes, the theorized defect lies in APIs commit 1e8a850 used, not in the commit itself. Nonetheless, commit 1e8a850 promoted the defect from one reachable only by writing C code to one reachable by merely configuring replication on Windows according to the documentation. For that, its committer owns this open item. Besides the one approach I mentioned, there exist several other fine ways to implement said ownership. > In any case, we have a draft patch, so what we should be pressing for > is for somebody to test it. Now done. (Thanks, Jobin.) -- 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] intermittent failures in Cygwin from select_parallel tests
On Thu, Aug 03, 2017 at 10:45:50AM -0400, Robert Haas wrote:
> On Wed, Aug 2, 2017 at 11:47 PM, Noah Misch wrote:
> > postmaster algorithms rely on the PG_SETMASK() calls preventing that.
> > Without
> > such protection, duplicate bgworkers are an understandable result. I caught
> > several other assertions; the PMChildFlags failure is another case of
> > duplicate postmaster children:
> >
> > 6 TRAP: FailedAssertion("!(entry->trans == ((void *)0))", File:
> > "pgstat.c", Line: 871)
> > 3 TRAP: FailedAssertion("!(PMSignalState->PMChildFlags[slot] == 1)",
> > File: "pmsignal.c", Line: 229)
> > 20 TRAP: FailedAssertion("!(RefCountErrors == 0)", File: "bufmgr.c",
> > Line: 2523)
> > 21 TRAP: FailedAssertion("!(vmq->mq_sender == ((void *)0))", File:
> > "shm_mq.c", Line: 221)
> > Also, got a few "select() failed in postmaster: Bad address"
> >
> > I suspect a Cygwin signals bug. I'll try to distill a self-contained test
> > case for the Cygwin hackers. The lack of failures on buildfarm member
> > brolga
> > argues that older Cygwin is not affected.
>
> Nice detective work.
Thanks. http://marc.info/?t=15018329641 has my upstream report. The
Cygwin project lead reproduced this, but a fix remained elusive.
I guess we'll ignore weird postmaster-associated lorikeet failures for the
foreseeable future.
--
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] [BUGS] Replication to Postgres 10 on Windows is broken
On Tue, Aug 08, 2017 at 07:25:37PM -0400, Peter Eisentraut wrote: > On 8/7/17 21:06, Noah Misch wrote: > >> That would fit. Until v10 (commit 1e8a850), PQconnectStart() had no > >> in-tree > >> callers outside of libpq itself. > > [Action required within three days. This is a generic notification.] > > > > The above-described topic is currently a PostgreSQL 10 open item. Peter, > > since you committed the patch believed to have created it, you own this open > > item. > > I don't think I can usefully contribute to this. Could someone else > take it? If nobody volunteers, you could always resolve this by reverting 1e8a850 and successors. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: ICU collation variant keywords and pg_collation entries (Was: [BUGS] Crash report for some ICU-52 (debian8) COLLATE and work_mem values)
On Mon, Aug 07, 2017 at 06:23:56PM -0400, Tom Lane wrote: > Peter Eisentraut writes: > > On 8/6/17 20:07, Peter Geoghegan wrote: > >> I've looked into this. I'll give an example of what keyword variants > >> there are for Greek, and then discuss what I think each is. > > > I'm not sure why we want to get into editorializing this. We query ICU > > for the names of distinct collations and use that. It's more than most > > people need, sure, but it doesn't cost us anything. > > Yes, *it does*. The cost will be borne by users who get screwed at update > time, not by developers, but that doesn't make it insignificant. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Peter, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] free space % calculation in pgstathashindex
On Tue, Aug 08, 2017 at 02:30:51PM +0530, Amit Kapila wrote: > On Mon, Aug 7, 2017 at 9:38 PM, Ashutosh Sharma wrote: > > On Mon, Aug 7, 2017 at 7:19 PM, Amit Kapila wrote: > >> On Mon, Aug 7, 2017 at 6:07 PM, Ashutosh Sharma > >> wrote: > >>> Hi, > >>> > >> .. > .. > >> Why an extra parenthesis in above case whereas not in below case? I > >> think the code will look consistent if you follow the same coding > >> practice. I suggest don't use it unless you need it. > > > > That is because in the 1st case, there are multiple operators (*, +) > > whereas in the 2nd case we have just one(*). So, just to ensure that > > '*' is performed before '+', i had used parenthesis, though it is not > > required as '*' has higher precedence than '+'. I have removed the > > extra parenthesis and attached is the new version of patch. Thanks. > > > > Your latest patch looks good to me. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Robert, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] [BUGS] Replication to Postgres 10 on Windows is broken
On Sun, Aug 06, 2017 at 08:50:37AM -0700, Noah Misch wrote: > On Sun, Aug 06, 2017 at 11:17:57AM -0400, Tom Lane wrote: > > Noah Misch writes: > > > I've added this as an open item. Confirmed in this setup: > > > > > -- Client > > > Windows Server 2016 > > > postgresql-10.0-beta2-windows-x64-binaries.zip from EnterpriseDB > > > > I wonder whether the other complainants were using EDB's build, > > and if not, just what were they using. The indirect question is: > > what version of OpenSSL is the Windows build using? > > Those binaries I used have OpenSSL 1.0.2l. > > > > I don't, however, see a smoking gun among commits. Would you bisect the > > > commits since 9.6 and see which one broke things? > > > > Gut instinct says that the reason this case fails when other tools > > can connect successfully is that libpqwalreceiver is the only tool > > that uses PQconnectStart/PQconnectPoll rather than a plain > > PQconnectdb, and that there is some behavioral difference between > > connectDBComplete's wait loop and libpqrcv_connect's wait loop that > > That would fit. Until v10 (commit 1e8a850), PQconnectStart() had no in-tree > callers outside of libpq itself. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Peter, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] [TRAP: FailedAssertion] causing server to crash
On Mon, Aug 07, 2017 at 05:29:34PM +1200, Thomas Munro wrote: > On Thu, Aug 3, 2017 at 3:03 AM, Robert Haas wrote: > > On Fri, Jul 21, 2017 at 1:31 AM, Thomas Munro > > wrote: > >> Thanks Neha. It's be best to post the back trace and if possible > >> print oldestXact and ShmemVariableCache->oldestXid from the stack > >> frame for TruncateCLOG. > >> > >> The failing assertion in TruncateCLOG() has a comment that says > >> "vac_truncate_clog already advanced oldestXid", but vac_truncate_clog > >> calls SetTransactionIdLimit() to write ShmemVariableCache->oldestXid > >> *after* it calls TruncateCLOG(). What am I missing here? > > > > This problem was introduced by commit > > ea42cc18c35381f639d45628d792e790ff39e271, so this should be added to > > the PostgreSQL 10 open items list. That commit intended to introduce a > > distinction between (1) the oldest XID that can be safely examined and > > (2) the oldest XID that can't yet be safely reused. These are the > > same except when we're in the middle of truncating CLOG: (1) advances > > before the truncation, and (2) advances afterwards. That's why > > AdvanceOldestClogXid() happens before truncation proper and > > SetTransactionIdLimit() happens afterwards, and changing the order > > would, I think, be quite wrong. > > Added to open items. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Robert, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] possible encoding issues with libxml2 functions
On Wed, Apr 05, 2017 at 10:53:39PM +0200, Pavel Stehule wrote:
> 2017-03-17 4:23 GMT+01:00 Noah Misch :
> > On Sun, Mar 12, 2017 at 10:26:33PM +0100, Pavel Stehule wrote:
> > > 2017-03-12 21:57 GMT+01:00 Noah Misch :
> > > > On Sun, Mar 12, 2017 at 08:36:58PM +0100, Pavel Stehule wrote:
> > > > > 2017-03-12 0:56 GMT+01:00 Noah Misch :
> > > > Please add a test case.
>
> I am sending test case.
>
> I am afraid so this cannot be part of regress tests due strong dependency
> on locale win1250.
Right. Based on that, I've distilled this portable test case:
-- Round-trip non-ASCII data through xpath().
DO $$
DECLARE
xml_declaration text := '';
degree_symbol text;
res xml[];
BEGIN
-- Per the documentation, xpath() doesn't work on non-ASCII data when
-- the server encoding is not UTF8. The EXCEPTION block below,
-- currently dead code, will be relevant if we remove this limitation.
IF current_setting('server_encoding') <> 'UTF8' THEN
RAISE LOG 'skip: encoding % unsupported for xml',
current_setting('server_encoding');
RETURN;
END IF;
degree_symbol := convert_from('\xc2b0', 'UTF8');
res := xpath('text()', (xml_declaration ||
'' || degree_symbol || '')::xml);
IF degree_symbol <> res[1]::text THEN
RAISE 'expected % (%), got % (%)',
degree_symbol, convert_to(degree_symbol, 'UTF8'),
res[1], convert_to(res[1]::text, 'UTF8');
END IF;
EXCEPTION
-- character with byte sequence 0xc2 0xb0 in encoding "UTF8" has no
equivalent in encoding "LATIN8"
WHEN untranslatable_character THEN RAISE LOG 'skip: %', SQLERRM;
-- default conversion function for encoding "UTF8" to "MULE_INTERNAL"
does not exist
WHEN undefined_function THEN RAISE LOG 'skip: %', SQLERRM;
END
$$;
> > > > Why not use xml_parse() instead of calling xmlCtxtReadMemory()
> > > > directly? The answer is probably in the archives, because someone
> > > > understood the problem enough to document "Some XML-related functions
> > > > may not work at all on non-ASCII data when the server encoding is not
> > > > UTF-8. This is known to be an issue for xpath() in particular."
> > >
> > > Probably there are two possible issues
> >
> > Would you research in the archives to confirm?
> >
> > > 1. what I touched - recv function does encoding to database encoding -
> > > but document encoding is not updated.
> >
> > Using xml_parse() would fix that, right?
> >
>
> It should to help, because it cuts a header - but it does little bit more
> work, than we would - it check if xml is doc or not too.
That's true. xpath() currently works on both DOCUMENT and CONTENT xml values.
If xpath() used xml_parse(), this would stop working:
SELECT xpath('text()', XMLPARSE (DOCUMENT 'bar'));
> One possible fix - and similar technique is used more times in xml.c code
> .. xmlroot
> + /* remove header */
> + parse_xml_decl(string, &header_len, NULL, NULL, NULL);
> - doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, NULL, 0);
> + doc = xmlCtxtReadMemory(ctxt, (char *) string + header_len, len -
> another possibility is using xml_out_internal - that is used in XMLTABLE
> function - what was initial fix.
>
> xml_out_internal uses parse_xml_decl too, but it is little bit more
> expensive due call print_xml_decl that has not any effect in this case
> (where only encoding is interesting) - but it can has sense, when server
> encoding is not UTF8, because in this case, xmlstr is not encoded to UTF8 -
> so now, I am think the correct solution should be using xml_out_internal -
> because it appends a header with server encoding to XML doc
As you may have been implying here, your patch never adds an xml declaration
that bears an encoding. (That is because it passes targetencoding=0 to
xml_out_internal().) If we were going to fix xpath() to support non-ASCII
characters in non-UTF8 databases, we would not do that by adding xml
declarations. We would do our own conversion to UTF8, like xml_parse() does.
In that light, I like this parse_xml_decl() approach better. Would you update
your patch to use it and to add the test case I give above?
This change can make things worse in a non-UTF8 database. The following
happens to work today in a LATIN1 database, but it will cease to work:
SELECT xpath('stri
[HACKERS] Re: Crash report for some ICU-52 (debian8) COLLATE and work_mem values
Adding -hackers. On Sat, Aug 05, 2017 at 03:55:13PM -0700, Noah Misch wrote: > On Thu, Aug 03, 2017 at 11:42:25AM -0700, Peter Geoghegan wrote: > > On Thu, Aug 3, 2017 at 8:49 AM, Daniel Verite > > wrote: > > > With query #2 it ends up crashing after ~5hours and produces > > > the log in log-valgrind-2.txt.gz with some other entries than > > > case #1, but AFAICS still all about reading uninitialised values > > > in space allocated by datumCopy(). > > > > Right. This part is really interesting to me: > > > > ==48827== Uninitialised value was created by a heap allocation > > ==48827==at 0x4C28C20: malloc (vg_replace_malloc.c:296) > > ==48827==by 0x80B597: AllocSetAlloc (aset.c:771) > > ==48827==by 0x810ADC: palloc (mcxt.c:862) > > ==48827==by 0x72BFEF: datumCopy (datum.c:171) > > ==48827==by 0x81A74C: tuplesort_putdatum (tuplesort.c:1515) > > ==48827==by 0x5E91EB: advance_aggregates (nodeAgg.c:1023) > > > > If you actually go to datum.c:171, you see that that's a codepath for > > pass-by-reference datatypes that lack a varlena header. Text is a > > datatype that has a varlena header, though, so that's clearly wrong. I > > don't know how this actually happened, but working back through the > > relevant tuplesort_begin_datum() caller, initialize_aggregate(), does > > suggest some things. (tuplesort_begin_datum() is where datumTypeLen is > > determined for the entire datum tuplesort.) > > > > I am once again only guessing, but I have to wonder if this is a > > problem in commit b8d7f053. It seems likely that the problem begins > > before tuplesort_begin_datum() is even called, which is the basis of > > this suspicion. If the problem is within tuplesort, then that could > > only be because get_typlenbyval() gives wrong answers, which seems > > very unlikely. > > [Action required within three days. This is a generic notification.] > > The above-described topic is currently a PostgreSQL 10 open item. Peter > (Eisentraut), since you committed the patch believed to have created it, you > own this open item. If some other commit is more relevant or if this does not > belong as a v10 open item, please let us know. Otherwise, please observe the > policy on open item ownership[1] and send a status update within three > calendar days of this message. Include a date for your subsequent status > update. Testers may discover new open items at any time, and I want to plan > to get them all fixed well in advance of shipping v10. Consequently, I will > appreciate your efforts toward speedy resolution. Thanks. > > [1] > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Subscription code improvements
On Wed, Aug 02, 2017 at 04:09:43PM -0400, Peter Eisentraut wrote: > On 8/1/17 00:17, Noah Misch wrote: > > The above-described topic is currently a PostgreSQL 10 open item. Peter, > > since you committed the patch believed to have created it, you own this open > > item. If some other commit is more relevant or if this does not belong as a > > v10 open item, please let us know. Otherwise, please observe the policy on > > open item ownership[1] and send a status update within three calendar days > > of > > this message. Include a date for your subsequent status update. Testers > > may > > discover new open items at any time, and I want to plan to get them all > > fixed > > well in advance of shipping v10. Consequently, I will appreciate your > > efforts > > toward speedy resolution. Thanks. > > I'm looking into this now and will report by Friday. This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] intermittent failures in Cygwin from select_parallel tests
On Wed, Jun 21, 2017 at 06:44:09PM -0400, Tom Lane wrote:
> Today, lorikeet failed with a new variant on the bgworker start crash:
>
> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=lorikeet&dt=2017-06-21%2020%3A29%3A10
>
> This one is even more exciting than the last one, because it sure looks
> like the crashing bgworker took the postmaster down with it. That is
> Not Supposed To Happen.
>
> Wondering if we broke something here recently, I tried to reproduce it
> on a Linux machine by adding a randomized Assert failure in
> shm_mq_set_sender. I don't see any such problem, even with EXEC_BACKEND;
> we recover from the crash as-expected.
>
> So I'm starting to get a distinct feeling that there's something wrong
> with the cygwin port. But I dunno what.
I think signal blocking broke on Cygwin.
On a system (gcc 5.4.0, CYGWIN_NT-10.0 2.7.0(0.306/5/3) 2017-02-12 13:18
x86_64) that reproduces lorikeet's symptoms, I instrumented the postmaster as
attached. The patch's small_parallel.sql is a subset of select_parallel.sql
sufficient to reproduce the mq_sender Assert failure and the postmaster silent
exit. (It occasionally needed hundreds of iterations to do so.) The parallel
query normally starts four bgworkers; when the mq_sender Assert fired, the
test had started five workers in response to four registrations.
The postmaster.c instrumentation regularly detects sigusr1_handler() calls
while another sigusr1_handler() is already on the stack:
6328 2017-08-02 07:25:42.788 GMT LOG: forbid signals @ sigusr1_handler
6328 2017-08-02 07:25:42.788 GMT DEBUG: saw slot-0 registration, want 0
6328 2017-08-02 07:25:42.788 GMT DEBUG: saw slot-0 registration, want 1
6328 2017-08-02 07:25:42.788 GMT DEBUG: slot 1 not yet registered
6328 2017-08-02 07:25:42.789 GMT DEBUG: registering background worker
"parallel worker for PID 4776" (slot 1)
6328 2017-08-02 07:25:42.789 GMT DEBUG: saw slot-1 registration, want 2
6328 2017-08-02 07:25:42.789 GMT DEBUG: saw slot-0 registration, want 2
6328 2017-08-02 07:25:42.789 GMT DEBUG: slot 2 not yet registered
6328 2017-08-02 07:25:42.789 GMT DEBUG: registering background worker
"parallel worker for PID 4776" (slot 2)
6328 2017-08-02 07:25:42.789 GMT DEBUG: saw slot-2 registration, want 3
6328 2017-08-02 07:25:42.789 GMT DEBUG: saw slot-1 registration, want 3
6328 2017-08-02 07:25:42.789 GMT DEBUG: saw slot-0 registration, want 3
6328 2017-08-02 07:25:42.789 GMT DEBUG: slot 3 not yet registered
6328 2017-08-02 07:25:42.789 GMT DEBUG: registering background worker
"parallel worker for PID 4776" (slot 3)
6328 2017-08-02 07:25:42.789 GMT DEBUG: saw slot-3 registration, want 4
6328 2017-08-02 07:25:42.789 GMT DEBUG: saw slot-2 registration, want 4
6328 2017-08-02 07:25:42.789 GMT DEBUG: saw slot-1 registration, want 4
6328 2017-08-02 07:25:42.789 GMT DEBUG: saw slot-0 registration, want 4
6328 2017-08-02 07:25:42.789 GMT DEBUG: slot 4 not yet registered
6328 2017-08-02 07:25:42.789 GMT DEBUG: registering background worker
"parallel worker for PID 4776" (slot 4)
6328 2017-08-02 07:25:42.789 GMT DEBUG: starting background worker process
"parallel worker for PID 4776"
6328 2017-08-02 07:25:42.790 GMT LOG: forbid signals @ sigusr1_handler
6328 2017-08-02 07:25:42.790 GMT WARNING: signals already forbidden @
sigusr1_handler
6328 2017-08-02 07:25:42.790 GMT LOG: permit signals @ sigusr1_handler
postmaster algorithms rely on the PG_SETMASK() calls preventing that. Without
such protection, duplicate bgworkers are an understandable result. I caught
several other assertions; the PMChildFlags failure is another case of
duplicate postmaster children:
6 TRAP: FailedAssertion("!(entry->trans == ((void *)0))", File:
"pgstat.c", Line: 871)
3 TRAP: FailedAssertion("!(PMSignalState->PMChildFlags[slot] == 1)",
File: "pmsignal.c", Line: 229)
20 TRAP: FailedAssertion("!(RefCountErrors == 0)", File: "bufmgr.c", Line:
2523)
21 TRAP: FailedAssertion("!(vmq->mq_sender == ((void *)0))", File:
"shm_mq.c", Line: 221)
Also, got a few "select() failed in postmaster: Bad address"
I suspect a Cygwin signals bug. I'll try to distill a self-contained test
case for the Cygwin hackers. The lack of failures on buildfarm member brolga
argues that older Cygwin is not affected.
diff --git a/src/backend/access/transam/parallel.c
b/src/backend/access/transam/parallel.c
index 17b1038..f42034d 100644
--- a/src/backend/access/transam/parallel.c
+++ b/src/backend/access/transam/parallel.c
@@ -985,6 +985,8 @@ ParallelWorkerMain(Datum main_arg)
error_queue_space = shm_toc_lookup(toc, PARALLEL_KEY_ERROR_QUEUE,
false);
mq = (shm_mq *) (error_queue_space +
ParallelWorkerNumber *
PARALLEL_ERROR_QUEUE_SIZE);
+ write_stderr("PID %d claiming queue %d (%p)\n",
+MyProc->pid, ParallelWorkerNumber, mq);
shm_mq_set_sender(mq, MyProc);
mqh = shm_mq_attach
Re: [HACKERS] VACUUM and ANALYZE disagreeing on what reltuples means
On Tue, Jul 25, 2017 at 07:02:28PM +0200, Tomas Vondra wrote: > On 7/25/17 5:04 PM, Tom Lane wrote: > >Tomas Vondra writes: > >>Attached is a patch that (I think) does just that. The disagreement > >>was caused by VACUUM treating recently dead tuples as live, while > >>ANALYZE treats both of those as dead. > > > >>At first I was worried that this will negatively affect plans in > >>the long-running transaction, as it will get underestimates (due > >>to reltuples not including rows it can see). But that's a problem > >>we already have anyway, you just need to run ANALYZE in the other > >>session. > > > >This definitely will have some impact on plans, at least in cases > >where there's a significant number of unvacuumable dead tuples. So I > >think it's a bit late for v10, and I wouldn't want to back-patch at > >all. Please add to the next commitfest. > > > > I dare to disagree here, for two reasons. > > Firstly, the impact *is* already there, it only takes running ANALYZE. Or > VACUUM ANALYZE. In both those cases we already end up with > reltuples=n_live_tup. > > Secondly, I personally strongly prefer stable predictable behavior over > intermittent oscillations between two values. That's a major PITA on > production, both to investigate and fix. > FWIW I personally see this as a fairly annoying bug, and would vote to > backpatch it, although I understand people might object. I tend to agree. If you have a setup that somehow never uses ANALYZE or never uses VACUUM on a particular table, you might prefer today's (accidental) behavior. However, the discrepancy arises only on a table that gets dead tuples, and a table that gets dead tuples will see both VACUUM and ANALYZE soon enough. It does seem like quite a stretch to imagine someone wanting plans to depend on which of VACUUM or ANALYZE ran most recently. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_stop_backup(wait_for_archive := true) on standby server
On Thu, Jul 27, 2017 at 10:27:36AM -0400, Stephen Frost wrote: > Noah, all, > > * Noah Misch (n...@leadboat.com) wrote: > > This PostgreSQL 10 open item is past due for your status update. Kindly > > send > > a status update within 24 hours, and include a date for your subsequent > > status > > update. Refer to the policy on open item ownership: > > Based on the ongoing discussion, this is really looking like it's > actually a fix that needs to be back-patched to 9.6 rather than a PG10 > open item. I don't have any issue with keeping it as an open item > though, just mentioning it. I'll provide another status update on or > before Monday, July 31st. This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Freeze on Cygwin w/ concurrency
On Mon, Mar 20, 2017 at 11:47:03PM -0400, Noah Misch wrote: > "pgbench -i -s 50; pgbench -S -j2 -c16 -T900 -P5" freezes consistently on > Cygwin 2.2.1 and Cygwin 2.6.0. (I suspect most other versions are affected.) > I've pinged[1] the Cygwin bug thread with some additional detail. The problem was cygserver thread exhaustion; cygserver needs a thread per simultaneous waiter. With "cygserver -r 40" or the equivalent config file setting, this test does not freeze. Cygwin 2.8.0 introduced a change to dynamically grow the thread count: https://cygwin.com/git/gitweb.cgi?p=newlib-cygwin.git;a=commitdiff;h=0b73dba4de3fdadde499edfbc7ca9d9a01c11487 However, Cygwin 2.8.0 introduced another source of cygserver freezes: https://cygwin.com/git/gitweb.cgi?p=newlib-cygwin.git;a=commitdiff;h=b80b2c011936f7f075b76b6e59f9e8a5ec49caa1 The 2.8.0-specific freezes have no known workaround. Cygwin 2.8.1 works, having reverted the problem commit. Do not use PostgreSQL with Cygwin 2.8.0. > If a Cygwin > buildfarm member starts using --enable-tap-tests, you may see failures in the > pgbench test suite. (lorikeet used --enable-tap-tests from 2017-03-18 to > 2017-03-20, but it failed before reaching the pgbench test suite.) Curious > that "make check" has too little concurrency to see more effects from this. I now understand the bug required eleven concurrent lock waiters, and it's plausible that "make check" doesn't experience that. The pgbench test suite uses -c5, so I expect it to be stable on almost any Cygwin. -- 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] segfault in HEAD when too many nested functions call
On Fri, Jul 28, 2017 at 02:42:06PM -0400, Robert Haas wrote: > On Fri, Jul 28, 2017 at 12:29 PM, Noah Misch wrote: > > Your colleagues achieve compliance despite uncertainty; for inspiration, I > > recommend examining Alvaro's status updates as examples of this. The policy > > currently governs your open items even if you disagree with it. Thanks for resolving this open item. > I emphatically agree with that. If the RMT is to accomplish its > purpose, it must be able to exert authority even when an individual > contributor doesn't like the decisions it makes. > > On the other hand, nothing in the open item policy the current RMT has > adopted prohibits you from using judgement about when and how > vigorously to enforce that policy in any particular case, and I would > encourage you to do so. I understand. When it comes to open item regulation, the aspects that keep me up at night are completeness and fairness. Minimizing list traffic about non-compliant open items is third priority at best. Furthermore, it takes an expensive and subjective calculation to determine whether a policy-violating open item is progressing well. I will keep an eye out for minor policy violations that I can ignore cheaply and fairly. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [BUGS] BUG #14758: Segfault with logical replication on a function index
On Mon, Jul 31, 2017 at 09:40:34AM +0900, Masahiko Sawada wrote: > On Sat, Jul 29, 2017 at 4:35 AM, Scott Milliken wrote: > > Thank you Masahiko! I've tested and confirmed that this patch fixes the > > problem. > > > > Thank you for the testing. This issue should be added to the open item > since this cause of the server crash. I'll add it. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Peter, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Subscription code improvements
On Fri, Jul 07, 2017 at 10:19:19PM +0200, Petr Jelinek wrote: > I have done some review of subscription handling (well self-review) and > here is the result of that (It's slightly improved version from another > thread [1]). > Only the 0002, 0004 and 0005 are actual bug fixes, but I'd still like to > get it all into PG10 as especially the locking now behaves really > differently than everything else and that does not seem like a good idea. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Peter, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] PL_stashcache, or, what's our minimum Perl version?
On Sun, Jul 30, 2017 at 12:05:10PM -0400, Tom Lane wrote: > Noah Misch writes: > > On Sun, Jul 30, 2017 at 01:21:28AM -0400, Tom Lane wrote: > >> I think it'd be a good idea to insist that "prove" be in > >> the same directory we found "perl" in. > > > Nah; on my machines, I use /usr/bin/perl and ~/sw/cpan/bin/prove. The > > latter > > is built against the former, so there's no particular hazard. > > Well, OK, but I'd still like to tweak configure so that it records > an absolute path for prove rather than just setting PROVE=prove. > That way you'd at least be able to tell from the configure log > whether you are possibly at risk. That's an improvement. -- 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_stashcache, or, what's our minimum Perl version?
On Sun, Jul 30, 2017 at 01:21:28AM -0400, Tom Lane wrote: > I wrote: > > So the question is, does anyone care? I wouldn't except that our > > documentation appears to claim that we work with Perl "5.8 or later". > > And the lack of field complaints suggests strongly that nobody else > > cares. So I'm inclined to think we just need to be more specific > > about the minimum Perl version --- but what exactly? > > I've done some more research on this. It seems to me there are four > distinct components to any claim about whether we work with a particular > Perl version: > > 1. Can it run the build-related Perl scripts needed to build PG from > a bare git checkout, on non-Windows platforms? > 2. Can it run our MSVC build scripts? > 3. Does pl/perl compile (and pass its regression tests) against it? > 4. Can it run our TAP tests? > 5.8.3 does appear to succeed at points 1,3,4. Now, to get through > the TAP tests you need to install IPC::Run, and you also need to > update Test::More because the version shipped with 5.8.3 is too old. > But at least the failures that you get from lacking these are pretty > clear. > Anyway, pending some news about compatibility of the MSVC scripts, > I think we ought to adjust our docs to state that 5.8.3 is the > minimum supported Perl version. Works for me. I wouldn't wait for testing of the MSVC scripts. Strawberry Perl started with 5.8.8. ActivePerl is far older, but it distributes old versions to subscription holders only. Besides, the main advantage of old-version support is letting folks use a packaged/preinstalled binary, and that doesn't apply on Windows. > Also, I got seriously confused at one point during these tests because, > while our configure script carefully sets PERL to an absolute path name, > it's content to set PROVE to "prove", paying no attention to whether > that version of "prove" matches "perl". Is it really okay to run the > TAP tests with a different perl version than we selected for other > purposes? Typically yes, though one can construct exceptions. > I think it'd be a good idea to insist that "prove" be in > the same directory we found "perl" in. Nah; on my machines, I use /usr/bin/perl and ~/sw/cpan/bin/prove. The latter is built against the former, so there's no particular hazard. 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] segfault in HEAD when too many nested functions call
On Thu, Jul 27, 2017 at 10:08:57PM -0700, Andres Freund wrote: > On 2017-07-27 22:04:59 -0700, Noah Misch wrote: > > On Thu, Jul 27, 2017 at 09:49:18PM -0700, Andres Freund wrote: > > > On 2017-07-27 21:46:57 -0700, Noah Misch wrote: > > > > On Thu, Jul 27, 2017 at 02:29:32AM +, Noah Misch wrote: > > > > > On Mon, Jul 24, 2017 at 08:04:30AM +0100, Andres Freund wrote: > > > > > > > > > > > > > > > > > > On July 24, 2017 7:10:19 AM GMT+01:00, Noah Misch > > > > > > wrote: > > > > > > >On Tue, Jul 18, 2017 at 01:04:10PM -0700, Andres Freund wrote: > > > > > > >> Ok, I'll flesh out the patch till Thursday. But I do think we're > > > > > > >going > > > > > > >> to have to do something about the back branches, too. > > > > > > > > > > > > > >This PostgreSQL 10 open item is past due for your status update. > > > > > > >Kindly send > > > > > > >a status update within 24 hours, and include a date for your > > > > > > >subsequent > > > > > > >status > > > > > > >update. Refer to the policy on open item ownership: > > > > > > >https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > > > > > > > > > > > I sent out a note fleshed out patch last week, which Tom reviewed. > > > > > > Planning to update it to address that review today or tomorrow. > > > > > > > > > > This PostgreSQL 10 open item is past due for your status update. > > > > > Kindly send > > > > > a status update within 24 hours, and include a date for your > > > > > subsequent status > > > > > update. Refer to the policy on open item ownership: > > > > > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > > > > > > > IMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long > > > > past due > > > > for your status update. Please reacquaint yourself with the policy on > > > > open > > > > item ownership[1] and then reply immediately. If I do not hear from > > > > you by > > > > 2017-07-29 05:00 UTC, I will transfer this item to release management > > > > team > > > > ownership without further notice. > > > > > > > > [1] > > > > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > > > > > I've updated the patch based on review (today). Awaiting new review. > > > > > > FWIW, I don't see the point of these messages when there is a new patch > > > version posted today. > > > > The policy says, "Each update shall state a date when the community will > > receive another update". Nothing you've sent today specifies a deadline for > > your next update, so your ownership of this item remains out of > > compliance. > > For me that means the policy isn't quite right. It's not like I can > force Tom to review the patch at a specific date. But the thread has > been progressing steadily over the last days, so I'm not particularly > concerned. Your colleagues achieve compliance despite uncertainty; for inspiration, I recommend examining Alvaro's status updates as examples of this. The policy currently governs your open items even if you disagree with it. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: [bug fix] PG10: libpq doesn't connect to alternative hosts when some errors occur
On Fri, May 19, 2017 at 11:08:41AM +0900, Michael Paquier wrote: > On Fri, May 19, 2017 at 11:01 AM, Tsunakawa, Takayuki > wrote: > > From: pgsql-hackers-ow...@postgresql.org > >> [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Peter Eisentraut > >> The problem is that if we decide to change the behavior mid-beta, then > >> we'll > >> only have the rest of beta to find out whether people will like the other > >> behavior. > >> > >> I would aim for the behavior that is most suitable for refinement in the > >> future. The current behavior seems to match that. > > > > I think the pre-final release period is the very timing for refinement, in > > the perspective of users and PG developers as users. > > Sure that is the correct period to argue. We've reached that period. If anyone is going to push for a change here, now is the time. Absent such arguments, the behavior won't change. -- 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] Quorum commit for multiple synchronous replication.
On Thu, Apr 06, 2017 at 08:55:37AM +0200, Petr Jelinek wrote: > On 06/04/17 03:51, Noah Misch wrote: > > On Thu, Apr 06, 2017 at 12:48:56AM +0900, Fujii Masao wrote: > >> On Wed, Apr 5, 2017 at 3:45 PM, Noah Misch wrote: > >>> On Mon, Dec 19, 2016 at 09:49:58PM +0900, Fujii Masao wrote: > >>>> Regarding this feature, there are some loose ends. We should work on > >>>> and complete them until the release. > >>>> > >>>> (1) > >>>> Which synchronous replication method, priority or quorum, should be > >>>> chosen when neither FIRST nor ANY is specified in s_s_names? Right now, > >>>> a priority-based sync replication is chosen for keeping backward > >>>> compatibility. However some hackers argued to change this decision > >>>> so that a quorum commit is chosen because they think that most users > >>>> prefer to a quorum. > >> The items (1) and (3) are not bugs. So I don't think that they need to be > >> resolved before the beta release. After the feature freeze, many users > >> will try and play with many new features including quorum-based syncrep. > >> Then if many of them complain about (1) and (3), we can change the code > >> at that timing. So we need more time that users can try the feature. > > > > I've moved (1) to a new section for things to revisit during beta. If > > someone > > feels strongly that the current behavior is Wrong and must change, speak up > > as > > soon as you reach that conclusion. Absent such arguments, the behavior > > won't > > change. > > > > I was one of the people who said in original thread that I think the > default behavior should change to quorum and I am still of that opinion. This item appears under "decisions to recheck mid-beta". If anyone is going to push for a change here, now is the time. -- 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] map_partition_varattnos() and whole-row vars
On Wed, Jul 26, 2017 at 04:58:08PM +0900, Amit Langote wrote: > Rajkumar Raghuwanshi reported [1] on the "UPDATE partition key" thread > that whole-row vars don't play nicely with partitioned table operations. > > For example, when used to convert WITH CHECK OPTION constraint expressions > and RETURNING target list expressions, it will error out if the > expressions contained whole-row vars. That's a bug, because whole-row > vars are legal in those expressions. I think the decision to output error > upon encountering a whole-row in the input expression was based on the > assumption that it will only ever be used to convert partition constraint > expressions, which cannot contain those. So, we can relax that > requirement so that its other users are not bitten by it. > > Attached fixes that. > > Adding this to the PG 10 open items list. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Robert, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] segfault in HEAD when too many nested functions call
On Thu, Jul 27, 2017 at 09:49:18PM -0700, Andres Freund wrote: > On 2017-07-27 21:46:57 -0700, Noah Misch wrote: > > On Thu, Jul 27, 2017 at 02:29:32AM +0000, Noah Misch wrote: > > > On Mon, Jul 24, 2017 at 08:04:30AM +0100, Andres Freund wrote: > > > > > > > > > > > > On July 24, 2017 7:10:19 AM GMT+01:00, Noah Misch > > > > wrote: > > > > >On Tue, Jul 18, 2017 at 01:04:10PM -0700, Andres Freund wrote: > > > > >> Ok, I'll flesh out the patch till Thursday. But I do think we're > > > > >going > > > > >> to have to do something about the back branches, too. > > > > > > > > > >This PostgreSQL 10 open item is past due for your status update. > > > > >Kindly send > > > > >a status update within 24 hours, and include a date for your subsequent > > > > >status > > > > >update. Refer to the policy on open item ownership: > > > > >https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > > > > > > > I sent out a note fleshed out patch last week, which Tom reviewed. > > > > Planning to update it to address that review today or tomorrow. > > > > > > This PostgreSQL 10 open item is past due for your status update. Kindly > > > send > > > a status update within 24 hours, and include a date for your subsequent > > > status > > > update. Refer to the policy on open item ownership: > > > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > > > IMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long past due > > for your status update. Please reacquaint yourself with the policy on open > > item ownership[1] and then reply immediately. If I do not hear from you by > > 2017-07-29 05:00 UTC, I will transfer this item to release management team > > ownership without further notice. > > > > [1] > > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > I've updated the patch based on review (today). Awaiting new review. > > FWIW, I don't see the point of these messages when there is a new patch > version posted today. The policy says, "Each update shall state a date when the community will receive another update". Nothing you've sent today specifies a deadline for your next update, so your ownership of this item remains out of compliance. -- 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] segfault in HEAD when too many nested functions call
On Thu, Jul 27, 2017 at 02:29:32AM +, Noah Misch wrote: > On Mon, Jul 24, 2017 at 08:04:30AM +0100, Andres Freund wrote: > > > > > > On July 24, 2017 7:10:19 AM GMT+01:00, Noah Misch wrote: > > >On Tue, Jul 18, 2017 at 01:04:10PM -0700, Andres Freund wrote: > > >> Ok, I'll flesh out the patch till Thursday. But I do think we're > > >going > > >> to have to do something about the back branches, too. > > > > > >This PostgreSQL 10 open item is past due for your status update. > > >Kindly send > > >a status update within 24 hours, and include a date for your subsequent > > >status > > >update. Refer to the policy on open item ownership: > > >https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > > > I sent out a note fleshed out patch last week, which Tom reviewed. Planning > > to update it to address that review today or tomorrow. > > This PostgreSQL 10 open item is past due for your status update. Kindly send > a status update within 24 hours, and include a date for your subsequent status > update. Refer to the policy on open item ownership: > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com IMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long past due for your status update. Please reacquaint yourself with the policy on open item ownership[1] and then reply immediately. If I do not hear from you by 2017-07-29 05:00 UTC, I will transfer this item to release management team ownership without further notice. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] pg_stop_backup(wait_for_archive := true) on standby server
On Fri, Jul 21, 2017 at 07:04:32PM -0400, Stephen Frost wrote: > Masahiko, all, > > * Masahiko Sawada (sawada.m...@gmail.com) wrote: > > On Tue, Jul 18, 2017 at 1:28 PM, Stephen Frost wrote: > > > Masahiko, Michael, > > > > > > * Masahiko Sawada (sawada.m...@gmail.com) wrote: > > >> > This is beginning to shape. > > >> > > >> Sorry, I missed lots of typo in the last patch. All comments from you > > >> are incorporated into the attached latest patch and I've checked it > > >> whether there is other typos. Please review it. > > > > > > I've taken an initial look through the patch and it looks pretty > > > reasonable. I need to go over it in more detail and work through > > > testing it myself next. > > > > > > I expect to commit this (with perhaps some minor tweaks primairly around > > > punctuation/wording), barring any issues found, on Wednesday or Thursday > > > of this week. > > > > I understood. Thank you for looking at this! > > I started discussing this with David off-list and he'd like a chance to > review it in a bit more detail (he's just returning from being gone for > a few weeks). That review will be posted to this thread on Monday, and > I'll wait until then to move forward with the patch. > > Next update will be before Tuesday, July 25th, COB. This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] segfault in HEAD when too many nested functions call
On Mon, Jul 24, 2017 at 08:04:30AM +0100, Andres Freund wrote: > > > On July 24, 2017 7:10:19 AM GMT+01:00, Noah Misch wrote: > >On Tue, Jul 18, 2017 at 01:04:10PM -0700, Andres Freund wrote: > >> Ok, I'll flesh out the patch till Thursday. But I do think we're > >going > >> to have to do something about the back branches, too. > > > >This PostgreSQL 10 open item is past due for your status update. > >Kindly send > >a status update within 24 hours, and include a date for your subsequent > >status > >update. Refer to the policy on open item ownership: > >https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > I sent out a note fleshed out patch last week, which Tom reviewed. Planning > to update it to address that review today or tomorrow. This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] pl/perl extension fails on Windows
On Wed, Jul 19, 2017 at 05:01:31PM -0400, Tom Lane wrote: > Ashutosh Sharma writes: > > Here are the list of macros and variables from 'intrpvar.h' file that > > are just defined in perl module but not in plperl on Windows, > > > #ifdef PERL_USES_PL_PIDSTATUS > > PERLVAR(I, pidstatus, HV *) /* pid-to-status mappings for waitpid */ > > #endif > > > #ifdef PERL_SAWAMPERSAND > > PERLVAR(I, sawampersand, U8)/* must save all match strings */ > > #endif > > I am really suspicious that this means your libperl was built in an unsafe > fashion, that is, by injecting configuration choices as random -D switches > in the build process rather than making sure the choices were recorded in > perl's config.h. As an example, looking at the perl 5.24.1 headers on > a Fedora box, it looks to me like PERL_SAWAMPERSAND could only get defined > if PERL_COPY_ON_WRITE were not defined, and the only way that that can > happen is if PERL_NO_COW is defined, and there are no references to the > latter anyplace except in this particular #if defined test in perl.h. > > Where did your perl installation come from, anyway? Are you sure the .h > files match up with the executables? I see corresponding symptoms with the following Perl distributions: strawberry-perl-5.26.0.1-64bit.msi: src/pl/plperl/Util.c: loadable library and perl binaries are mismatched (got handshake key 11800080, needed 11c00080) ActivePerl-5.24.1.2402-MSWin32-x64-401627.exe: src/pl/plperl/Util.c: loadable library and perl binaries are mismatched (got handshake key 11500080, needed 11900080) So, this affects each of the two prominent families of Perl Windows binaries. Notes for anyone trying to reproduce: - Both of those Perl distributions require the hacks described here: https://www.postgresql.org/message-id/flat/CABcP5fjEjgOsh097cWnQrsK9yCswo4DZxp-V47DKCH-MxY9Gig%40mail.gmail.com - Add PERL_USE_UNSAFE_INC=1 to the environment until we update things to cope with this Perl 5.26 change: http://search.cpan.org/~xsawyerx/perl-5.26.0/pod/perldelta.pod#Removal_of_the_current_directory_(%22.%22)_from_@INC -- 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] segfault in HEAD when too many nested functions call
On Tue, Jul 18, 2017 at 01:04:10PM -0700, Andres Freund wrote: > Ok, I'll flesh out the patch till Thursday. But I do think we're going > to have to do something about the back branches, too. This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] segfault in HEAD when too many nested functions call
On Sat, Jul 15, 2017 at 11:22:37AM -0400, Tom Lane wrote: > Julien Rouhaud writes: > > Since b8d7f053c5c2bf2a7e8734fe3327f6a8bc711755 (Andres in Cc), if you > > write queries which result in infinite recursion (or just too many > > nested function calls), execution ends with segfault instead of intended > > exhausted max_stack_depth: > > Yes. We discussed this before the patch went in [1]. I wanted to put > a stack depth check in ExecProcNode(), and still do. Andres demurred, > claiming that that was too much overhead, but didn't really provide a > credible alternative. The thread drifted off without any resolution, > but clearly we need to do something before 10.0 final. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Andres, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] More race conditions in logical replication
On Wed, Jul 12, 2017 at 06:48:28PM -0400, Alvaro Herrera wrote: > Petr Jelinek wrote: > > > So best idea I could come up with is to make use of the new condition > > variable API. That lets us wait for variable which can be set per slot. > > Here's a cleaned up version of that patch, which I intend to get in the > tree tomorrow. I verified that this allows the subscription tests to > pass with Tom's sleep additions. This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Multi column range partition table
On Sun, Jul 09, 2017 at 08:42:32AM +0100, Dean Rasheed wrote: > On 6 July 2017 at 22:43, Joe Conway wrote: > > I agree we should get this right the first time and I also agree with > > Dean's proposal, so I guess I'm a +2 > > > > On 7 July 2017 at 03:21, Amit Langote wrote: > > +1 to releasing this syntax in PG 10. > > > > So, that's 3 votes in favour of replacing UNBOUNDED with > MINVALUE/MAXVALUE for range partition bounds in PG 10. Not a huge > consensus, but no objections either. Any one else have an opinion? > > Robert, have you been following this thread? > > I was thinking of pushing this later today, in time for beta2. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Robert, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: retry shm attach for windows (WAS: Re: OK, so culicidae is *still* broken)
On Mon, Jul 10, 2017 at 10:46:09AM -0400, Tom Lane wrote: > Magnus Hagander writes: > > On Jul 10, 2017 16:08, "Tom Lane" wrote: > >> Okay, so that leaves us with a decision to make: push it into beta2, or > >> wait till after wrap? I find it pretty scary to push a patch with > >> portability implications so soon before wrap, but a quick look at the > >> buildfarm suggests we'd get runs from 3 or 4 Windows members before the > >> wrap, if I do it quickly. If we wait, then it will hit the field in > >> production releases with reasonable buildfarm testing but little more. > >> That's also pretty scary. > >> On balance I'm tempted to push it now for beta2, but it's a close call. > >> Thoughts? > > > Given the lack of windows testing on non packaged releases I think we > > should definitely push this for beta2. That will give it a much better real > > world testing on what is still a beta. > > If it breaks its a lot better to do it in beta2 (and possibly quickly roll > > a beta3) than in production. > > Shall we go for broke and also remove the ASLR-disabling patch in beta2? > I do not feel a need to back-patch that one, at least not yet. But if > we're going to do it any time soon, a beta seems like the time. As I mentioned in my message eight hours ago, no. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: retry shm attach for windows (WAS: Re: OK, so culicidae is *still* broken)
On Mon, Jul 10, 2017 at 10:08:53AM -0400, Tom Lane wrote: > Noah Misch writes: > > I recommend pushing your patch so the August back-branch releases have it. > > One can see by inspection that your patch has negligible effect on systems > > healthy today. I have a reasonable suspicion it will help some systems. If > > others remain broken after this, that fact will provide a useful clue. > > Okay, so that leaves us with a decision to make: push it into beta2, or > wait till after wrap? I find it pretty scary to push a patch with > portability implications so soon before wrap, but a quick look at the > buildfarm suggests we'd get runs from 3 or 4 Windows members before the > wrap, if I do it quickly. If we wait, then it will hit the field in > production releases with reasonable buildfarm testing but little more. > That's also pretty scary. > > On balance I'm tempted to push it now for beta2, but it's a close call. > Thoughts? Pushing now for beta2 sounds good. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: retry shm attach for windows (WAS: Re: OK, so culicidae is *still* broken)
On Mon, Jun 05, 2017 at 09:56:33AM -0400, Tom Lane wrote: > Amit Kapila writes: > > Sure. I think it is slightly tricky because specs don't say clearly > > how ASLR can impact the behavior of any API and in my last attempt I > > could not reproduce the issue. > > > I can try to do basic verification with the patch you have proposed, > > but I fear, to do the actual tests as suggested by you is difficult as > > it is not reproducible on my machine, but I can still try. > > Yeah, being able to reproduce the problem reliably enough to say whether > it's fixed or not is definitely the sticking point here. I have some > ideas about that: > > 1. Be sure to use Win32 not Win64 --- the odds of a failure in the larger > address space are so small you'd never prove anything. (And of course > it has to be a version that has ASLR enabled.) > > 2. Revert 7f3e17b48 so that you have an ASLR-enabled build. > > 3. Crank shared_buffers to the maximum the machine will allow, reducing > the amount of free address space and improving the odds of a collision. > > 4. Spawn lots of sessions --- pgbench with -C option might be a useful > testing tool. > > With luck, that will get failures often enough that you can be pretty > sure whether a patch improves the situation or not. I tried this procedure without finding a single failure. Attributes: - 32-bit build of commit fad7873 w/ 7f3e17b48 reverted - confirmed ASLR-enabled with "dumpbin /headers postgres.exe" - OS = 64-bit Windows Server 2016 - compiler = Visual Studio 2015 Express - no config.pl, so not linked with any optional library - tried shared_buffers=1100M and shared_buffers=24M - echo 'select 1;' >pgbench-trivial; pgbench -n -f pgbench-trivial -C -c 30 -j5 -T900 - tried starting as a service at boot time, in addition to manual start === postgresql.conf === log_connections = on log_line_prefix = '%p %m ' autovacuum = off listen_addresses = '127.0.0.1' log_min_messages = debug1 max_connections = 40 shared_buffers = 24MB deadlock_timeout = '20ms' wal_buffers = '16MB' fsync = off I watched the ensuing memory maps, which led me to these interesting facts: An important result of the ASLR design in Windows Vista is that some address space layout parameters, such as PEB, stack, and heap locations, are selected once per program execution. Other parameters, such as the location of the program code, data segment, BSS segment, and libraries, change only between reboots. ... This offset is selected once per reboot, although we’ve uncovered at least one other way to cause this offset to be reset without a reboot (see Appendix II). -- http://www.symantec.com/avcenter/reference/Address_Space_Layout_Randomization.pdf So, reattach failures might be reproducible on some reboots and not others. While I had a few reboots during the test work, I did not exercise that dimension systematically. This information also implies we should not re-enable ASLR, even if your patch helps, due to addresses that change less than once per process creation but occasionally more than once per boot. I did try the combination of your patch and the following to simulate a 95% failure rate: --- a/src/backend/port/win32_shmem.c +++ b/src/backend/port/win32_shmem.c @@ -410,2 +410,4 @@ pgwin32_ReserveSharedMemoryRegion(HANDLE hChild) MEM_RESERVE, PAGE_READWRITE); + if (random() > MAX_RANDOM_VALUE / 20) + address = NULL; if (address == NULL) This confirmed retries worked. During a 900s test run, one connection attempt failed permanently by exhausting its 100 retries. The run achieved 11 connections per second. That is an order of magnitude slower than a run without simulated failures, but most applications would tolerate it. I recommend pushing your patch so the August back-branch releases have it. One can see by inspection that your patch has negligible effect on systems healthy today. I have a reasonable suspicion it will help some systems. If others remain broken after this, that fact will provide a useful clue. Thanks, nm -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Re: Oddity in error handling of constraint violation in ExecConstraints for partitioned tables
On Fri, Jul 07, 2017 at 06:47:26PM +0900, Amit Langote wrote: > On 2017/07/06 16:06, Etsuro Fujita wrote: > > Looks odd to me because the error message doesn't show any DETAIL info; > > since the CTE query, which produces the message, is the same as the above > > query, the message should also be the same as the one for the above > > query. > > I agree that the DETAIL should be shown. > The patch keeps tests that you added in your patch. Added this to the > open items list. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Robert, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] pg_stop_backup(wait_for_archive := true) on standby server
On Fri, Jun 30, 2017 at 02:59:11PM -0400, Stephen Frost wrote: > Peter, all, > > * Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote: > > On 6/30/17 04:08, Masahiko Sawada wrote: > > >> I'm not sure. I think this can be considered a bug in the implementation > > >> for > > >> 10, and as such is "open for fixing". However, it's not a very critical > > >> bug > > >> so I doubt it should be a release blocker, but if someone wants to work > > >> on a > > >> fix I think we should commit it. > > > > > > I agree with you. I'd like to hear opinions from other hackers as well. > > > > It's preferable to make it work. If it's not easily possible, then we > > should prohibit it. > > > > Comments from Stephen (original committer)? > > I agree that it'd be preferable to make it work, but I'm not sure I can > commit to having it done in short order. I'm happy to work to prohibit > it, but if someone has a few spare cycles to make it actually work, > that'd be great. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Stephen, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] hash index on unlogged tables doesn't behave as expected
On Mon, Jul 03, 2017 at 09:12:01AM +0530, Amit Kapila wrote: > While discussing the behavior of hash indexes with Bruce in the nearby > thread [1], it has been noticed that hash index on unlogged tables > doesn't behave as expected. Prior to 10, it has the different set of > problems (mainly because hash indexes are not WAL-logged) which were > discussed on that thread [1], however when I checked, it doesn't work > even for 10. Below are steps to reproduce the problem. > > 1. Setup master and standby > 2. On the master, create unlogged table and hash index. >2A. Create unlogged table t1(c1 int); >2B. Create hash index idx_t1_hash on t1 using hash(c1); > 3. On Standby, try selecting data, >select * from t1; >ERROR: cannot access temporary or unlogged relations during recovery > ---Till here everything works as expected > 4. Promote standby to master (I have just stopped the standby and > master and removed recovery.conf file from the standby database > location) and try starting the new master, it > gives below error and doesn't get started. > FATAL: could not create file "base/12700/16387": File exists > > The basic issue was that the WAL logging for Create Index operation > was oblivion of the fact that for unlogged tables only INIT forks need > to be logged. Another point which we need to consider is that while > replaying the WAL for the create index operation, we need to flush the > buffer if it is for init fork. This needs to be done only for pages > that can be part of init fork file (like metapage, bitmappage). > Attached patch fixes the issue. > > Another approach to fix the issue could be that always log complete > pages for the create index operation on unlogged tables (in > hashbuildempty). However, the logic for initial hash index pages > needs us to perform certain other actions (like update metapage after > the creation of bitmappage) which make it difficult to follow that > approach. > > I think this should be considered a PostgreSQL 10 open item. > > > [1] - https://www.postgresql.org/message-id/20170630005634.GA4448%40momjian.us [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Robert, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Reducing runtime of stats regression test
On Wed, May 03, 2017 at 11:43:43PM -0400, Tom Lane wrote:
> On a reasonably fast development machine, one of the biggest time sinks
> while running the core regression tests is the long "sleep" calls in the
> stats.sql regression test. I took a closer look at these, and I think
> we could basically get rid of them.
>
> First up is this bit at the beginning of that test script:
>
> -- wait to let any prior tests finish dumping out stats;
> -- else our messages might get lost due to contention
> SELECT pg_sleep_for('2 seconds');
>
> The stated concern isn't really all that plausible, since even if we
> launch a batch of test scripts at once, they don't all finish at once,
> so there's unlikely to be a big pileup of traffic to the stats collector.
> But we don't have to take that on faith: in assert-enabled builds,
> pgstat_send() logs any failure to send a stats message.
>
> I have grepped the buildfarm logs for "could not send to statistics
> collector" log messages during the "make check" stage (a total of 754313
> runs dating back ten years). What I find is that members mereswine and
> gull occasionally report "Network is down", and a few times currawong and
> thrips have complained "Invalid argument", and there are exactly no other
> such messages. In particular there are no EAGAIN or EWOULDBLOCK failures
> that would suggest congestion on the stats collector's input. So this
> is basically not something that happens at all in the regression tests,
> let alone during startup of the stats test in particular.
Linux and AIX, at least, do not make send() return an error when dropping a
UDP message for lack of queue space. Thus, the lack of buildfarm logs
reporting send() failure is inconclusive. Nonetheless, the lack of stats.sql
failures since you finished modifying this test in mid-May suggests this was a
good change. Besides, if a new member were slow enough to experience a stats
pileup, I wouldn't have firm hope that 2s would suffice.
--
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] More race conditions in logical replication
On Sun, Jul 02, 2017 at 07:54:48PM -0400, Tom Lane wrote: > I noticed a recent failure that looked suspiciously like a race condition: > > https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=hornet&dt=2017-07-02%2018%3A02%3A07 > > The critical bit in the log file is > > error running SQL: 'psql::1: ERROR: could not drop the replication > slot "tap_sub" on publisher > DETAIL: The error was: ERROR: replication slot "tap_sub" is active for PID > 3866790' > while running 'psql -XAtq -d port=59543 host=/tmp/QpCJtafT7R > dbname='postgres' -f - -v ON_ERROR_STOP=1' with sql 'DROP SUBSCRIPTION > tap_sub' at > /home/nm/farm/xlc64/HEAD/pgsql.build/src/test/subscription/../../../src/test/perl/PostgresNode.pm > line 1198. > > After poking at it a bit, I found that I can cause several different > failures of this ilk in the subscription tests by injecting delays at > the points where a slot's active_pid is about to be cleared, as in the > attached patch (which also adds some extra printouts for debugging > purposes; none of that is meant for commit). It seems clear that there > is inadequate interlocking going on when we kill and restart a logical > rep worker: we're trying to start a new one before the old one has > gotten out of the slot. > > I'm not particularly interested in fixing this myself, so I'm just > going to add it to the open items list. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Peter, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Race conditions with WAL sender PID lookups
On Mon, Jul 03, 2017 at 12:14:55PM -0400, Alvaro Herrera wrote: > Michael Paquier wrote: > > > In passing, clean up some leftover braces which were used to create > > > unconditional blocks. Once upon a time these were used for > > > volatile-izing accesses to those shmem structs, which is no longer > > > required. Many other occurrences of this pattern remain. > > > > Here are the places where a cleanup can happen: > > - WalSndSetState > > - ProcessStandbyReplyMessage > > - XLogRead, 2 places > > - XLogSendLogical > > - WalRcvWaitForStartPosition > > - WalRcvDie > > - XLogWalRcvFlush > > - ProcessWalSndrMessage > > In most of the places of the WAL sender, braces could be removed to > > improve the style. For the WAL receiver, declarations are not > > necessary. As a matter of style, why not cleaning up just the WAL > > sender stuff? Changing the WAL receiver code just to remove some > > declarations would not improve readability, and would make back-patch > > more difficult. > > I think we should clean this up whenever we're modifying the surrounding > code, but otherwise we can leave well enough alone. It's not a high > priority item at any rate. Bundling code cleanup into commits that also do something else is strictly worse than bundling whitespace cleanup, which is itself bad: https://postgr.es/m/flat/20160113144826.gb3379...@tornado.leadboat.com Deferring cleanup and pushing cleanup-only commits are each good options. -- 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] Code quality issues in ICU patch
On Sun, Jun 25, 2017 at 09:28:51PM -0700, Noah Misch wrote: > On Sat, Jun 24, 2017 at 10:03:25AM -0400, Peter Eisentraut wrote: > > On 6/23/17 12:31, Tom Lane wrote: > > > icu_to_uchar() and icu_from_uchar(), and perhaps other places, are > > > touchingly naive about integer overflow hazards in buffer size > > > calculations. I call particular attention to this bit in > > > icu_from_uchar(): > > > > > > len_result = UCNV_GET_MAX_BYTES_FOR_STRING(len_uchar, > > > ucnv_getMaxCharSize(icu_converter)); > > > > > > The ICU man pages say that that macro is defined as > > > > > > #define UCNV_GET_MAX_BYTES_FOR_STRING(length, maxCharSize) > > > (((int32_t)(length)+10)*(int32_t)(maxCharSize)) > > > > > > which means that getting this to overflow (resulting in > > > probably-exploitable memory overruns) would be about as hard as taking > > > candy from a baby. > > > > Here is a patch that should address this. > > [Action required within three days. This is a generic notification.] > > The above-described topic is currently a PostgreSQL 10 open item. Peter, > since you committed the patch believed to have created it, you own this open > item. If some other commit is more relevant or if this does not belong as a > v10 open item, please let us know. Otherwise, please observe the policy on > open item ownership[1] and send a status update within three calendar days of > this message. Include a date for your subsequent status update. Testers may > discover new open items at any time, and I want to plan to get them all fixed > well in advance of shipping v10. Consequently, I will appreciate your efforts > toward speedy resolution. Thanks. > > [1] > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] gen_random_uuid security not explicit in documentation
On Sun, Jun 25, 2017 at 09:26:28PM -0700, Noah Misch wrote: > On Fri, Jun 23, 2017 at 10:23:36AM +0900, Michael Paquier wrote: > > On Fri, Jun 23, 2017 at 3:02 AM, Heikki Linnakangas wrote: > > > I'm inclined to change gen_random_uuid() to throw an error if the server > > > is > > > built with --disable-strong-random, like gen_random_bytes() does. That > > > way, > > > they would behave the same. > > > > No objections to do that. I guess you don't need a patch. As this is > > new to 10, I have added an open item. > > [Action required within three days. This is a generic notification.] > > The above-described topic is currently a PostgreSQL 10 open item. Heikki, > since you committed the patch believed to have created it, you own this open > item. If some other commit is more relevant or if this does not belong as a > v10 open item, please let us know. Otherwise, please observe the policy on > open item ownership[1] and send a status update within three calendar days of > this message. Include a date for your subsequent status update. Testers may > discover new open items at any time, and I want to plan to get them all fixed > well in advance of shipping v10. Consequently, I will appreciate your efforts > toward speedy resolution. Thanks. > > [1] > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Race conditions with WAL sender PID lookups
On Wed, Jun 28, 2017 at 06:52:14PM -0400, Alvaro Herrera wrote: > I think I'm done with the walsender half of this patch; I still need to > review the walreceiver part. I will report back tomorrow 19:00 CLT. This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Get stuck when dropping a subscription during synchronizing table
On Wed, Jun 28, 2017 at 03:22:18AM +, Noah Misch wrote: > On Fri, Jun 23, 2017 at 09:42:10PM -0400, Peter Eisentraut wrote: > > On 6/21/17 22:47, Peter Eisentraut wrote: > > > On 6/20/17 22:44, Noah Misch wrote: > > >>> A patch has been posted, and it's being reviewed. Next update Monday. > > >> > > >> This PostgreSQL 10 open item is past due for your status update. Kindly > > >> send > > >> a status update within 24 hours, and include a date for your subsequent > > >> status > > >> update. Refer to the policy on open item ownership: > > >> https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > > > > > I'm not sure how to proceed here. Nobody is else seems terribly > > > interested, and this is really a minor issue, so I don't want to muck > > > around with the locking endlessly. Let's say, if there are no new > > > insights by Friday, I'll pick one of the proposed patches or just close > > > it without any patch. > > > > After some comments, a new patch has been posted, and I'll give until > > Monday for review. > > This PostgreSQL 10 open item is past due for your status update. Kindly send > a status update within 24 hours, and include a date for your subsequent status > update. Refer to the policy on open item ownership: > https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com IMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long past due for your status update. Please reacquaint yourself with the policy on open item ownership[1] and then reply immediately. If I do not hear from you by 2017-07-01 04:00 UTC, I will transfer this item to release management team ownership without further notice. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Get stuck when dropping a subscription during synchronizing table
On Fri, Jun 23, 2017 at 09:42:10PM -0400, Peter Eisentraut wrote: > On 6/21/17 22:47, Peter Eisentraut wrote: > > On 6/20/17 22:44, Noah Misch wrote: > >>> A patch has been posted, and it's being reviewed. Next update Monday. > >> > >> This PostgreSQL 10 open item is past due for your status update. Kindly > >> send > >> a status update within 24 hours, and include a date for your subsequent > >> status > >> update. Refer to the policy on open item ownership: > >> https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.com > > > > I'm not sure how to proceed here. Nobody is else seems terribly > > interested, and this is really a minor issue, so I don't want to muck > > around with the locking endlessly. Let's say, if there are no new > > insights by Friday, I'll pick one of the proposed patches or just close > > it without any patch. > > After some comments, a new patch has been posted, and I'll give until > Monday for review. This PostgreSQL 10 open item is past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. Refer to the policy on open item ownership: https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] transition table behavior with inheritance appears broken
On Sat, Jun 24, 2017 at 10:57:49PM -0700, Noah Misch wrote: > On Fri, Jun 23, 2017 at 02:39:48PM +0100, Andrew Gierth wrote: > > >>>>> "Noah" == Noah Misch writes: > > > > Noah> This PostgreSQL 10 open item is past due for your status update. > > Noah> Kindly send a status update within 24 hours, > > > > oops, sorry! I forgot to include a date in the last one, and in fact a > > personal matter delayed things anyway. I expect to have this wrapped up > > by 23:59 BST on the 24th. > > This PostgreSQL 10 open item is again past due for your status update. Kindly > send a status update within 24 hours, and include a date for your subsequent > status update. IMMEDIATE ATTENTION REQUIRED. This PostgreSQL 10 open item is long past due for your status update. Please reacquaint yourself with the policy on open item ownership[1] and then reply immediately. If I do not hear from you by 2017-06-28 06:00 UTC, I will transfer this item to release management team ownership without further notice. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] Code quality issues in ICU patch
On Sat, Jun 24, 2017 at 10:03:25AM -0400, Peter Eisentraut wrote: > On 6/23/17 12:31, Tom Lane wrote: > > icu_to_uchar() and icu_from_uchar(), and perhaps other places, are > > touchingly naive about integer overflow hazards in buffer size > > calculations. I call particular attention to this bit in > > icu_from_uchar(): > > > > len_result = UCNV_GET_MAX_BYTES_FOR_STRING(len_uchar, > > ucnv_getMaxCharSize(icu_converter)); > > > > The ICU man pages say that that macro is defined as > > > > #define UCNV_GET_MAX_BYTES_FOR_STRING(length, maxCharSize) > > (((int32_t)(length)+10)*(int32_t)(maxCharSize)) > > > > which means that getting this to overflow (resulting in > > probably-exploitable memory overruns) would be about as hard as taking > > candy from a baby. > > Here is a patch that should address this. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Peter, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] gen_random_uuid security not explicit in documentation
On Fri, Jun 23, 2017 at 10:23:36AM +0900, Michael Paquier wrote: > On Fri, Jun 23, 2017 at 3:02 AM, Heikki Linnakangas wrote: > > I'm inclined to change gen_random_uuid() to throw an error if the server is > > built with --disable-strong-random, like gen_random_bytes() does. That way, > > they would behave the same. > > No objections to do that. I guess you don't need a patch. As this is > new to 10, I have added an open item. [Action required within three days. This is a generic notification.] The above-described topic is currently a PostgreSQL 10 open item. Heikki, since you committed the patch believed to have created it, you own this open item. If some other commit is more relevant or if this does not belong as a v10 open item, please let us know. Otherwise, please observe the policy on open item ownership[1] and send a status update within three calendar days of this message. Include a date for your subsequent status update. Testers may discover new open items at any time, and I want to plan to get them all fixed well in advance of shipping v10. Consequently, I will appreciate your efforts toward speedy resolution. Thanks. [1] https://www.postgresql.org/message-id/20170404140717.GA2675809%40tornado.leadboat.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] transition table behavior with inheritance appears broken
On Fri, Jun 23, 2017 at 02:39:48PM +0100, Andrew Gierth wrote: > >>>>> "Noah" == Noah Misch writes: > > Noah> This PostgreSQL 10 open item is past due for your status update. > Noah> Kindly send a status update within 24 hours, > > oops, sorry! I forgot to include a date in the last one, and in fact a > personal matter delayed things anyway. I expect to have this wrapped up > by 23:59 BST on the 24th. This PostgreSQL 10 open item is again past due for your status update. Kindly send a status update within 24 hours, and include a date for your subsequent status update. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
