Re: [HACKERS] How about a option to disable autovacuum cancellation on lock conflict?

[Jim Nasby]

On 11/29/14, 2:22 AM, Andres Freund wrote:

Hi,

I've more than once seen that autovacuums on certain tables never
succeed because regular exclusive (or similar) lockers cause it to give
way/up before finishing.  Usually if some part of the application uses
explicit exclusive locks.

In general I think it's quite imortant that autovacuum bheaves that
way. But I think it might be worhtwile to offer an option to disable
that behaviour. If some piece of application logic requires exclusive
locks and that leads to complete starvation of autovacuum, there's
really nothing that can be done but to manually schedule vacuums right
now.

I can see two possible solutions:

1) Add a reloption that allows to configure whether autovacuum gives way
to locks acquired by user backends.
2) Add a second set of autovacuum_*_scale_factor variables that governs
a threshhold after which autovacuum doesn't get cancelled anymore.

Opinions?


What do you mean by "never succeed"? Is it skipping a large number of pages? 
Might re-trying the locks within the same vacuum help, or are the user locks too 
persistent?
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.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] [COMMITTERS] pgsql: Revert "Add libpq function PQhostaddr()."

[Tom Lane]

David G Johnston  writes:
> Noah Misch-2 wrote
>> I had considered that, and one could make a reasonable case for living
>> with the new symbol on that basis.  For the release candidate stage to
>> have value, though, the "treat as released" principle must not be
>> absolute.  I regret not noticing the problem earlier.

> Then the question becomes whether any backward incompatibility introduced in
> an RC release necessitates another RC release instead of going live with the
> next revision.  I'll go with the idea that all betas and RC versions can
> have API changes but the release immediately preceding the actual release
> should not be allowed to do so.  People who have tested their code on the
> most recent RC prior to release should be assured that when the final
> version is released that all API testing done previously is good.

> The RC is final release phase that our vendor community can use to polish
> their applications before end users are able to download the functionally
> equivalent final release.  We do not want to force our vendors to have to
> finalize their 9.4 support in the short period between announcing the final
> release and it being made available in repositories.

> It is just as valid to insist that there will only be a single RC in which
> case API changes must not occur.  But unles the community feels too burdened
> but an unbounded RC policy the ability to fix problems even during the RC
> phase seems valuable.  These should be minor things so the biggest cost is
> the added RC release and not so much that applications end to be modified.

Well, personally, as one of the people on whom the burden of an extra RC
release would fall, let me make it clear that there will *not* be an RC2
release just for this.  If the community consensus is that this change
would require us to do an RC2, it's going to get reverted.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Securing "make check" (CVE-2014-0067)

[Noah Misch]

On Sun, Sep 21, 2014 at 02:31:15AM -0400, Noah Misch wrote:
> It then dawned on me that every Windows build of PostgreSQL already has a way
> to limit connections to a particular OS user.  SSPI authentication is
> essentially the Windows equivalent of peer authentication.  A brief trial
> thereof looked promising.  Regression runs will need a pg_ident.conf listing
> each role used in the regression tests.  That's not ideal, but the buildfarm
> will quickly reveal any omissions.  Unless someone sees a problem here, I will
> look at fleshing this out into a complete patch.  I bet it will even turn out
> to be back-patchable.

That worked out nicely.  "pg_regress --temp-install" rewrites pg_ident.conf
and pg_hba.conf such that the current OS user may authenticate as the
bootstrap superuser and as any user named in --create-role.  Suites not using
--temp-install (pg_upgrade, TAP) call "pg_regress --config-auth=DATADIR" to
pick up those same configuration changes.  My hope is that out-of-tree test
harnesses wanting this hardening can do likewise.  On non-Windows systems,
"pg_regress --config-auth" does nothing.

The TAP suite did not and does not succeed on Windows.  I have good confidence
in my changes to make it use SSPI, but I tested them fully on GNU/Linux only.

Adding the explicit PGHOST=localhost to the pg_upgrade test suite is necessary
to avoid the "host name must be specified" error under SSPI authentication.  I
tentatively view that as a bug in libpq, but it's orthogonal to this patch.
pg_regress.c already sets PGHOST explicitly.

Since I was rewriting various test suite "initdb" calls anyway, I made a few
use "-N" that weren't using it previously.

Thanks,
nm
diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh
index 7bbd2c7..3bda790 100644
--- a/contrib/pg_upgrade/test.sh
+++ b/contrib/pg_upgrade/test.sh
@@ -17,13 +17,20 @@ set -e
 unset MAKEFLAGS
 unset MAKELEVEL
 
+# Run a given "initdb" binary and overlay the regression testing
+# authentication configuration.
+standard_initdb() {
+   "$1" -N
+   ../../src/test/regress/pg_regress --config-auth "$PGDATA"
+}
+
 # Establish how the server will listen for connections
 testhost=`uname -s`
 
 case $testhost in
MINGW*)
LISTEN_ADDRESSES="localhost"
-   PGHOST=""; unset PGHOST
+   PGHOST=localhost
;;
*)
LISTEN_ADDRESSES=""
@@ -49,11 +56,11 @@ case $testhost in
trap 'rm -rf "$PGHOST"' 0
trap 'exit 3' 1 2 13 15
fi
-   export PGHOST
;;
 esac
 
 POSTMASTER_OPTS="-F -c listen_addresses=$LISTEN_ADDRESSES -k \"$PGHOST\""
+export PGHOST
 
 temp_root=$PWD/tmp_check
 
@@ -141,7 +148,7 @@ export EXTRA_REGRESS_OPTS
 # enable echo so the user can see what is being executed
 set -x
 
-"$oldbindir"/initdb -N
+standard_initdb "$oldbindir"/initdb
 "$oldbindir"/pg_ctl start -l "$logdir/postmaster1.log" -o "$POSTMASTER_OPTS" -w
 if "$MAKE" -C "$oldsrc" installcheck; then
pg_dumpall -f "$temp_root"/dump1.sql || pg_dumpall1_status=$?
@@ -181,7 +188,7 @@ fi
 
 PGDATA=$BASE_PGDATA
 
-initdb -N
+standard_initdb 'initdb'
 
 pg_upgrade $PG_UPGRADE_OPTS -d "${PGDATA}.old" -D "${PGDATA}" -b "$oldbindir" 
-B "$bindir" -p "$PGPORT" -P "$PGPORT"
 
diff --git a/doc/src/sgml/regress.sgml b/doc/src/sgml/regress.sgml
index 71196a1..504d8da 100644
--- a/doc/src/sgml/regress.sgml
+++ b/doc/src/sgml/regress.sgml
@@ -56,19 +56,6 @@ make check
failure represents a serious problem.
   
 
-  
-   
-On systems lacking Unix-domain sockets, notably Windows, this test method
-starts a temporary server configured to accept any connection originating
-on the local machine.  Any local user can gain database superuser
-privileges when connecting to this server, and could in principle exploit
-all privileges of the operating-system user running the tests.  Therefore,
-it is not recommended that you use make check on an affected
-system shared with untrusted users.  Instead, run the tests after
-completing the installation, as described in the next section.
-   
-  
-

 Because this test method runs a temporary server, it will not work
 if you did the build as the root user, since the server will not start as
diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 63ff50b..2743c8f 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -320,7 +320,7 @@ endef
 define prove_check
 $(MKDIR_P) tmp_check/log
 $(MAKE) -C $(top_builddir) DESTDIR='$(CURDIR)'/tmp_check/install install 
>'$(CURDIR)'/tmp_check/log/install.log 2>&1
-cd $(srcdir) && TESTDIR='$(CURDIR)' 
PATH="$(CURDIR)/tmp_check/install$(bindir):$$PATH" $(call 
add_to_path,$(ld_library_path_var),$(CURDIR)/tmp_check/install$(libdir)) 
PGPORT='6$(DEF_PGPORT)' $(PROVE) $(PG_PROVE_FLAGS) $(PROVE_FLAGS) t/*.pl
+cd $(srcdir) && TESTDIR='$(CURDIR)' 
PATH="$(CURDIR)/tmp_check/insta

[HACKERS] Determining typmod of *source* of a cast

[Jim Nasby]

Is there any way to determine the typemod of the source data for a cast? 
Perhaps a modification on get_call_expr_argtype(), though I'd hate to put that 
in an extension...

BTW, it'd be nice if we better emphasized that the typmod passed to a cast 
function is for the destination... :/
--
Jim Nasby, Data Architect, Blue Treble Consulting
Data in Trouble? Get it in Treble! http://BlueTreble.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] [COMMITTERS] pgsql: Revert "Add libpq function PQhostaddr()."

[David G Johnston]

Noah Misch-2 wrote
> On Sat, Nov 29, 2014 at 02:09:09PM -0500, Tom Lane wrote:
>> Noah Misch <

> noah@

> > writes:
>> > Revert "Add libpq function PQhostaddr()."
>> > This reverts commit 9f80f4835a55a1cbffcda5d23a617917f3286c14.  The
>> > function returned the raw value of a connection parameter, a task
>> served
>> > by PQconninfo().  The next commit will reimplement the psql \conninfo
>> > change that way.  Back-patch to 9.4, where that commit first appeared.
>> 
>> I confess to not having been paying too much attention to your discussion
>> with Fujii over the holiday, but isn't it a bit too late to be making
>> client-API-breaking changes in 9.4?  I would have been fine with this
>> before RC1 went out, but once we do that, the branch should be treated
>> as released.
> 
> I had considered that, and one could make a reasonable case for living
> with
> the new symbol on that basis.  For the release candidate stage to have
> value,
> though, the "treat as released" principle must not be absolute.  I regret
> not
> noticing the problem earlier.

Then the question becomes whether any backward incompatibility introduced in
an RC release necessitates another RC release instead of going live with the
next revision.  I'll go with the idea that all betas and RC versions can
have API changes but the release immediately preceding the actual release
should not be allowed to do so.  People who have tested their code on the
most recent RC prior to release should be assured that when the final
version is released that all API testing done previously is good.

The RC is final release phase that our vendor community can use to polish
their applications before end users are able to download the functionally
equivalent final release.  We do not want to force our vendors to have to
finalize their 9.4 support in the short period between announcing the final
release and it being made available in repositories.

It is just as valid to insist that there will only be a single RC in which
case API changes must not occur.  But unles the community feels too burdened
but an unbounded RC policy the ability to fix problems even during the RC
phase seems valuable.  These should be minor things so the biggest cost is
the added RC release and not so much that applications end to be modified.

David J.




--
View this message in context: 
http://postgresql.nabble.com/Re-COMMITTERS-pgsql-Revert-Add-libpq-function-PQhostaddr-tp5828660p5828672.html
Sent from the PostgreSQL - hackers mailing list archive at Nabble.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] [COMMITTERS] pgsql: Revert "Add libpq function PQhostaddr()."

[Tom Lane]

Noah Misch  writes:
> On Sat, Nov 29, 2014 at 02:09:09PM -0500, Tom Lane wrote:
>> I confess to not having been paying too much attention to your discussion
>> with Fujii over the holiday, but isn't it a bit too late to be making
>> client-API-breaking changes in 9.4?  I would have been fine with this
>> before RC1 went out, but once we do that, the branch should be treated
>> as released.

> I had considered that, and one could make a reasonable case for living with
> the new symbol on that basis.  For the release candidate stage to have value,
> though, the "treat as released" principle must not be absolute.  I regret not
> noticing the problem earlier.

I don't plan to go to war over this, but it's not apparent to me that
PQhostaddr was such a broken concept that we should risk library
compatibility issues post-RC1.  I will grant that *probably* there are
no users of the function yet, but why do we need to take the chance?
There are plenty of other access functions just like this one in libpq.
I think the bar for deciding that we can break compatibility at this
point is a lot higher than "well, maybe this isn't very useful".

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] Review of GetUserId() Usage

[Stephen Frost]

All,

* Stephen Frost (sfr...@snowman.net) wrote:
> * Stephen Frost (sfr...@snowman.net) wrote:
> > Attached is a patch to address the pg_cancel/terminate_backend and the
> > statistics info as discussed previously.  It sounds like we're coming to
> 
> And I forgot the attachment, of course.  Apologies.

Updated patch attached which also changes the error messages (which
hadn't been updated in the prior versions and really should be).

Barring objections, I plan to move forward with this one and get this
relatively minor change wrapped up.  As mentioned in the commit, while
this might be an arguably back-patchable change, the lack of field
complaints and the fact that it changes existing behavior mean it should
go only against master, imv.

Thanks,

Stephen
From bc1913464421921b7f846bcad332698ca6369e75 Mon Sep 17 00:00:00 2001
From: Stephen Frost 
Date: Sat, 29 Nov 2014 14:53:08 -0500
Subject: [PATCH] GetUserId() Cleanup for pg_stat and pg_signal

The pg_stat and pg_signal-related functions have been using GetUserId()
instead of has_privs_of_role() for checking if the current user should
be able to see see details in pg_stat_activity or signal other
processes.  This is arguably a bug in existing branches, but as it's a
user-visible change and we haven't gotten complaints about it, it's
probably best to just do it in master.

Per discussion with Alvaro, Peter and Adam (though not using Adam's
patch).
---
 src/backend/utils/adt/misc.c| 31 +++
 src/backend/utils/adt/pgstatfuncs.c | 19 ++-
 2 files changed, 33 insertions(+), 17 deletions(-)

diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
index 67539ec..f3dca1f 100644
--- a/src/backend/utils/adt/misc.c
+++ b/src/backend/utils/adt/misc.c
@@ -37,6 +37,7 @@
 #include "utils/lsyscache.h"
 #include "utils/ruleutils.h"
 #include "tcop/tcopprot.h"
+#include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/timestamp.h"
 
@@ -113,8 +114,16 @@ pg_signal_backend(int pid, int sig)
 		return SIGNAL_BACKEND_ERROR;
 	}
 
-	if (!(superuser() || proc->roleId == GetUserId()))
-		return SIGNAL_BACKEND_NOPERMISSION;
+	/* Only allow superusers to signal superuser-owned backends. */
+	if (superuser_arg(proc->roleId) && !superuser())
+		ereport(ERROR,
+(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+ (errmsg("permission denied to send signal to other process"),
+  errdetail("You must be a superuser to signal processes owned by superusers.";
+
+	/* Users can signal backends they have role membership in. */
+	if (!has_privs_of_role(GetUserId(), proc->roleId))
+			return SIGNAL_BACKEND_NOPERMISSION;
 
 	/*
 	 * Can the process we just validated above end, followed by the pid being
@@ -141,8 +150,10 @@ pg_signal_backend(int pid, int sig)
 }
 
 /*
- * Signal to cancel a backend process.  This is allowed if you are superuser or
- * have the same role as the process being canceled.
+ * Signal to cancel a backend process.  This is allowed if you are a member of
+ * the role whose process is being canceled.
+ *
+ * Note that only superusers can signal superuser-owned processes.
  */
 Datum
 pg_cancel_backend(PG_FUNCTION_ARGS)
@@ -152,14 +163,17 @@ pg_cancel_backend(PG_FUNCTION_ARGS)
 	if (r == SIGNAL_BACKEND_NOPERMISSION)
 		ereport(ERROR,
 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be superuser or have the same role to cancel queries running in other server processes";
+ (errmsg("permission denied to cancel query"),
+  errdetail("You must be a member of the role whose query you are cancelling.";
 
 	PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS);
 }
 
 /*
- * Signal to terminate a backend process.  This is allowed if you are superuser
- * or have the same role as the process being terminated.
+ * Signal to terminate a backend process.  This is allowed if you are a member
+ * of the role whose process is being terminated.
+ *
+ * Note that only superusers can signal superuser-owned processes.
  */
 Datum
 pg_terminate_backend(PG_FUNCTION_ARGS)
@@ -169,7 +183,8 @@ pg_terminate_backend(PG_FUNCTION_ARGS)
 	if (r == SIGNAL_BACKEND_NOPERMISSION)
 		ereport(ERROR,
 (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
- (errmsg("must be superuser or have the same role to terminate other server processes";
+ (errmsg("permission denied to terminate server process"),
+  errdetail("You must be a member of the role whose process you are terminating.";
 
 	PG_RETURN_BOOL(r == SIGNAL_BACKEND_SUCCESS);
 }
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index d621a68..3663e93 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -20,6 +20,7 @@
 #include "libpq/ip.h"
 #include "miscadmin.h"
 #include "pgstat.h"
+#include "utils/acl.h"
 #include "utils/builtins.h"
 #include "utils/inet.h"
 #include "utils/timestamp.h"
@@ -674,8 +675,8 @@ pg_stat_get_activity(PG_F

Re: [HACKERS] [COMMITTERS] pgsql: Revert "Add libpq function PQhostaddr()."

[Noah Misch]

On Sat, Nov 29, 2014 at 02:09:09PM -0500, Tom Lane wrote:
> Noah Misch  writes:
> > Revert "Add libpq function PQhostaddr()."
> > This reverts commit 9f80f4835a55a1cbffcda5d23a617917f3286c14.  The
> > function returned the raw value of a connection parameter, a task served
> > by PQconninfo().  The next commit will reimplement the psql \conninfo
> > change that way.  Back-patch to 9.4, where that commit first appeared.
> 
> I confess to not having been paying too much attention to your discussion
> with Fujii over the holiday, but isn't it a bit too late to be making
> client-API-breaking changes in 9.4?  I would have been fine with this
> before RC1 went out, but once we do that, the branch should be treated
> as released.

I had considered that, and one could make a reasonable case for living with
the new symbol on that basis.  For the release candidate stage to have value,
though, the "treat as released" principle must not be absolute.  I regret not
noticing the problem earlier.


-- 
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] [COMMITTERS] pgsql: Revert "Add libpq function PQhostaddr()."

[Tom Lane]

Andrew Dunstan  writes:
> Looks like this has broken the docs. See 
> 

That's because the cross-reference from the release notes wasn't removed.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] [COMMITTERS] pgsql: Revert "Add libpq function PQhostaddr()."

[Tom Lane]

Noah Misch  writes:
> Revert "Add libpq function PQhostaddr()."
> This reverts commit 9f80f4835a55a1cbffcda5d23a617917f3286c14.  The
> function returned the raw value of a connection parameter, a task served
> by PQconninfo().  The next commit will reimplement the psql \conninfo
> change that way.  Back-patch to 9.4, where that commit first appeared.

I confess to not having been paying too much attention to your discussion
with Fujii over the holiday, but isn't it a bit too late to be making
client-API-breaking changes in 9.4?  I would have been fine with this
before RC1 went out, but once we do that, the branch should be treated
as released.  And we would never consider removing a library API symbol
in a released branch.

I concur that PQhostaddr wasn't correctly documented, but maybe the
answer to that is to fix the documentation.

regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: [HACKERS] PATCH: decreasing memory needlessly consumed by array_agg

[Tomas Vondra]

Hi,

Attached is v2 of the patch lowering array_agg memory requirements.
Hopefully it addresses the issues issues mentioned by TL in this thread
(not handling some of the callers appropriately etc.).

The v2 of the patch does this:

* adds 'subcontext' flag to initArrayResult* methods

  If it's 'true' then a separate context is created for the
  ArrayBuildState instance, otherwise it's built within the parent
  context.

  Currently, only the array_agg_* functions pass 'subcontext=false' so
  that the array_agg() aggregate does not create separate context for
  each group. All other callers use 'true' and thus keep using the
  original implementation (this includes ARRAY_SUBLINK subplans, and
  various other places building array incrementally).

* adds 'release' flag to makeArrayResult

  This is mostly to make it consistent with makeArrayResultArr and
  makeArrayResultAny. All current callers use 'release=true'.

  Also, it seems that using 'subcontext=false' and then 'release=true'
  would be a bug. Maybe it would be appropriate to store the
  'subcontext' value into the ArrayBuildState and then throw an error
  if makeArrayResult* is called with (release=true && subcontext=false).

* modifies all the places calling those functions

* decreases number of preallocated elements to 8

  The original value was 64 (512B), the current value is 64B. (Not
  counting the 'nulls' array). More about this later ...


Now, some performance measurements - attached is a simple SQL script
that executes a few GROUP BY queries with various numbers of groups and
group elements. I ran the tests with two dataset sizes:

small
=
a) 1M groups, 1 item per group
b) 100k groups, 16 items per group
c) 100k groups, 64 items per group
d) 10k groups, 1024 items per group

large
=
a) 10M groups, 1 item per group
b) 1M groups, 16 items per group
c) 1M groups, 64 items per group
d) 100k groups, 1024 items per group

So essentially the 'large' dataset uses 10x the number of groups. The
results (average from the 5 runs, in ms) look like this:

small
=

test | master | patched | diff
-||-|---
   a |   1419 | 834 | -41%
   b |595 | 498 | -16%
   c |   2061 |1832 | -11%
   d |   2197 |1957 | -11%

large
=

test | master | patched | diff
-||-|---
   a |OOM |9144 |  n/a
   b |   7366 |6257 | -15%
   c |  29899 |   22940 | -23%
   d |  35456 |   31347 | -12%

So it seems to give solid speedup across the whole test suite - I'm yet
to find a case where it's actually slower than what we have now. The
test cases (b) and (c) were actually created with this goal, because
both should be OK with the original array size (64 elements), but with
the new size it requires a few repalloc() calls. But even those are much
faster.

This is most likely thanks to removing the AllocSetContextCreate call
and sharing freelists across groups (although the test cases don't seem
extremely suitable for that, as all the groups grow in parallel).

I even tried to bump the initial array size back to 64 elements, but the
performance actually decreased a bit for some reason. I have no idea why
this happens ...

The test script is attached - tweak the 'size' variable for different
dataset sizes. The (insane) work_mem sizes are used to force a hash
aggregate - clearly I don't have 1TB of RAM.

regards
Tomas
diff --git a/contrib/dblink/dblink.c b/contrib/dblink/dblink.c
index 18ae318..48e66bf 100644
--- a/contrib/dblink/dblink.c
+++ b/contrib/dblink/dblink.c
@@ -1271,13 +1271,13 @@ dblink_get_connections(PG_FUNCTION_ARGS)
 			/* stash away current value */
 			astate = accumArrayResult(astate,
 	  CStringGetTextDatum(hentry->name),
-	  false, TEXTOID, CurrentMemoryContext);
+	  false, TEXTOID, CurrentMemoryContext, true);
 		}
 	}
 
 	if (astate)
 		PG_RETURN_ARRAYTYPE_P(makeArrayResult(astate,
-			  CurrentMemoryContext));
+			  CurrentMemoryContext, true));
 	else
 		PG_RETURN_NULL();
 }
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index c16b38e..bd5eb32 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -671,7 +671,7 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
 /* No match, so keep old option */
 astate = accumArrayResult(astate, oldoptions[i],
 		  false, TEXTOID,
-		  CurrentMemoryContext);
+		  CurrentMemoryContext, true);
 			}
 		}
 	}
@@ -758,12 +758,12 @@ transformRelOptions(Datum oldOptions, List *defList, char *namspace,
 
 			astate = accumArrayResult(astate, PointerGetDatum(t),
 	  false, TEXTOID,
-	  CurrentMemoryContext);
+	  CurrentMemoryContext, true);
 		}
 	}
 
 	if (astate)
-		result = makeArrayResult(astate, CurrentMemoryContext);
+		result = makeArrayResult(astate, CurrentMemoryContext, true);
 	else
 		result = (Datum) 0;
 
diff --git a/src/

Re: [HACKERS] How about a option to disable autovacuum cancellation on lock conflict?

[Magnus Hagander]

On Nov 29, 2014 9:23 AM, "Andres Freund"  wrote:
>
> Hi,
>
> I've more than once seen that autovacuums on certain tables never
> succeed because regular exclusive (or similar) lockers cause it to give
> way/up before finishing.  Usually if some part of the application uses
> explicit exclusive locks.
>
> In general I think it's quite imortant that autovacuum bheaves that
> way. But I think it might be worhtwile to offer an option to disable
> that behaviour. If some piece of application logic requires exclusive
> locks and that leads to complete starvation of autovacuum, there's
> really nothing that can be done but to manually schedule vacuums right
> now.
>
> I can see two possible solutions:
>
> 1) Add a reloption that allows to configure whether autovacuum gives way
>to locks acquired by user backends.
> 2) Add a second set of autovacuum_*_scale_factor variables that governs
>a threshhold after which autovacuum doesn't get cancelled anymore.
>
> Opinions?

I definitely think having such a tunable would be very useful, in edge
cases (so as you say the default should definitely be what it is today).

Another "trigger option" could be to say "you may terminate autovaccum this
many times before forcing one through", rather than triggers on tuple
count. But tuples is probably a better choice, as it gives more dynamics -
unless we want to do both.

/Magnus

[HACKERS] Removing INNER JOINs

[David Rowley]

Hi,

Starting a new thread which continues on from
http://www.postgresql.org/message-id/caaphdvoec8ygwoahvsri-84en2k0tnh6gpxp1k59y9juc1w...@mail.gmail.com

To give a brief summary for any new readers:

The attached patch allows for INNER JOINed relations to be removed from the
plan,
providing none of the columns are used for anything, and a foreign key
exists which
proves that a record must exist in the table being removed which matches
the join
condition:

Example:

test=# create table b (id int primary key);
CREATE TABLE
test=# create table a (id int primary key, b_id int not null references
b(id));
CREATE TABLE
test=# explain (costs off) select a.* from a inner join b on a.b_id = b.id;
  QUERY PLAN
---
 Seq Scan on a

This has worked for a few years now for LEFT JOINs, so this patch is just
extending
the joins types that can be removed.

This optimisation should prove to be quite useful for views in which a
subset of its
columns are queried.

The attached is an updated patch which fixes a conflict from a recent
commit and
also fixes a bug where join removals did not properly work for PREPAREd
statements.

I'm looking for a bit of feedback around the method I'm using to prune the
redundant
plan nodes out of the plan tree at executor startup. Particularly around
not stripping
the Sort nodes out from below a merge join, even if the sort order is no
longer
required due to the merge join node being removed. This potentially could
leave
the plan suboptimal when compared to a plan that the planner could generate
when the removed relation was never asked for in the first place.

There are also other cases such as MergeJoins performing btree index scans
in order to obtain ordered results for a MergeJoin that would be better
executed
as a SeqScan when the MergeJoin can be removed.

Perhaps some costs could be adjusted at planning time when there's a
possibility
that joins could be removed at execution time, although I'm not quite sure
about
this as it risks generating a poor plan in the case when the joins cannot
be removed.

I currently can't see much of a way around these cases, but in both cases
removing
the join should prove to be a win, though just perhaps not with the most
optimal of
plans.

There are some more details around the reasons behind doing this weird
executor
startup plan pruning around here:

http://www.postgresql.org/message-id/20141006145957.ga20...@awork2.anarazel.de

Comments are most welcome

Regards

David Rowley


inner_join_removals_2014-11-29_be69869.patch
Description: Binary data

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

[HACKERS] How about a option to disable autovacuum cancellation on lock conflict?

[Andres Freund]

Hi,

I've more than once seen that autovacuums on certain tables never
succeed because regular exclusive (or similar) lockers cause it to give
way/up before finishing.  Usually if some part of the application uses
explicit exclusive locks.

In general I think it's quite imortant that autovacuum bheaves that
way. But I think it might be worhtwile to offer an option to disable
that behaviour. If some piece of application logic requires exclusive
locks and that leads to complete starvation of autovacuum, there's
really nothing that can be done but to manually schedule vacuums right
now.

I can see two possible solutions:

1) Add a reloption that allows to configure whether autovacuum gives way
   to locks acquired by user backends.
2) Add a second set of autovacuum_*_scale_factor variables that governs
   a threshhold after which autovacuum doesn't get cancelled anymore.

Opinions?

Greetings,

Andres Freund

-- 
 Andres Freund http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers