Re: [HACKERS] [PATCH] A hook for session start

[Jim Mlodgenski]

>
> > Can a user do anything remotely interesting or useful without hitting
> either
> > ExecutorStart_hook or ProcessUtility_hook? They can parse queries I guess
> > but you could just set your hook up in the parser instead. If you hook
> the
> > parser all they can do is open an idle session and sit there...
>
> That's an exceedingly-weak argument for rejecting this patch.  The
> fact that you can probably hack around the lack of a hook for most
> reasonable use cases is not an argument for having a hook that does
> what people actually want to do.
>
>
When I first saw this thread, my initial thought of a use case is to
prepare some key application queries so they are there and ready to go.
That would need to be before the ExecutorStart_hook or ProcessUtility_hook
if an app would just want to execute the prepared statement.

Re: [HACKERS] mat views stats

[Jim Mlodgenski]

>
>
> But that seems pretty ugly.  Given the lack of previous reports, I'm
> personally content to leave this unfixed in the back branches.
>
> Comments?
>
> Most instances of this I've seen out in the field have worked around this
by just running analyze in the scheduled jobs after the refresh  so we're
probably good not back patching.

Re: [HACKERS] mat views stats

[Jim Mlodgenski]

On Wed, Mar 1, 2017 at 8:39 PM, Michael Paquier <michael.paqu...@gmail.com>
wrote:

> On Thu, Mar 2, 2017 at 7:20 AM, Jim Mlodgenski <jimm...@gmail.com> wrote:
> >
> >
> > On Sun, Feb 26, 2017 at 11:49 AM, Robert Haas <robertmh...@gmail.com>
> wrote:
> >>
> >> On Wed, Feb 22, 2017 at 11:13 AM, Jim Nasby <jim.na...@bluetreble.com>
> >> wrote:
> >> > Certainly easier, but I don't think it'd be better. Matviews really
> >> > aren't
> >> > the same thing as tables. Off-hand (without reviewing the patch),
> update
> >> > and
> >> > delete counts certainly wouldn't make any sense. "Insert" counts
> might,
> >> > in
> >> > as much as it's how many rows have been added by refreshes. You'd
> want a
> >> > refresh count too.
> >>
> >> Regular REFRESH truncates the view and repopulates it, but REFRESH
> >> CONCURRENTLY does inserts, updates, and deletes as needed to adjust
> >> the contrs that make sense for
> >> regular tables are also sensible here.
> >>
> >
> > After digging into things further, just making refresh report the stats
> for
> > what is it basically doing simplifies and solves it and it is something
> we
> > can back patch if that the consensus. See the attached patch.
>
> This is unhappy:
> $ git diff master --check
> src/backend/commands/matview.c:155: indent with spaces.
> +uint64  processed = 0;
>
> +/*
> + * Send the stats to mimic what we are essentially doing.
> + * A truncate and insert
> + */
> This sentence is unfinished.
>
> There is also no need to report the number of inserts if WITH NO DATA is
> used.
>


Here is the cleaned up patch
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index a18c917..94a69dd 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -30,6 +30,7 @@
 #include "executor/spi.h"
 #include "miscadmin.h"
 #include "parser/parse_relation.h"
+#include "pgstat.h"
 #include "rewrite/rewriteHandler.h"
 #include "storage/lmgr.h"
 #include "storage/smgr.h"
@@ -59,7 +60,7 @@ static void transientrel_startup(DestReceiver *self, int operation, TupleDesc ty
 static bool transientrel_receive(TupleTableSlot *slot, DestReceiver *self);
 static void transientrel_shutdown(DestReceiver *self);
 static void transientrel_destroy(DestReceiver *self);
-static void refresh_matview_datafill(DestReceiver *dest, Query *query,
+static uint64 refresh_matview_datafill(DestReceiver *dest, Query *query,
 		 const char *queryString);
 
 static char *make_temptable_name_n(char *tempname, int n);
@@ -151,6 +152,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
 	Oid			save_userid;
 	int			save_sec_context;
 	int			save_nestlevel;
+	uint64  processed = 0;
 	ObjectAddress address;
 
 	/* Determine strength of lock needed. */
@@ -322,7 +324,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
 
 	/* Generate the data, if wanted. */
 	if (!stmt->skipData)
-		refresh_matview_datafill(dest, dataQuery, queryString);
+		processed = refresh_matview_datafill(dest, dataQuery, queryString);
 
 	heap_close(matviewRel, NoLock);
 
@@ -345,8 +347,18 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
 		Assert(matview_maintenance_depth == old_depth);
 	}
 	else
+	{
 		refresh_by_heap_swap(matviewOid, OIDNewHeap, relpersistence);
 
+		/*
+		 * Send the stats to mimic what we are essentially doing. Swapping the heap
+		 * is equilivant to truncating the relation and inserting the new data.
+		 */
+		pgstat_count_truncate(matviewRel);
+		if (!stmt->skipData)
+			pgstat_count_heap_insert(matviewRel, processed);
+	}
+
 	/* Roll back any GUC changes */
 	AtEOXact_GUC(false, save_nestlevel);
 
@@ -361,7 +373,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
 /*
  * refresh_matview_datafill
  */
-static void
+static uint64
 refresh_matview_datafill(DestReceiver *dest, Query *query,
 		 const char *queryString)
 {
@@ -369,6 +381,7 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 	PlannedStmt *plan;
 	QueryDesc  *queryDesc;
 	Query	   *copied_query;
+	uint64 processed;
 
 	/* Lock and rewrite, using a copy to preserve the original query. */
 	copied_query = copyObject(query);
@@ -406,6 +419,8 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 	/* run the plan */
 	ExecutorRun(queryDesc, ForwardScanDirection, 0L);
 
+	processed = queryDesc->estate->es_processed;
+
 	/* and clean up */
 	ExecutorFinish(queryDesc);
 	ExecutorEnd(queryDesc);
@@ -413,6 +428,8 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 	FreeQueryDesc(queryDesc);
 
 	PopActiveSnapshot();
+
+	return processed;
 }
 
 DestReceiver *

-- 
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] mat views stats

[Jim Mlodgenski]

On Sun, Feb 26, 2017 at 11:49 AM, Robert Haas  wrote:

> On Wed, Feb 22, 2017 at 11:13 AM, Jim Nasby 
> wrote:
> > Certainly easier, but I don't think it'd be better. Matviews really
> aren't
> > the same thing as tables. Off-hand (without reviewing the patch), update
> and
> > delete counts certainly wouldn't make any sense. "Insert" counts might,
> in
> > as much as it's how many rows have been added by refreshes. You'd want a
> > refresh count too.
>
> Regular REFRESH truncates the view and repopulates it, but REFRESH
> CONCURRENTLY does inserts, updates, and deletes as needed to adjust
> the contents.  So I think all the same counters that make sense for
> regular tables are also sensible here.
>
>
After digging into things further, just making refresh report the stats for
what is it basically doing simplifies and solves it and it is something we
can back patch if that the consensus. See the attached patch.
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index a18c917..4383312 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -30,6 +30,7 @@
 #include "executor/spi.h"
 #include "miscadmin.h"
 #include "parser/parse_relation.h"
+#include "pgstat.h"
 #include "rewrite/rewriteHandler.h"
 #include "storage/lmgr.h"
 #include "storage/smgr.h"
@@ -59,7 +60,7 @@ static void transientrel_startup(DestReceiver *self, int operation, TupleDesc ty
 static bool transientrel_receive(TupleTableSlot *slot, DestReceiver *self);
 static void transientrel_shutdown(DestReceiver *self);
 static void transientrel_destroy(DestReceiver *self);
-static void refresh_matview_datafill(DestReceiver *dest, Query *query,
+static uint64 refresh_matview_datafill(DestReceiver *dest, Query *query,
 		 const char *queryString);
 
 static char *make_temptable_name_n(char *tempname, int n);
@@ -151,6 +152,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
 	Oid			save_userid;
 	int			save_sec_context;
 	int			save_nestlevel;
+uint64  processed = 0;
 	ObjectAddress address;
 
 	/* Determine strength of lock needed. */
@@ -322,7 +324,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
 
 	/* Generate the data, if wanted. */
 	if (!stmt->skipData)
-		refresh_matview_datafill(dest, dataQuery, queryString);
+		processed = refresh_matview_datafill(dest, dataQuery, queryString);
 
 	heap_close(matviewRel, NoLock);
 
@@ -345,8 +347,17 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
 		Assert(matview_maintenance_depth == old_depth);
 	}
 	else
+{
 		refresh_by_heap_swap(matviewOid, OIDNewHeap, relpersistence);
 
+/* 
+ * Send the stats to mimic what we are essentially doing. 
+ * A truncate and insert 
+ */
+pgstat_count_truncate(matviewRel);
+pgstat_count_heap_insert(matviewRel, processed);
+}
+
 	/* Roll back any GUC changes */
 	AtEOXact_GUC(false, save_nestlevel);
 
@@ -361,7 +372,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
 /*
  * refresh_matview_datafill
  */
-static void
+static uint64 
 refresh_matview_datafill(DestReceiver *dest, Query *query,
 		 const char *queryString)
 {
@@ -369,6 +380,7 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 	PlannedStmt *plan;
 	QueryDesc  *queryDesc;
 	Query	   *copied_query;
+uint64 processed;
 
 	/* Lock and rewrite, using a copy to preserve the original query. */
 	copied_query = copyObject(query);
@@ -406,6 +418,8 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 	/* run the plan */
 	ExecutorRun(queryDesc, ForwardScanDirection, 0L);
 
+processed = queryDesc->estate->es_processed;
+
 	/* and clean up */
 	ExecutorFinish(queryDesc);
 	ExecutorEnd(queryDesc);
@@ -413,6 +427,8 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 	FreeQueryDesc(queryDesc);
 
 	PopActiveSnapshot();
+
+return processed;
 }
 
 DestReceiver *

-- 
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] mat views stats

[Jim Mlodgenski]

On Wed, Feb 22, 2017 at 12:43 AM, Jim Nasby 
wrote:

> On 2/21/17 4:22 PM, Peter Eisentraut wrote:
>
>> Attached is a patch to trigger autovacuum based on a matview refresh
>>> along with a system view pg_stat_all_matviews to show information more
>>> meaningful for materialized views.
>>>
>> It might be easier to include materialized views into pg_stat_*_tables.
>>
>
> Certainly easier, but I don't think it'd be better. Matviews really aren't
> the same thing as tables. Off-hand (without reviewing the patch), update
> and delete counts certainly wouldn't make any sense. "Insert" counts might,
> in as much as it's how many rows have been added by refreshes. You'd want a
> refresh count too.


Matviews already show up in the pg_stat_*_tables and the patch does
leverage the existing pg_stat_*_tables underlying structure, but it creates
more meaningful pg_stat_*_matviews leaving out things like insert and
update counts.


>
>
> I think these should be two separate patches.  We might want to
>> backpatch the first one.
>>
>
I was originally thinking 2 patches, but I couldn't think of a way to
trigger the analyze reliably without adding a refresh count or sending
bogus stats. We can certainly send a stats message containing the number of
rows inserted by the refresh, but are we going to also send the number of
deletes as well? Consider a matview that has month to date data. At the end
of the month, there will be about 30n live tuples. The next day on the new
month, there will be n inserts with the stats thinking there are 30n live
tuples which is below the analyze scale factor.  We want to analyze the
matview on the first of the day of the new month, but it wouldn't be
triggered for a few days. We can have REFRESH also track live tuples, but
it was quickly becoming a slippery slope of changing behavior for a back
patch. Maybe that's OK and we can go down that road.

We can back patch some documentation about the existing refresh behavior
with autovacuum.

[HACKERS] mat views stats

[Jim Mlodgenski]

I've come across a number of times where the statistics on materialized
views become stale producing bad plans. It turns out that autovacuum only
touches a materialized view when it is first created and ignores it on a
refresh. When you have a materialized view like yesterdays_sales the data
in the materialized view turns over every day.

Attached is a patch to trigger autovacuum based on a matview refresh along
with a system view pg_stat_all_matviews to show information more meaningful
for materialized views.

-- Jim
diff --git a/doc/src/sgml/monitoring.sgml b/doc/src/sgml/monitoring.sgml
index fad5cb0..ec27e2c 100644
--- a/doc/src/sgml/monitoring.sgml
+++ b/doc/src/sgml/monitoring.sgml
@@ -436,6 +436,21 @@ postgres   27093  0.0  0.0  30096  2752 ?Ss   11:34   0:00 postgres: ser
  
 
  
+  pg_stat_all_matviewspg_stat_all_matviews
+  
+   One row for each materialized view in the current database, showing statistics
+   about accesses to that specific materialized view.
+   See  for details.
+  
+ 
+
+ 
+  pg_stat_user_matviewspg_stat_user_matviews
+  Same as pg_stat_all_matviews, except that only
+  user materialized views are shown.
+ 
+
+ 
   pg_statio_all_tablespg_statio_all_tables
   
One row for each table in the current database, showing statistics
@@ -2277,6 +2292,97 @@ SELECT pid, wait_event_type, wait_event FROM pg_stat_activity WHERE wait_event i

   
 
+  
+   pg_stat_all_matviews View
+   
+
+
+  Column
+  Type
+  Description
+ 
+
+
+   
+
+ relid
+ oid
+ OID of this materialize view
+
+
+ schemaname
+ name
+ Name of the schema that this materialized view is in
+
+
+ relname
+ name
+ Name of this materialized view
+
+
+ seq_scan
+ bigint
+ Number of sequential scans initiated on this materialized view
+
+
+ seq_tup_read
+ bigint
+ Number of live rows fetched by sequential scans
+
+
+ idx_scan
+ bigint
+ Number of index scans initiated on this materialized ciew
+
+
+ idx_tup_fetch
+ bigint
+ Number of live rows fetched by index scans
+
+
+ last_refresh
+ timestamp with time zone
+ Last time at which this materialized view was refreshed
+
+
+ refresh_count
+ bigint
+ Number of times this materialized view has been refreshed
+
+
+ last_analyze
+ timestamp with time zone
+ Last time at which this materialized view was manually analyzed
+
+
+ last_autoanalyze
+ timestamp with time zone
+ Last time at which this materialized ciew was analyzed by the
+  autovacuum daemon
+
+
+ analyze_count
+ bigint
+ Number of times this materialized view has been manually analyzed
+
+
+ autoanalyze_count
+ bigint
+ Number of times this materialized view has been analyzed by the
+  autovacuum daemon
+
+   
+   
+  
+
+  
+   The pg_stat_all_matviews view will contain
+   one row for each materialized view in the current database, showing 
+   statistics about accesses to that specific materialized view. The
+   pg_stat_user_matviews contain the same 
+   information, but filtered to only show user materialized views.
+  
+
   
pg_statio_all_tables View

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 816483e..8d60d48 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -164,6 +164,15 @@ static relopt_int intRelOpts[] =
 		},
 		-1, 0, INT_MAX
 	},
+{
+{
+"autovacuum_analyze_refresh_threshold",
+"Minimum number of materialized view refreshes prior to analyze",
+RELOPT_KIND_HEAP,
+ShareUpdateExclusiveLock
+},
+-1, 0, INT_MAX
+},
 	{
 		{
 			"autovacuum_vacuum_cost_delay",
@@ -1283,6 +1292,8 @@ default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
 		offsetof(StdRdOptions, autovacuum) +offsetof(AutoVacOpts, vacuum_threshold)},
 		{"autovacuum_analyze_threshold", RELOPT_TYPE_INT,
 		offsetof(StdRdOptions, autovacuum) +offsetof(AutoVacOpts, analyze_threshold)},
+{"autovacuum_analyze_refresh_threshold", RELOPT_TYPE_INT,
+offsetof(StdRdOptions, autovacuum) +offsetof(AutoVacOpts, analyze_ref_threshold)},
 		{"autovacuum_vacuum_cost_delay", RELOPT_TYPE_INT,
 		offsetof(StdRdOptions, autovacuum) +offsetof(AutoVacOpts, vacuum_cost_delay)},
 		{"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 38be9cf..07b9f1c 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -535,6 +535,28 @@ CREATE VIEW 

Re: [HACKERS] PoC: Make it possible to disallow WHERE-less UPDATE and DELETE

[Jim Mlodgenski]

On Thu, Jul 21, 2016 at 12:57 AM, David Fetter  wrote:

> Folks,
>
> Please find attached a patch which makes it possible to disallow
> UPDATEs and DELETEs which lack a WHERE clause.  As this changes query
> behavior, I've made the new GUCs PGC_SUSET.
>
> What say?
>
>
Can't you implement this as a extension? The SQL Firewall project is
already doing some similar concepts by catching prohibiting SQL and
preventing it from executing.
https://github.com/uptimejp/sql_firewall

Re: Custom Scan APIs (Re: [HACKERS] Custom Plan node)

[Jim Mlodgenski]

KaiGai


On Tue, Nov 19, 2013 at 9:41 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:

 Thanks for your review.

 2013/11/19 Jim Mlodgenski jimm...@gmail.com:
  My initial review on this feature:
  - The patches apply and build, but it produces a warning:
  ctidscan.c: In function ‘CTidInitCustomScanPlan’:
  ctidscan.c:362:9: warning: unused variable ‘scan_relid’
 [-Wunused-variable]
 
 This variable was only used in Assert() macro, so it causes a warning if
 you
 don't put --enable-cassert on the configure script.
 Anyway, I adjusted the code to check relid of RelOptInfo directly.



The warning is now gone.


  I'd recommend that you split the part1 patch containing the ctidscan
 contrib
  into its own patch. It is more than half of the patch and its certainly
  stands on its own. IMO, I think ctidscan fits a very specific use case
 and
  would be better off being an extension instead of in contrib.
 
 OK, I split them off. The part-1 is custom-scan API itself, the part-2 is
 ctidscan portion, and the part-3 is remote join on postgres_fdw.


Attached is a patch for the documentation. I think the documentation still
needs a little more work, but it is pretty close. I can add some more
detail to it once finish adapting the hadoop_fdw to using the custom scan
api and have a better understanding of all of the calls.


 Thanks,
 --
 KaiGai Kohei kai...@kaigai.gr.jp

*** a/doc/src/sgml/custom-scan.sgml	2013-11-18 17:50:02.652039003 -0500
--- b/doc/src/sgml/custom-scan.sgml	2013-11-22 09:09:13.624254649 -0500
***
*** 8,47 
secondaryhandler for/secondary
   /indexterm
   para
!   Custom-scan API enables extension to provide alternative ways to scan or
!   join relations, being fully integrated with cost based optimizer,
!   in addition to the built-in implementation.
!   It consists of a set of callbacks, with a unique name, to be invoked during
!   query planning and execution. Custom-scan provider should implement these
!   callback functions according to the expectation of API.
   /para
   para
!   Overall, here is four major jobs that custom-scan provider should implement.
!   The first one is registration of custom-scan provider itself. Usually, it
!   shall be done once at literal_PG_init()/literal entrypoint on module
!   loading.
!   The other three jobs shall be done for each query planning and execution.
!   The second one is submission of candidate paths to scan or join relations,
!   with an adequate cost, for the core planner.
!   Then, planner shall chooses a cheapest path from all the candidates.
!   If custom path survived, the planner kicks the third job; construction of
!   literalCustomScan/literal plan node, being located within query plan
!   tree instead of the built-in plan node.
!   The last one is execution of its implementation in answer to invocations
!   by the core executor.
   /para
   para
!   Some of contrib module utilize the custom-scan API. It may be able to
!   provide a good example for new development.
variablelist
 varlistentry
  termxref linkend=ctidscan/term
  listitem
   para
!   Its logic enables to skip earlier pages or terminate scan prior to
!   end of the relation, if inequality operator on literalctid/literal
!   system column can narrow down the scope to be scanned, instead of
!   the sequential scan that reads a relation from the head to the end.
   /para
  /listitem
 /varlistentry
--- 8,46 
secondaryhandler for/secondary
   /indexterm
   para
!   The custom-scan API enables an extension to provide alternative ways to scan
!   or join relations leveraging the cost based optimizer. The API consists of a
!   set of callbacks, with a unique names, to be invoked during query planning 
!   and execution. A custom-scan provider should implement these callback 
!   functions according to the expectation of the API.
   /para
   para
!   Overall, there are four major tasks that a custom-scan provider should 
!   implement. The first task is the registration of custom-scan provider itself.
!   Usually, this needs to be done once at the literal_PG_init()/literal 
!   entrypoint when the module is loading. The remaing three tasks are all done
!   when a query is planning and executing. The second task is the submission of
!   candidate paths to either scan or join relations with an adequate cost for
!   the core planner. Then, the planner will choose the cheapest path from all of
!   the candidates. If the custom path survived, the planner starts the third 
!   task; construction of a literalCustomScan/literal plan node, located
!   within the query plan tree instead of the built-in plan node. The last task
!   is the execution of its implementation in answer to invocations by the core
!   executor.
   /para
   para
!   Some of contrib modules utilize the custom-scan API. They may provide a good
!   example for new development.
variablelist
 varlistentry
  termxref linkend=ctidscan/term

Re: Custom Scan APIs (Re: [HACKERS] Custom Plan node)

[Jim Mlodgenski]

On Mon, Nov 18, 2013 at 7:25 AM, Kohei KaiGai kai...@kaigai.gr.jp wrote:

 The attached patches are the revised custom-scan APIs.


My initial review on this feature:
- The patches apply and build, but it produces a warning:
ctidscan.c: In function ‘CTidInitCustomScanPlan’:
ctidscan.c:362:9: warning: unused variable ‘scan_relid’ [-Wunused-variable]

I'd recommend that you split the part1 patch containing the ctidscan
contrib into its own patch. It is more than half of the patch and its
certainly stands on its own. IMO, I think ctidscan fits a very specific use
case and would be better off being an extension instead of in contrib.




 - Custom-scan.sgml was added to introduce the way to write custom-scan
   provider in the official documentation.
 - Much code duplication in postgres_fdw.c was eliminated. I split some fdw-
   handlers into two parts; common portion and fdw specific one.
   Executor callbacks of custom-scan code utilizes the common portion above
   because most of its implementations are equivalent.

 I'd like to see comments regarding to the way to handle Var reference onto
 a custom-scan that replaced relations join.
 A varno of Var that references a join relation is rtindex of either
 right or left
 relation, then setrefs.c adjust it well; INNER_VAR or OUTER_VAR shall be
 set instead.
 However, it does not work well if a custom-scan that just references result
 of remote join query was chosen instead of local join, because its result
 shall be usually set in the ps_ResultTupleSlot of PlanState, thus
 ExecEvalScalarVar does not reference neither inner nor outer slot.
 Instead of existing solution, I added one more special varno; CUSTOM_VARNO
 that just references result-tuple-slot of the target relation.
 If CUSTOM_VARNO is given, EXPLAIN(verbose) generates column name from
 the TupleDesc of underlying ps_ResultTupleSlot.
 I'm not 100% certain whether it is the best approach for us, but it works
 well.

 Also, I'm uncertain for usage of param_info in Path structure, even though
 I followed the manner in other portion. So, please point out if my usage
 was not applicable well.

 Thanks,

 2013/11/11 Kohei KaiGai kai...@kaigai.gr.jp:
  Hi,
 
  I tried to write up a wikipage to introduce how custom-scan works.
 
  https://wiki.postgresql.org/wiki/CustomScanAPI
 
  Any comments please.
 
  2013/11/6 Kohei KaiGai kai...@kaigai.gr.jp:
  The attached patches provide a feature to implement custom scan node
  that allows extension to replace a part of plan tree with its own code
  instead of the built-in logic.
  In addition to the previous proposition, it enables us to integrate
 custom
  scan as a part of candidate paths to be chosen by optimizer.
  Here is two patches. The first one (pgsql-v9.4-custom-scan-apis) offers
  a set of API stuff and a simple demonstration module that implement
  regular table scan using inequality operator on ctid system column.
  The second one (pgsql-v9.4-custom-scan-remote-join) enhances
  postgres_fdw to support remote join capability.
 
  Below is an example to show how does custom-scan work.
 
  We usually run sequential scan even if clause has inequality operator
  that references ctid system column.
 
  postgres=# EXPLAIN SELECT ctid,* FROM t1 WHERE ctid  '(10,0)'::tid;
 QUERY PLAN
  
   Seq Scan on t1  (cost=0.00..209.00 rows= width=43)
 Filter: (ctid  '(10,0)'::tid)
  (2 rows)
 
  An extension that performs as custom-scan provider suggests
  an alternative path, and its cost was less than sequential scan,
  thus optimizer choose it.
 
  postgres=# LOAD 'ctidscan';
  LOAD
  postgres=# EXPLAIN SELECT ctid,* FROM t1 WHERE ctid  '(10,0)'::tid;
QUERY PLAN
  --
   Custom Scan (ctidscan) on t1  (cost=0.00..100.00 rows= width=43)
 Filter: (ctid  '(10,0)'::tid)
  (2 rows)
 
  Of course, more cost effective plan will win if exists.
 
  postgres=# EXPLAIN SELECT ctid,* FROM t1 WHERE ctid  '(10,0)'::tid AND
 a = 200;
  QUERY PLAN
  ---
   Index Scan using t1_pkey on t1  (cost=0.29..8.30 rows=1 width=43)
 Index Cond: (a = 200)
 Filter: (ctid  '(10,0)'::tid)
  (3 rows)
 
  One other worthwhile example is remote-join enhancement on the
  postgres_fdw as follows. Both of ft1 and ft2 are foreign table being
  managed by same foreign server.
 
  postgres=# EXPLAIN (verbose) SELECT * FROM ft1 JOIN ft2 ON a = x
WHERE f_leak(b) AND y
  like '%aaa%';
 QUERY
 PLAN
 
 --
   Custom Scan (postgres-fdw)  (cost=100.00..100.01 rows=0 width=72)
 Output: a, b, x, y
 Filter: 

Re: [HACKERS] Client Messages

[Jim Mlodgenski]

On Tue, Jan 24, 2012 at 7:38 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 On 23.01.2012 22:52, Jim Mlodgenski wrote:

 On Wed, Jan 18, 2012 at 9:19 AM, Jim Mlodgenskijimm...@gmail.com  wrote:

 On Wed, Jan 18, 2012 at 3:08 AM, Heikki Linnakangas

 I don't think that's a problem, it's just a free-form message to
 display.

 But it also doesn't seem very useful to have it PGC_USERSET: if it's
 only
 displayed at connect time, there's no point in changing it after
 connecting.

 Should we make it PGC_BACKEND?


 In hindsight, making it PGC_BACKEND makes it much less useful, because then
 you can't set it on a per-database basis with ALTER DATABASE foo SET ...
 So I made it PGC_USERSET again.


 Here is the revised patch based on the feedback.


 Thanks! I renamed the GUC to welcome_message, to avoid confusion with
 client_min_messages. I also moved it to Connection Settings category.
 Although it's technically settable within a session, the purpose is to
 display a message when connecting, so Connection Settings feels more
 fitting.

 There's one little problem remaining with this, which is what to do if the
 message is in a different encoding than used by the client? That's not a new
 problem, we have the same problem with any string GUC, if you do SHOW
 setting. We restricted application_name to ASCII characters, which is an
 obvious way to avoid the problem, but I think it would be a shame to
 restrict this to ASCII-only.
Isn't that an issue for the administrator understanding his audience.
Maybe some additional documentation explaining the encoding issue?



 --
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.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] Client Messages

[Jim Mlodgenski]

On Wed, Jan 18, 2012 at 9:19 AM, Jim Mlodgenski jimm...@gmail.com wrote:
 On Wed, Jan 18, 2012 at 3:08 AM, Heikki Linnakangas
 heikki.linnakan...@enterprisedb.com wrote:
 On 18.01.2012 07:49, Fujii Masao wrote:

 On Fri, Jan 6, 2012 at 1:38 AM, Jim Mlodgenskijimm...@gmail.com  wrote:

 I have a need to send banner messages to a psql client that I can set
 on the server and will be displayed on any psql client that connects
 to the database. This would be mostly used as an additional indicator
 to which database you are connecting, but could also be used by people
 to force their users to see an security message when connecting to the
 database. The attached patch will allow you to execute

 ALTER DATABASE postgres SET

 client_message=E'\nBEWARE:
 You are connecting to a production database. If you do anything to\n
     bring this server down, you will be destroyed by your supreme

 overlord.\n\n';

 And then when you connect to psql, you will see:

 [e3@workstation bin]$ ./psql -U user1 postgres
 psql (9.2devel)

 
 BEWARE: You are connecting to a production database. If you do anything
 to
        bring this server down, you will be destroyed by your supreme
 overlord.

 

 Type help for help.

 postgres=


 Any feedback is welcome.


 Adding new GUC parameter only for the purpose of warning psql users
 seems overkill to me.  Basically we try to reduce the number of GUC
 parameters to make a configuration easier to a user, so I don't think that
 it's good idea to add new GUC for such a small benefit.


 It seems quite useful to me...


 Instead, how
 about using .psqlrc file and writing a warning message in it by using
 \echo command?


 That's not the same thing at all. Each client would need to put the warning
 in that file, and you'd get it regardless of the database you connect to.


 Anyway, I found one problem in the patch. The patch defines client_message
 as PGC_USERSET parameter, which means that any psql can falsify a
 warning message, e.g., by setting the environment variable PGOPTIONS
 to -c client_message=hoge. This seems to be something to avoid from
 security point of view.


 I don't think that's a problem, it's just a free-form message to display.
 But it also doesn't seem very useful to have it PGC_USERSET: if it's only
 displayed at connect time, there's no point in changing it after connecting.
 Should we make it PGC_BACKEND?


 The only security problem that I can think of is a malicious server
 (man-in-the-middle perhaps), that sends a banner that confuses

 Docs for PQparameterStatus() needs adjustment, now that client_message is
 also one of the settings automatically reported to the client.
 I'll add the docs for that..


 The placement of the banner in psql looks currently like this:

 $ psql postgres

 psql (9.2devel)
 Hello world!
 Type help for help.


 or

 postgres=# \c postgres
 Hello world!
 You are now connected to database postgres as user heikki.


 Are we happy with that? I think it would be better to print the banner just
 before the prompt:
 I like that better. I'll make that change as well.

Here is the revised patch based on the feedback.



 psql (9.2devel)
 Type help for help.

 Hello world!

 postgres=# \c postgres
 You are now connected to database postgres as user heikki.

 Hello world!
 postgres=#

 Should we prefix the banner with something that makes it clear that it's a
 message coming from the server? Something like:
 I don't think the default prefix adds much for the user. If the
 administrator wants to let the user know that its from the server, he
 can add it to the message.


 psql (9.2devel)
 Type help for help.

 Notice from server: Hello world!

 postgres=# \c postgres
 You are now connected to database postgres as user heikki.
 Notice from server: Hello world!
 postgres=#

 --
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index e55b503..04bc671 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5324,6 +5324,19 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
   /listitem
  /varlistentry
 
+ varlistentry id=guc-client-message xreflabel=client_message
+  termvarnameclient_message/varname (typestring/type)/term
+  indexterm
+   primaryvarnameclient_message/ configuration parameter/primary
+  /indexterm
+  listitem
+   para
+The varnameclient_message/varname can be any string that will be 
+displayed to the user in the banner of psql. 
+   /para
+  /listitem
+ /varlistentry
+
  /variablelist
 /sect2
/sect1
diff --git a/doc/src/sgml

Re: [HACKERS] Client Messages

[Jim Mlodgenski]

On Wed, Jan 18, 2012 at 3:08 AM, Heikki Linnakangas
heikki.linnakan...@enterprisedb.com wrote:
 On 18.01.2012 07:49, Fujii Masao wrote:

 On Fri, Jan 6, 2012 at 1:38 AM, Jim Mlodgenskijimm...@gmail.com  wrote:

 I have a need to send banner messages to a psql client that I can set
 on the server and will be displayed on any psql client that connects
 to the database. This would be mostly used as an additional indicator
 to which database you are connecting, but could also be used by people
 to force their users to see an security message when connecting to the
 database. The attached patch will allow you to execute

 ALTER DATABASE postgres SET

 client_message=E'\nBEWARE:
 You are connecting to a production database. If you do anything to\n
     bring this server down, you will be destroyed by your supreme

 overlord.\n\n';

 And then when you connect to psql, you will see:

 [e3@workstation bin]$ ./psql -U user1 postgres
 psql (9.2devel)

 
 BEWARE: You are connecting to a production database. If you do anything
 to
        bring this server down, you will be destroyed by your supreme
 overlord.

 

 Type help for help.

 postgres=


 Any feedback is welcome.


 Adding new GUC parameter only for the purpose of warning psql users
 seems overkill to me.  Basically we try to reduce the number of GUC
 parameters to make a configuration easier to a user, so I don't think that
 it's good idea to add new GUC for such a small benefit.


 It seems quite useful to me...


 Instead, how
 about using .psqlrc file and writing a warning message in it by using
 \echo command?


 That's not the same thing at all. Each client would need to put the warning
 in that file, and you'd get it regardless of the database you connect to.


 Anyway, I found one problem in the patch. The patch defines client_message
 as PGC_USERSET parameter, which means that any psql can falsify a
 warning message, e.g., by setting the environment variable PGOPTIONS
 to -c client_message=hoge. This seems to be something to avoid from
 security point of view.


 I don't think that's a problem, it's just a free-form message to display.
 But it also doesn't seem very useful to have it PGC_USERSET: if it's only
 displayed at connect time, there's no point in changing it after connecting.
Should we make it PGC_BACKEND?


 The only security problem that I can think of is a malicious server
 (man-in-the-middle perhaps), that sends a banner that confuses

 Docs for PQparameterStatus() needs adjustment, now that client_message is
 also one of the settings automatically reported to the client.
I'll add the docs for that..


 The placement of the banner in psql looks currently like this:

 $ psql postgres

 psql (9.2devel)
 Hello world!
 Type help for help.


 or

 postgres=# \c postgres
 Hello world!
 You are now connected to database postgres as user heikki.


 Are we happy with that? I think it would be better to print the banner just
 before the prompt:
I like that better. I'll make that change as well.


 psql (9.2devel)
 Type help for help.

 Hello world!

 postgres=# \c postgres
 You are now connected to database postgres as user heikki.

 Hello world!
 postgres=#

 Should we prefix the banner with something that makes it clear that it's a
 message coming from the server? Something like:
I don't think the default prefix adds much for the user. If the
administrator wants to let the user know that its from the server, he
can add it to the message.


 psql (9.2devel)
 Type help for help.

 Notice from server: Hello world!

 postgres=# \c postgres
 You are now connected to database postgres as user heikki.
 Notice from server: Hello world!
 postgres=#

 --
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.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] Client Messages

[Jim Mlodgenski]

I have a need to send banner messages to a psql client that I can set
on the server and will be displayed on any psql client that connects
to the database. This would be mostly used as an additional indicator
to which database you are connecting, but could also be used by people
to force their users to see an security message when connecting to the
database. The attached patch will allow you to execute

ALTER DATABASE postgres SET
client_message=E'\nBEWARE:
You are connecting to a production database. If you do anything to\n
 bring this server down, you will be destroyed by your supreme
overlord.\n\n';

And then when you connect to psql, you will see:

[e3@workstation bin]$ ./psql -U user1 postgres
psql (9.2devel)

BEWARE: You are connecting to a production database. If you do anything to
bring this server down, you will be destroyed by your supreme overlord.


Type help for help.

postgres=


Any feedback is welcome.

Thanks
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
new file mode 100644
index 0cc3296..fa5b942
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*** dynamic_library_path = 'C:\tools\postgre
*** 5323,5328 
--- 5323,5341 
/listitem
   /varlistentry
  
+  varlistentry id=guc-client-message xreflabel=client_message
+   termvarnameclient_message/varname (typestring/type)/term
+   indexterm
+primaryvarnameclient_message/ configuration parameter/primary
+   /indexterm
+   listitem
+para
+ The varnameclient_message/varname can be any string that will be 
+ displayed to the user in the banner of psql. 
+/para
+   /listitem
+  /varlistentry
+ 
   /variablelist
  /sect2
 /sect1
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
new file mode 100644
index 5c910dd..58b6000
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
*** static char *log_destination_string;
*** 455,460 
--- 455,461 
  static char *syslog_ident_str;
  static bool phony_autocommit;
  static bool session_auth_is_superuser;
+ static char *client_message_string;
  static double phony_random_seed;
  static char *client_encoding_string;
  static char *datestyle_string;
*** static struct config_string ConfigureNam
*** 3018,3023 
--- 3019,3035 
  		check_application_name, assign_application_name, NULL
  	},
  
+ {
+ {client_message, PGC_USERSET, CLIENT_CONN_OTHER,
+ gettext_noop(Sets a message to be displayed to the user when connecting via psql.),
+ NULL,
+ GUC_REPORT | GUC_NO_SHOW_ALL
+ },
+ client_message_string,
+ ,
+ NULL, NULL, NULL
+ },
+ 
  	/* End-of-list marker */
  	{
  		{NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL, NULL
diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
new file mode 100644
index 315db46..8eb5af5
*** a/src/backend/utils/misc/postgresql.conf.sample
--- b/src/backend/utils/misc/postgresql.conf.sample
***
*** 515,520 
--- 515,521 
  
  #dynamic_library_path = '$libdir'
  #local_preload_libraries = ''
+ #client_message = ''
  
  
  #--
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
new file mode 100644
index 69fac83..44bf114
*** a/src/bin/psql/command.c
--- b/src/bin/psql/command.c
*** static bool get_create_function_cmd(PGco
*** 65,70 
--- 65,71 
  static int	strip_lineno_from_funcdesc(char *func);
  static void minimal_error_message(PGresult *res);
  
+ static void printClientMessage(void);
  static void printSSLInfo(void);
  
  #ifdef WIN32
*** connection_warnings(bool in_startup)
*** 1699,1709 
--- 1700,1729 
  		checkWin32Codepage();
  #endif
  		printSSLInfo();
+ 
+ 		printClientMessage();
  	}
  }
  
  
  /*
+  * printClientMessage
+  *
+  * Prints any message stored in the client_message GUC
+  */
+ static void
+ printClientMessage(void)
+ {
+ 	const char *message;
+ 
+ 	message = PQparameterStatus(pset.db, client_message);
+ 
+ 	if (message)
+ 		printf(_(%s\n), message);
+ }
+ 
+ 
+ /*
   * printSSLInfo
   *
   * Prints information about the current SSL connection, if SSL is in use

-- 
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] testing HS/SR - 1 vs 2 performance

[Jim Mlodgenski]

On Mon, Apr 12, 2010 at 7:07 AM, Robert Haas robertmh...@gmail.com wrote:
 On Mon, Apr 12, 2010 at 5:06 AM, Fujii Masao masao.fu...@gmail.com wrote:
 On Sat, Apr 10, 2010 at 8:23 AM, Erik Rijkers e...@xs4all.nl wrote:
 I understand that in the scale=1000 case, there is a huge
 cache effect, but why doesn't that apply to the pgbench runs
 against the standby?  (and for the scale=10_000 case the
 differences are still rather large)

 I guess that this performance degradation happened because a number of
 buffer replacements caused UpdateMinRecoveryPoint() often. So I think
 increasing shared_buffers would improve the performance significantly.

 I think we need to investigate this more.  It's not going to look good
 for the project if people find that a hot standby server runs two
 orders of magnitude slower than the primary.
As a data point, I did a read only pgbench test and found that the
standby runs about 15% slower than the primary with identical hardware
and configs.

 ...Robert

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




-- 
--
Jim Mlodgenski
EnterpriseDB (http://www.enterprisedb.com)

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