Re: [HACKERS] polymorphic types - enforce casting to most common type automatically
Hi
I am sending a proof concept. Current implementation is not suboptimal - I
wrote this code for demonstration of current issues, and checking possible
side effects of changes in this patch.
The basic problem is strong restrictive implementation of polymorphic types
- now these types doesn't allow any cast although it is possible. It can be
changed relatively simply I though (after we implemented variadic
functions).
CREATE OR REPLACE FUNCTION public.foo1(anyelement, anyelement)
RETURNS anyelement
LANGUAGE sql
AS $function$
SELECT $1 + $2;
$function$
CREATE OR REPLACE FUNCTION public.foo2(anyelement, anyelement)
RETURNS anyarray
LANGUAGE sql
AS $function$
SELECT ARRAY[$1, $2]
$function$
Now, polymorphic functions don't allow some natively expected calls:
postgres=# select foo1(1,1);
foo1
--
2
(1 row)
postgres=# select foo1(1,1.1);
ERROR: function foo1(integer, numeric) does not exist
LINE 1: select foo1(1,1.1);
^
HINT: No function matches the given name and argument types. You might
need to add explicit type casts.
postgres=# select foo2(1,1);
foo2
---
{1,1}
(1 row)
postgres=# select foo2(1,1.1);
ERROR: function foo2(integer, numeric) does not exist
LINE 1: select foo2(1,1.1);
^
HINT: No function matches the given name and argument types. You might
need to add explicit type casts.
CREATE OR REPLACE FUNCTION public.foo3(VARIADIC anyarray)
RETURNS anyelement
LANGUAGE sql
AS $function$
SELECT min(v) FROM unnest($1) g(v)
$function$
postgres=# SELECT foo3(1,2,3);
foo3
--
1
(1 row)
postgres=# SELECT foo3(1,2,3.1);
ERROR: function foo3(integer, integer, numeric) does not exist
LINE 1: SELECT foo3(1,2,3.1);
^
HINT: No function matches the given name and argument types. You might
need to add explicit type casts.
Some our functions like COALESCE are not too restrictive and allow to use
types from same category.
postgres=# select coalesce(1,1.1);
coalesce
--
1
(1 row)
With attached patch the polymorphic functions use same mechanism as our
buildin functions. It is applied on ANYARRAY, ANYELEMENT types only.
postgres=# select foo1(1,1.1), foo2(1,1.1), foo3(1.1,2,3.1);
foo1 | foo2 | foo3
--+-+--
2.1 | {1,1.1} | 1.1
(1 row)
Comments, notices, ... ?
Regards
Pavel
2014-11-24 20:52 GMT+01:00 Pavel Stehule pavel.steh...@gmail.com:
Hello
now a functions with more than one polymorphic arguments are relative
fragile due missing casting to most common type. Some our functions like
coalesce can do it, so it is surprising for our users.
our custom polymorphic function foo(anyelement, anyelement) working well
for
foo(10,20) or foo(10.1, 20.1), but not for foo(10, 20.1)
I am thinking, so we can add a searching most common type stage without
breaking to backing compatibility.
What do you think about it?
Regards
Pavel
commit 4eb8c9bf9e5b2d20d69e8e68fa34e760537347c2
Author: Pavel Stehule pavel.steh...@gooddata.com
Date: Sun Mar 8 19:28:49 2015 +0100
proof concept
diff --git a/src/backend/parser/parse_coerce.c b/src/backend/parser/parse_coerce.c
index a4e494b..92e63de 100644
--- a/src/backend/parser/parse_coerce.c
+++ b/src/backend/parser/parse_coerce.c
@@ -1261,6 +1261,100 @@ select_common_type(ParseState *pstate, List *exprs, const char *context,
}
/*
+ * select_common_type_from_vector()
+ * Determine the common supertype of vector of Oids.
+ *
+ * Similar to select_common_type() but simplified for polymorphics
+ * type processing. When there are no supertype, then returns InvalidOid,
+ * when noerror is true, or raise exception when noerror is false.
+ */
+Oid
+select_common_type_from_vector(int nargs, Oid *typeids, bool noerror)
+{
+ int i = 0;
+ Oid ptype;
+ TYPCATEGORY pcategory;
+ bool pispreferred;
+
+ Assert(nargs 0);
+ ptype = typeids[0];
+
+ /* fast leave when all types are same */
+ if (ptype != UNKNOWNOID)
+ {
+ for (i = 1; i nargs; i++)
+ {
+ if (ptype != typeids[i])
+break;
+ }
+
+ if (i == nargs)
+ return ptype;
+ }
+
+ /*
+ * Nope, so set up for the full algorithm. Note that at this point, lc
+ * points to the first list item with type different from pexpr's; we need
+ * not re-examine any items the previous loop advanced over.
+ */
+ ptype = getBaseType(ptype);
+ get_type_category_preferred(ptype, pcategory, pispreferred);
+
+ for (; i nargs; i++)
+ {
+ Oid ntype = getBaseType(typeids[i]);
+
+ /* move on to next one if no new information... */
+ if (ntype != UNKNOWNOID ntype != ptype)
+ {
+ TYPCATEGORY ncategory;
+ bool nispreferred;
+
+ get_type_category_preferred(ntype, ncategory, nispreferred);
+
+ if (ptype == UNKNOWNOID)
+ {
+/* so far, only unknowns so take anything... */
+ptype = ntype;
+pcategory = ncategory;
+pispreferred = nispreferred;
+ }
+ else if (ncategory != pcategory)
+ {
+if (noerror)
+ return InvalidOid;
+
+ereport(ERROR,
+
Re: [HACKERS] MD5 authentication needs help -SCRAM
All, Since SCRAM has been brought up a number of times here, I thought I'd loop in the PostgreSQL contributor who is co-author of the SCRAM standard to see if he has anything to say about implementing SCRAM as a built-in auth method for Postgres. Abhijit? -- Josh Berkus PostgreSQL Experts Inc. http://pgexperts.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] Bootstrap DATA is a pita
Stephen Frost sfr...@snowman.net writes: * Andrew Dunstan (and...@dunslane.net) wrote: On 03/07/2015 05:46 PM, Andres Freund wrote: On 2015-03-07 16:43:15 -0600, Jim Nasby wrote: Semi-related... if we put some special handling in some places for bootstrap mode, couldn't most catalog objects be created using SQL, once we got pg_class, pg_attributes and pg_type created? Several people have now made that suggestion, but I *seriously* doubt that we actually want to go there. The overhead of executing SQL commands in comparison to the bki stuff is really rather noticeable. Doing the majority of the large number of insertions via SQL will make initdb noticeably slower. And it's already annoyingly slow. Besides make install it's probably the thing I wait most for during development. My reaction exactly. We should not make users pay a price for developers' convenience. Another reason not to do this is that it would require a significant (in my judgment) amount of crapification of a lot of code with bootstrap-mode special cases. Neither the parser, the planner, nor the executor could function in bootstrap mode without a lot of lobotomization. Far better to confine all that ugliness to bootstrap.c. Just to clarify, since Jim was responding to my comment, my thought was *not* to use SQL commands inside initdb, but rather to use PG to create the source files that we have today in our tree, which wouldn't slow down initdb at all. That, on the other hand, might be a sane suggestion. I'm not sure though. It feels more like use the hammer you have at hand than necessarily being a good fit. In particular, keeping the raw data in some tables doesn't seem like an environment that would naturally distinguish between hard-coded and defaultable values. For instance, how would you distinguish hard-coded OIDs from ones that could be assigned at initdb's whim? 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] New functions
Sorry, 3 functions. 08.03.2015, 22:16, Dmitry Voronin carriingfat...@yandex.ru: Hello, I make an a patch, which adds 4 functions to sslinfo extension module: 1) ssl_extension_names() --- get short names of X509v3 extensions from client certificate and it's values; 2) ssl_extension_value(text) --- get value of extension from certificate (argument --- short name of extension); 3) ssl_extension_is_critical(text) --- returns true, if extension is critical and false, if is not (argument --- short name of extension). You can view some information of certificate's extensions via those functions. What do you think about it? -- Best regards, Dmitry Voronin , -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Best regards, Dmitry Voronin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] New functions
Hello,
I make an a patch, which adds 4 functions to sslinfo extension module:1) ssl_extension_names() --- get short names of X509v3 extensions from client certificate and it's values;
2) ssl_extension_value(text) --- get value of extension from certificate (argument --- short name of extension);
3) ssl_extension_is_critical(text) --- returns true, if extension is critical and false, if is not (argument --- short name of extension).
You can view some information of certificate's extensions via those functions.
What do you think about it?-- Best regards, Dmitry Voronin *** a/contrib/sslinfo/sslinfo.c
--- b/contrib/sslinfo/sslinfo.c
***
*** 14,21
--- 14,23
#include miscadmin.h
#include utils/builtins.h
#include mb/pg_wchar.h
+ #include funcapi.h
#include openssl/x509.h
+ #include openssl/x509v3.h
#include openssl/asn1.h
PG_MODULE_MAGIC;
***
*** 23,28 PG_MODULE_MAGIC;
--- 25,31
static Datum X509_NAME_field_to_text(X509_NAME *name, text *fieldName);
static Datum X509_NAME_to_text(X509_NAME *name);
static Datum ASN1_STRING_to_text(ASN1_STRING *str);
+ int get_extension(X509 *cert, const char *ext_name, X509_EXTENSION **extension);
/*
***
*** 354,356 ssl_issuer_dn(PG_FUNCTION_ARGS)
--- 357,581
PG_RETURN_NULL();
return X509_NAME_to_text(X509_get_issuer_name(MyProcPort-peer));
}
+
+
+ /*
+ * Returns extension object by given certificate and extension's name.
+ *
+ * Try to get extension from certificate by extension's name.
+ * We returns at extension param pointer to X509_EXTENSION.
+ *
+ * Returns 0, if we have found extension in certificate and 1, if we not.
+ */
+ int get_extension(X509* cert, const char *ext_name, X509_EXTENSION **extension)
+ {
+ int nid = 0;
+ int loc = 0;
+
+ /* try to convert extension name to ObjectID */
+ nid = OBJ_txt2nid(ext_name);
+ /* Not success ? */
+ if (nid == NID_undef)
+ return 1;
+
+ loc = X509_get_ext_by_NID(cert, nid, -1);
+
+ /* palloc memory for extension and copy it */
+ *extension = (X509_EXTENSION *) palloc(sizeof(X509_EXTENSION *));
+ memcpy(*extension, X509_get_ext(cert, loc), sizeof(X509_EXTENSION));
+
+ return 0;
+ }
+
+
+ /* Returns value of extension
+ *
+ * This function returns value of extension by given name in client certificate.
+ *
+ * Returns text datum.
+ */
+ PG_FUNCTION_INFO_V1(ssl_extension_value);
+ Datum
+ ssl_extension_value(PG_FUNCTION_ARGS)
+ {
+ X509 *cert = MyProcPort-peer;
+ X509_EXTENSION *ext = NULL;
+ char *ext_name = text_to_cstring(PG_GETARG_TEXT_P(0));
+ BIO *membuf = NULL;
+ char *val = NULL;
+ char nullterm = '\0';
+ int error = 0;
+
+ /* If we have no ssl security */
+ if (cert == NULL)
+ PG_RETURN_NULL();
+
+ /* If extension's converting from text name to extension's OID failed (return NID_undef) */
+ if (OBJ_txt2nid(ext_name) == NID_undef)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg(Unknown extension name \%s\, ext_name)));
+
+ /* Extension's name is correct, try to get extension object from certificate */
+ error = get_extension(cert, ext_name, ext);
+
+ /* Not found? */
+ if (error)
+ PG_RETURN_NULL();
+
+ /* Print extension to BIO */
+ membuf = BIO_new(BIO_s_mem());
+ X509V3_EXT_print(membuf, ext, 0, 0);
+ BIO_write(membuf, nullterm, 1);
+ BIO_get_mem_data(membuf, val);
+
+ /* Copy value */
+ val = pstrdup(val);
+
+ /* Clear BIO */
+ BIO_free(membuf);
+
+ /* free extension */
+ if (ext)
+ pfree(ext);
+
+ PG_RETURN_TEXT_P(cstring_to_text(val));
+ }
+
+
+ /* Returns status of extension
+ *
+ * Returns true, if extension is critical and false, if it is not.
+ *
+ * Returns bool datum.
+ */
+ PG_FUNCTION_INFO_V1(ssl_extension_is_critical);
+ Datum
+ ssl_extension_is_critical(PG_FUNCTION_ARGS)
+ {
+ X509 *cert = MyProcPort-peer;
+ X509_EXTENSION *ext = NULL;
+ char *ext_name = text_to_cstring(PG_GETARG_TEXT_P(0));
+ int critical;
+ int error = 0;
+
+ /* If we have no ssl security */
+ if (cert == NULL)
+ PG_RETURN_NULL();
+
+ /* If extension's converting from text name to extension's OID failed (return NID_undef) */
+ if (OBJ_txt2nid(ext_name) == NID_undef)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg(Unknown extension name \%s\, ext_name)));
+
+ /* Extension's name is correct, try to get extension object from certificate */
+ error = get_extension(cert, ext_name, ext);
+
+ /* Not found? */
+ if (error)
+ PG_RETURN_NULL();
+
+ critical = X509_EXTENSION_get_critical(ext);
+
+ /* free extension */
+ if (ext)
+ pfree(ext);
+
+ PG_RETURN_BOOL(critical);
+ }
+
+
+ /* Returns key-value pairs of extension name and it's value
+ *
+ * This function print extensions of client certificate at table form (extension's name and it's value).
+ *
+ * Returns setof text datum.
+ */
+
Re: [HACKERS] Bootstrap DATA is a pita
Andres Freund and...@2ndquadrant.com writes: On 2015-03-04 10:25:58 -0500, Robert Haas wrote: Another advantage of this is that it would probably make git less likely to fumble a rebase. If there are lots of places in the file where we have the same 10 lines in a row with occasional variations, rebasing a patch could easily pick the the wrong place to reapply the hunk. I would personally consider a substantial increase in the rate of such occurrences as being a cure far, far worse than the disease. If you keep the entry for each function on just a couple of lines the chances of this happening are greatly reduced, because you're much likely to get a false match to surrounding context. I'm not particularly worried about this. Especially with attribute defaults it seems unlikely that you often have the same three surrounding lines in both directions in a similar region of the file. Really? A lot depends on the details of how we choose to lay out these files, but you could easily blow all your safety margin on lines containing just braces, for instance. I'll reserve judgment on this till I see the proposed new catalog data files, but I absolutely reject any contention that it's not something to worry about. And even if it turns out to actually be bothersome, you can help yourself by passing -U 5/setting diff.context = 5 or something like that. Um. Good luck with getting every patch submitter to do that. 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
[HACKERS] PATCH: pgbench - merging transaction logs
Hi there,
attached is a patch implementing merging of pgbench logs. These logs are
written by each thread, so with N threads you get N files with names
pgbench_log.PID
pgbench_log.PID.1
...
pgbench_log.PID.N
Before analyzing these logs, these files need to be combined. I usually
ended up wrinting ad-hoc scripts doing that, lost them, written them
again and so on over and over again.
The other disadvantage of the external scripts is that you have to pass
all the info about the logs (whether the logs are aggregated, whther
there's throttling, etc.).
So integrating this into pgbench directly seems like a better approach,
and the attached patch implements that.
With '-m' or '--merge-logs' on the command-line, the logs are merged at
the end, using a simple 2-way merge to keep the log sorted by the time
field. It should work with all the other options that influence the log
format (--rate, --aggregate-interval and --latency-limit).
I'll add this to CF 2016-06.
--
Tomas Vondrahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 706fdf5..d6ec87e 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -157,6 +157,11 @@ char *tablespace = NULL;
char *index_tablespace = NULL;
/*
+ * merge logs (transaction logs, aggregated logs) at the end
+ */
+bool merge_logs = false;
+
+/*
* end of configurable parameters
*/
@@ -367,6 +372,10 @@ static void *threadRun(void *arg);
static void doLog(TState *thread, CState *st, FILE *logfile, instr_time *now,
AggVals *agg, bool skipped);
+static void merge_log_files(int agg_interval, int nthreads);
+static void merge_aggregated_logs(FILE *infile_a, FILE *infile_b, FILE *outfile);
+static void merge_simple_logs(FILE *infile_a, FILE *infile_b, FILE *outfile);
+
static void
usage(void)
{
@@ -408,6 +417,7 @@ usage(void)
-v, --vacuum-all vacuum all four standard tables before tests\n
--aggregate-interval=NUM aggregate data over NUM seconds\n
--sampling-rate=NUM fraction of transactions to log (e.g. 0.01 for 1%%)\n
+ -m, --merge-logs merge logs produced by multiple threads\n
\nCommon options:\n
-d, --debug print debugging output\n
-h, --host=HOSTNAME database server host or socket directory\n
@@ -2733,6 +2743,7 @@ main(int argc, char **argv)
{aggregate-interval, required_argument, NULL, 5},
{rate, required_argument, NULL, 'R'},
{latency-limit, required_argument, NULL, 'L'},
+ {merge-logs, no_argument, NULL, 'm'},
{NULL, 0, NULL, 0}
};
@@ -2808,7 +2819,7 @@ main(int argc, char **argv)
state = (CState *) pg_malloc(sizeof(CState));
memset(state, 0, sizeof(CState));
- while ((c = getopt_long(argc, argv, ih:nvp:dqSNc:j:Crs:t:T:U:lf:D:F:M:P:R:L:, long_options, optindex)) != -1)
+ while ((c = getopt_long(argc, argv, ih:nvp:dqSNc:j:Crs:t:T:U:lf:D:F:M:P:R:L:m, long_options, optindex)) != -1)
{
switch (c)
{
@@ -3017,6 +3028,10 @@ main(int argc, char **argv)
latency_limit = (int64) (limit_ms * 1000);
}
break;
+ case 'm':
+printf(merge logs\n);
+merge_logs = true;
+break;
case 0:
/* This covers long options which take no argument. */
if (foreign_keys || unlogged_tables)
@@ -3137,6 +3152,12 @@ main(int argc, char **argv)
exit(1);
}
+ if (merge_logs (! use_log))
+ {
+ fprintf(stderr, log merging is allowed only when actually logging transactions\n);
+ exit(1);
+ }
+
/*
* is_latencies only works with multiple threads in thread-based
* implementations, not fork-based ones, because it supposes that the
@@ -3418,6 +3439,10 @@ main(int argc, char **argv)
throttle_lag, throttle_lag_max, throttle_latency_skipped,
latency_late);
+ /* Merge logs, if needed */
+ if (merge_logs)
+ merge_log_files(agg_interval, nthreads);
+
return 0;
}
@@ -3783,6 +3808,339 @@ done:
return result;
}
+static void
+merge_log_files(int agg_interval, int nthreads)
+{
+ int i;
+
+ /* we can do this as 2-way merges (all the logs are sorted by timestamp) */
+ for (i = 1; i nthreads; i++)
+ {
+ char logpath[64];
+ char logpath_new[64];
+
+ /* input and output files */
+ FILE *infile_a, *infile_b;
+ FILE *outfile;
+
+ /* the first input is always the 'main' logfile */
+ snprintf(logpath, sizeof(logpath), pgbench_log.%d, main_pid);
+ infile_a = fopen(logpath, r);
+
+ if (infile_b == NULL)
+ {
+ fprintf(stderr, Couldn't open logfile \%s\: %s, logpath, strerror(errno));
+ return;
+ }
+
+ snprintf(logpath, sizeof(logpath), pgbench_log.%d.%d, main_pid, i);
+ infile_b = fopen(logpath, r);
+
+ if (infile_b == NULL)
+ {
+ fprintf(stderr, Couldn't open logfile \%s\: %s, logpath, strerror(errno));
+ return;
+ }
+
+ snprintf(logpath, sizeof(logpath),
Re: [HACKERS] Strange debug message of walreciver?
On Sat, Mar 7, 2015 at 10:18 PM, Tatsuo Ishii is...@postgresql.org wrote: When I set log_min_messages to debug5 and looked into walreciver log, I saw this: 3600 2015-03-08 09:47:38 JST DEBUG: sendtime 2015-03-08 09:47:38.31493+09 receipttime 2015-03-08 09:47:38.315027+09 replication apply delay -1945478837 ms tran sfer latency 0 ms The replication apply delay -1945478837 ms part looks strange because the delay is below 0. The number is formatted as %d in elog call, and I suspect this is kind of integer overflow. Looking at GetReplicationApplyDelay() in walreceiverfuncs.c I noticed that the integer overflow occurs because sometimes the return of the GetCurrentChunkReplayStartTime() is 0 (zero). I added an elog into GetReplicationApplyDelay to check this info: DEBUG: GetReplicationApplyDelay 479099832 seconds, 352 milliseconds, (0.00, 479099832352083.00) DEBUG: sendtime 2015-03-08 00:17:12.351987-03 receipttime 2015-03-08 00:17:12.352043-03 replication apply delay -1936504800 ms transfer latency 0 ms DEBUG: GetReplicationApplyDelay 479099841 seconds, 450 milliseconds, (0.00, 479099841450320.00) DEBUG: sendtime 2015-03-08 00:17:21.45018-03 receipttime 2015-03-08 00:17:21.450294-03 replication apply delay -1936495702 ms transfer latency 0 ms Right. Until walreceiver gets the first WAL record to replay, xlogctl-currentChunkStartTime remains 0. Maybe we should check before and return zero from GetReplicationApplyDelay. The attached patch implement it. Your patch regards 0 replication apply delay in the case above which is confusing if the delay is actually 0. What about something like this to explicitly stats the delay data is not available? elog(DEBUG2, sendtime %s receipttime %s replication apply delay (N/A) transfer latency %d ms, Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] PATCH: pgbench - logging aggregated info and transactions at the same time
Hi,
another thing that I find annoying on pgbench is that you can either log
aggregated summary (per interval) or detailed transaction info (possibly
sampled), but not both at the same time.
That's annoying because what I generally use the aggregated info, but
sometimes the transaction info would be handy too for verification and
further analysis.
Attached patch makes that possible by decoupling these two kinds of
logging. Up to now, '--agg-interval' option required '-l'. When both
options were used, aggregated data were logged. When only '-l' was used,
per-transaction info was logged. The patch makes those two options
independent.
When '-l' is used, per-transaction info (possibly for only a sample of
transactions, when --sample-rate is used) is written into
pgbench_log.PID.THREAD files.
When '--agg-interval' is used, aggregated info is collected and written
into pgbench_agg_log.PID.THREAD files.
It's possible to use all three options at the same time - in that case,
the sampling is only applied to the per-transaction logs. The aggregated
log will contain data from all the transactions.
This produces one log per thread, but combining this with the other
pgbench patch (log merge) should be trivial.
--
Tomas Vondrahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
diff --git a/contrib/pgbench/pgbench.c b/contrib/pgbench/pgbench.c
index 706fdf5..fb0cf00 100644
--- a/contrib/pgbench/pgbench.c
+++ b/contrib/pgbench/pgbench.c
@@ -174,7 +174,8 @@ char *index_tablespace = NULL;
*/
#define SCALE_32BIT_THRESHOLD 2
-bool use_log; /* log transaction latencies to a file */
+bool use_trans_log; /* log transaction latencies to a file */
+bool use_agg_log; /* log aggregated info to a file */
bool use_quiet; /* quiet logging onto stderr */
int agg_interval; /* log aggregates instead of individual
* transactions */
@@ -364,8 +365,8 @@ static char *select_only = {
static void setalarm(int seconds);
static void *threadRun(void *arg);
-static void doLog(TState *thread, CState *st, FILE *logfile, instr_time *now,
- AggVals *agg, bool skipped);
+static void doLog(TState *thread, CState *st, FILE *tlogfile, FILE *alogfile,
+ instr_time *now, AggVals *agg, bool skipped);
static void
usage(void)
@@ -1121,7 +1122,8 @@ agg_vals_init(AggVals *aggs, instr_time start)
/* return false iff client should be disconnected */
static bool
-doCustom(TState *thread, CState *st, instr_time *conn_time, FILE *logfile, AggVals *agg)
+doCustom(TState *thread, CState *st, instr_time *conn_time, FILE *tlogfile,
+ FILE *alogfile, AggVals *agg)
{
PGresult *res;
Command **commands;
@@ -1176,8 +1178,8 @@ top:
{
thread-throttle_latency_skipped++;
-if (logfile)
- doLog(thread, st, logfile, now, agg, true);
+if (tlogfile || alogfile)
+ doLog(thread, st, tlogfile, alogfile, now, agg, true);
wait = getPoissonRand(thread, throttle_delay);
thread-throttle_trigger += wait;
@@ -1278,8 +1280,8 @@ top:
}
/* record the time it took in the log */
- if (logfile)
-doLog(thread, st, logfile, now, agg, false);
+ if (tlogfile || alogfile)
+doLog(thread, st, tlogfile, alogfile, now, agg, false);
}
if (commands[st-state]-type == SQL_COMMAND)
@@ -1365,7 +1367,7 @@ top:
}
/* Record transaction start time under logging, progress or throttling */
- if ((logfile || progress || throttle_delay || latency_limit) st-state == 0)
+ if ((tlogfile || alogfile || progress || throttle_delay || latency_limit) st-state == 0)
{
INSTR_TIME_SET_CURRENT(st-txn_begin);
@@ -1695,20 +1697,12 @@ top:
* print log entry after completing one transaction.
*/
static void
-doLog(TState *thread, CState *st, FILE *logfile, instr_time *now, AggVals *agg,
- bool skipped)
+doLog(TState *thread, CState *st, FILE *tlogfile, FILE *alogfile, instr_time *now,
+ AggVals *agg, bool skipped)
{
double lag;
double latency;
- /*
- * Skip the log entry if sampling is enabled and this row doesn't belong
- * to the random sample.
- */
- if (sample_rate != 0.0
- pg_erand48(thread-random_state) sample_rate)
- return;
-
if (INSTR_TIME_IS_ZERO(*now))
INSTR_TIME_SET_CURRENT(*now);
@@ -1719,7 +1713,7 @@ doLog(TState *thread, CState *st, FILE *logfile, instr_time *now, AggVals *agg,
lag = (double) (INSTR_TIME_GET_MICROSEC(st-txn_begin) - st-txn_scheduled);
/* should we aggregate the results or not? */
- if (agg_interval 0)
+ if (alogfile)
{
/*
* Are we still in the same interval? If yes, accumulate the values
@@ -1771,7 +1765,7 @@ doLog(TState *thread, CState *st, FILE *logfile, instr_time *now, AggVals *agg,
* ifdef in usage), so we don't need to handle
* this in a special way (see below).
*/
-fprintf(logfile, %ld %d %.0f %.0f %.0f %.0f,
+fprintf(alogfile, %ld %d %.0f %.0f %.0f %.0f,
agg-start_time,
agg-cnt,
Re: [HACKERS] Clamping reulst row number of joins.
I wrote: Stephen Frost sfr...@snowman.net writes: I've certainly seen and used values() constructs in joins for a variety of reasons and I do think it'd be worthwhile for the planner to know how to pull up a VALUES construct. I spent a bit of time looking at this, and realized that the blocker is the same as the reason why we don't pull up sub-selects with empty rangetables (SELECT expression(s)). Namely, that the upper query's jointree can't handle a null subtree. (This is not only a matter of the jointree data structure, but the fact that the whole planner identifies relations by bitmapsets of RTE indexes, and subtrees with empty RTE sets couldn't be told apart.) We could probably fix both cases for order-of-a-hundred lines of new code in prepjointree. The plan I'm thinking about is to allow such vacuous subquery jointrees to be pulled up, but only if they are in a place in the upper query's jointree where it's okay to delete the subtree. This would basically be two cases: (1) the immediate parent is a FromExpr that would have at least one remaining child, or (2) the immediate parent is an INNER JOIN whose other child isn't also being deleted (so that we can convert the JoinExpr to a nonempty FromExpr, or just use the other child as-is if the JoinExpr has no quals). Here's a draft patch along those lines. Unsurprisingly, it changes the plans generated for a number of regression-test queries. In most cases I felt it desirable to force the old plan shape to be retained (by inserting offset 0 or equivalent) because the test was trying to test proper generation of a query plan of that shape. I did add a couple cases where the optimization was allowed to go through. The patch is a bit bigger than I'd hoped (a net of about 330 lines added to prepjointree.c), but it's not hugely ugly, and it doesn't add any meaningful overhead in cases where no optimization happens. Barring objections I will commit this. regards, tom lane diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c index 775f482..0d3db71 100644 *** a/src/backend/nodes/outfuncs.c --- b/src/backend/nodes/outfuncs.c *** _outPlannerInfo(StringInfo str, const Pl *** 1762,1767 --- 1762,1768 WRITE_BOOL_FIELD(hasInheritedTarget); WRITE_BOOL_FIELD(hasJoinRTEs); WRITE_BOOL_FIELD(hasLateralRTEs); + WRITE_BOOL_FIELD(hasDeletedRTEs); WRITE_BOOL_FIELD(hasHavingQual); WRITE_BOOL_FIELD(hasPseudoConstantQuals); WRITE_BOOL_FIELD(hasRecursion); diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index b02a107..88b91f1 100644 *** a/src/backend/optimizer/plan/planner.c --- b/src/backend/optimizer/plan/planner.c *** subquery_planner(PlannerGlobal *glob, Qu *** 352,359 * Check to see if any subqueries in the jointree can be merged into this * query. */ ! parse-jointree = (FromExpr *) ! pull_up_subqueries(root, (Node *) parse-jointree); /* * If this is a simple UNION ALL query, flatten it into an appendrel. We --- 352,358 * Check to see if any subqueries in the jointree can be merged into this * query. */ ! pull_up_subqueries(root); /* * If this is a simple UNION ALL query, flatten it into an appendrel. We diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c index 8a0199b..50acfe4 100644 *** a/src/backend/optimizer/prep/prepjointree.c --- b/src/backend/optimizer/prep/prepjointree.c *** static Node *pull_up_sublinks_qual_recur *** 65,76 static Node *pull_up_subqueries_recurse(PlannerInfo *root, Node *jtnode, JoinExpr *lowest_outer_join, JoinExpr *lowest_nulling_outer_join, ! AppendRelInfo *containing_appendrel); static Node *pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte, JoinExpr *lowest_outer_join, JoinExpr *lowest_nulling_outer_join, ! AppendRelInfo *containing_appendrel); static Node *pull_up_simple_union_all(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte); static void pull_up_union_leaf_queries(Node *setOp, PlannerInfo *root, --- 65,78 static Node *pull_up_subqueries_recurse(PlannerInfo *root, Node *jtnode, JoinExpr *lowest_outer_join, JoinExpr *lowest_nulling_outer_join, ! AppendRelInfo *containing_appendrel, ! bool deletion_ok); static Node *pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte, JoinExpr *lowest_outer_join, JoinExpr *lowest_nulling_outer_join, ! AppendRelInfo *containing_appendrel, ! bool deletion_ok); static Node *pull_up_simple_union_all(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte); static void pull_up_union_leaf_queries(Node *setOp, PlannerInfo *root, *** static void pull_up_union_leaf_queries(N *** 79,85 static
Re: [HACKERS] Strange debug message of walreciver?
On Sun, Mar 8, 2015 at 8:13 PM, Tatsuo Ishii is...@postgresql.org wrote: On Sat, Mar 7, 2015 at 10:18 PM, Tatsuo Ishii is...@postgresql.org wrote: When I set log_min_messages to debug5 and looked into walreciver log, I saw this: 3600 2015-03-08 09:47:38 JST DEBUG: sendtime 2015-03-08 09:47:38.31493+09 receipttime 2015-03-08 09:47:38.315027+09 replication apply delay -1945478837 ms tran sfer latency 0 ms The replication apply delay -1945478837 ms part looks strange because the delay is below 0. The number is formatted as %d in elog call, and I suspect this is kind of integer overflow. Looking at GetReplicationApplyDelay() in walreceiverfuncs.c I noticed that the integer overflow occurs because sometimes the return of the GetCurrentChunkReplayStartTime() is 0 (zero). I added an elog into GetReplicationApplyDelay to check this info: DEBUG: GetReplicationApplyDelay 479099832 seconds, 352 milliseconds, (0.00, 479099832352083.00) DEBUG: sendtime 2015-03-08 00:17:12.351987-03 receipttime 2015-03-08 00:17:12.352043-03 replication apply delay -1936504800 ms transfer latency 0 ms DEBUG: GetReplicationApplyDelay 479099841 seconds, 450 milliseconds, (0.00, 479099841450320.00) DEBUG: sendtime 2015-03-08 00:17:21.45018-03 receipttime 2015-03-08 00:17:21.450294-03 replication apply delay -1936495702 ms transfer latency 0 ms Right. Until walreceiver gets the first WAL record to replay, xlogctl-currentChunkStartTime remains 0. Maybe we should check before and return zero from GetReplicationApplyDelay. The attached patch implement it. Your patch regards 0 replication apply delay in the case above which is confusing if the delay is actually 0. What about something like this to explicitly stats the delay data is not available? elog(DEBUG2, sendtime %s receipttime %s replication apply delay (N/A) transfer latency %d ms, Makes sense. Attached patch implement what you suggested. Looks good. If there's no objection, I will commit this. Best regards, -- Tatsuo Ishii SRA OSS, Inc. Japan English: http://www.sraoss.co.jp/index_en.php Japanese:http://www.sraoss.co.jp -- 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] Bootstrap DATA is a pita
Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: And even if it turns out to actually be bothersome, you can help yourself by passing -U 5/setting diff.context = 5 or something like that. Um. Good luck with getting every patch submitter to do that. Can we do it centrally somehow? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, 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
Re: [HACKERS] improving speed of make check-world
On Sun, Mar 8, 2015 at 10:22 PM, Peter Eisentraut pete...@gmx.net wrote:
On 2/24/15 3:06 AM, Michael Paquier wrote:
On Sun, Feb 15, 2015 at 11:01 AM, Peter Eisentraut wrote:
Here is an updated patch.
Nice patch. This is going to save a lot of resources.
An update of vcregress.pl is necessary. This visibly just consists in
updating the options that have been renamed in pg_regress (don't mind
testing any code sent out).
Well, that turns out to be more complicated than initially thought.
Apparently, the msvc has a bit of a different idea of what check and
installcheck do with respect to temporary installs. For instance,
vcregress installcheck does not use psql from the installation but from
the build tree. vcregress check uses psql from the build tree but other
binaries (initdb, pg_ctl) from the temporary installation. It is hard
for me to straighten this out by just looking at the code. Attached is
a patch that shows the idea, but I can't easily take it further than that.
Urg. Yes for installcheck I guess that we cannot do much but simply
use the psql from the tree, but on the contrary for all the other
targets we can use the temporary installation as $topdir/tmp_install.
Regarding the patch you sent, IMO it is not a good idea to kick
install with system() as this can fail as an unrecognized command
runnable. And the command that should be used is not vcregress
install $path but simply vcregress install. Hence I think that
calling simply Install() makes more sense. Also, I think that it would
be better to not enforce PATH before kicking the commands.
Speaking of which, attached is a patch rewritten in-line with those
comments, simplifying a bit the whole at the same time. Note this
patch changes ecpgcheck as it should be patched, but as ecpgcheck test
is broken even on HEAD, I'll raise a separate thread about that with a
proper fix (for example bowerbird skips this test).
On my side, with this patch, installcheck, check, plcheck,
upgradecheck work properly and all of them use the common
installation. It would be more adapted to add checks on the existence
of $tmp_installdir/bin though in InstallTemp instead of kicking an
installation all the time. But that's simple enough to change.
Regards,
--
Michael
From 2b4551a7bc411fc03703f2641b655faf76f2c679 Mon Sep 17 00:00:00 2001
From: Michael Paquier mich...@otacoo.com
Date: Sun, 8 Mar 2015 22:39:10 -0700
Subject: [PATCH] Make vcregress use common installation path for all tests
installcheck is let as-is as it depends on psql being present in PATH.
---
src/tools/msvc/vcregress.pl | 66 +
1 file changed, 43 insertions(+), 23 deletions(-)
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index bd3dd2c..aa7fbaa 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -16,6 +16,7 @@ my $startdir = getcwd();
chdir ../../.. if (-d ../../../src/tools/msvc);
my $topdir = getcwd();
+my $tmp_installdir = $topdir/tmp_install;
require 'src/tools/msvc/config_default.pl';
require 'src/tools/msvc/config.pl' if (-f 'src/tools/msvc/config.pl');
@@ -94,7 +95,7 @@ sub installcheck
my @args = (
../../../$Config/pg_regress/pg_regress,
--dlpath=.,
- --psqldir=../../../$Config/psql,
+ --bindir=../../../$Config/psql,
--schedule=${schedule}_schedule,
--encoding=SQL_ASCII,
--no-locale);
@@ -106,15 +107,19 @@ sub installcheck
sub check
{
+ chdir $startdir;
+
+ InstallTemp();
+ chdir ${topdir}/src/test/regress;
+
my @args = (
- ../../../$Config/pg_regress/pg_regress,
+ ${tmp_installdir}/bin/pg_regress,
--dlpath=.,
- --psqldir=../../../$Config/psql,
+ --bindir=${tmp_installdir}/bin,
--schedule=${schedule}_schedule,
--encoding=SQL_ASCII,
--no-locale,
- --temp-install=./tmp_check,
- --top-builddir=\$topdir\);
+ --temp-instance=./tmp_check);
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
system(@args);
@@ -125,20 +130,24 @@ sub check
sub ecpgcheck
{
chdir $startdir;
+
system(msbuild ecpg_regression.proj /p:config=$Config);
my $status = $? 8;
exit $status if $status;
+
+ InstallTemp();
chdir $topdir/src/interfaces/ecpg/test;
+
$schedule = ecpg;
my @args = (
- ../../../../$Config/pg_regress_ecpg/pg_regress_ecpg,
- --psqldir=../../../$Config/psql,
+ ${tmp_installdir}/bin/pg_regress_ecpg,
+ --bindir=${tmp_installdir}/bin,
--dbname=regress1,connectdb,
--create-role=connectuser,connectdb,
--schedule=${schedule}_schedule,
--encoding=SQL_ASCII,
--no-locale,
- --temp-install=./tmp_chk,
+ --temp-instance=./tmp_chk,
--top-builddir=\$topdir\);
push(@args, $maxconn) if $maxconn;
system(@args);
@@ -148,12 +157,14 @@ sub ecpgcheck
sub isolationcheck
{
- chdir ../isolation;
- copy(../../../$Config/isolationtester/isolationtester.exe,
- ../../../$Config/pg_isolation_regress);
+ chdir $startdir;
+
+ InstallTemp();
+ chdir ${topdir}/src/test/isolation;
+
my
Re: [HACKERS] [REVIEW] Re: Compression of full-page-writes
On Mon, Feb 16, 2015 at 9:08 PM, Andres Freund and...@2ndquadrant.com wrote: On 2015-02-16 20:55:20 +0900, Michael Paquier wrote: On Mon, Feb 16, 2015 at 8:30 PM, Syed, Rahila rahila.s...@nttdata.com wrote: Regarding the sanity checks that have been added recently. I think that they are useful but I am suspecting as well that only a check on the record CRC is done because that's reliable enough and not doing those checks accelerates a bit replay. So I am thinking that we should simply replace them by assertions. Removing the checks makes sense as CRC ensures correctness . Moreover ,as error message for invalid length of record is present in the code , messages for invalid block length can be redundant. Checks have been replaced by assertions in the attached patch. After more thinking, we may as well simply remove them, an error with CRC having high chances to complain before reaching this point... Surely not. The existing code explicitly does it like if (blk-has_data blk-data_len == 0) report_invalid_record(state, BKPBLOCK_HAS_DATA set, but no data included at %X/%X, (uint32) (state-ReadRecPtr 32), (uint32) state-ReadRecPtr); these cross checks are important. And I see no reason to deviate from that. The CRC sum isn't foolproof - we intentionally do checks at several layers. And, as you can see from some other locations, we actually try to *not* fatally error out when hitting them at times - so an Assert also is wrong. Heikki: /* cross-check that the HAS_DATA flag is set iff data_length 0 */ if (blk-has_data blk-data_len == 0) report_invalid_record(state, BKPBLOCK_HAS_DATA set, but no data included at %X/%X, (uint32) (state-ReadRecPtr 32), (uint32) state-ReadRecPtr); if (!blk-has_data blk-data_len != 0) report_invalid_record(state, BKPBLOCK_HAS_DATA not set, but data length is %u at %X/%X, (unsigned int) blk-data_len, (uint32) (state-ReadRecPtr 32), (uint32) state-ReadRecPtr); those look like they're missing a goto err; to me. Yes. I pushed the fix. Thanks! Regards, -- Fujii Masao -- 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] polymorphic types - enforce casting to most common type automatically
2015-03-08 19:31 GMT+01:00 Pavel Stehule pavel.steh...@gmail.com:
Hi
I am sending a proof concept. Current implementation is not suboptimal - I
wrote this code for demonstration of current issues, and checking possible
side effects of changes in this patch.
I am sorry - typo Current implementation (patch) ***is*** suboptimal
Best regards
Pavel
The basic problem is strong restrictive implementation of polymorphic
types - now these types doesn't allow any cast although it is possible. It
can be changed relatively simply I though (after we implemented variadic
functions).
CREATE OR REPLACE FUNCTION public.foo1(anyelement, anyelement)
RETURNS anyelement
LANGUAGE sql
AS $function$
SELECT $1 + $2;
$function$
CREATE OR REPLACE FUNCTION public.foo2(anyelement, anyelement)
RETURNS anyarray
LANGUAGE sql
AS $function$
SELECT ARRAY[$1, $2]
$function$
Now, polymorphic functions don't allow some natively expected calls:
postgres=# select foo1(1,1);
foo1
--
2
(1 row)
postgres=# select foo1(1,1.1);
ERROR: function foo1(integer, numeric) does not exist
LINE 1: select foo1(1,1.1);
^
HINT: No function matches the given name and argument types. You might
need to add explicit type casts.
postgres=# select foo2(1,1);
foo2
---
{1,1}
(1 row)
postgres=# select foo2(1,1.1);
ERROR: function foo2(integer, numeric) does not exist
LINE 1: select foo2(1,1.1);
^
HINT: No function matches the given name and argument types. You might
need to add explicit type casts.
CREATE OR REPLACE FUNCTION public.foo3(VARIADIC anyarray)
RETURNS anyelement
LANGUAGE sql
AS $function$
SELECT min(v) FROM unnest($1) g(v)
$function$
postgres=# SELECT foo3(1,2,3);
foo3
--
1
(1 row)
postgres=# SELECT foo3(1,2,3.1);
ERROR: function foo3(integer, integer, numeric) does not exist
LINE 1: SELECT foo3(1,2,3.1);
^
HINT: No function matches the given name and argument types. You might
need to add explicit type casts.
Some our functions like COALESCE are not too restrictive and allow to use
types from same category.
postgres=# select coalesce(1,1.1);
coalesce
--
1
(1 row)
With attached patch the polymorphic functions use same mechanism as our
buildin functions. It is applied on ANYARRAY, ANYELEMENT types only.
postgres=# select foo1(1,1.1), foo2(1,1.1), foo3(1.1,2,3.1);
foo1 | foo2 | foo3
--+-+--
2.1 | {1,1.1} | 1.1
(1 row)
Comments, notices, ... ?
Regards
Pavel
2014-11-24 20:52 GMT+01:00 Pavel Stehule pavel.steh...@gmail.com:
Hello
now a functions with more than one polymorphic arguments are relative
fragile due missing casting to most common type. Some our functions like
coalesce can do it, so it is surprising for our users.
our custom polymorphic function foo(anyelement, anyelement) working well
for
foo(10,20) or foo(10.1, 20.1), but not for foo(10, 20.1)
I am thinking, so we can add a searching most common type stage without
breaking to backing compatibility.
What do you think about it?
Regards
Pavel
Re: [HACKERS] In-core regression tests for replication, cascading, archiving, PITR, etc.
On Mon, Dec 2, 2013 at 7:07 PM, Andres Freund and...@2ndquadrant.com wrote: On 2013-12-02 18:45:37 +0900, Michael Paquier wrote: On Mon, Dec 2, 2013 at 6:24 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: +1. The need for such a test suite has been mentioned every single time that a bug or new feature related to replication, PITR or hot standby has come up. So yes please! The only thing missing is someone to actually write the thing. So if you have the time and energy, that'd be great! I am sure you know who we need to convince in this case :) If you're alluding to Tom, I'd guess he doesn't need to be convinced of such a facility in general. I seem to remember him complaining about the lack of testing that as well. Maybe that it shouldn't be part of the main regression schedule... +many from me as well. I think the big battle will be how to do it, not if in general. (Reviving an old thread) So I am planning to seriously focus soon on this stuff, basically using the TAP tests as base infrastructure for this regression test suite. First, does using the TAP tests sound fine? On the top of my mind I got the following items that should be tested: - WAL replay: from archive, from stream - hot standby and read-only queries - node promotion - recovery targets and their interferences when multiple targets are specified (XID, name, timestamp, immediate) - timelines - recovery_target_action - recovery_min_apply_delay (check that WAL is fetch from a source at some correct interval, can use a special restore_command for that) - archive_cleanup_command (check that command is kicked at each restart point) - recovery_end_command (check that command is kicked at the end of recovery) - timeline jump of a standby after reconnecting to a promoted node Regards, -- Michael -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improving speed of make check-world
On 2/24/15 3:06 AM, Michael Paquier wrote:
On Sun, Feb 15, 2015 at 11:01 AM, Peter Eisentraut wrote:
Here is an updated patch.
Nice patch. This is going to save a lot of resources.
An update of vcregress.pl is necessary. This visibly just consists in
updating the options that have been renamed in pg_regress (don't mind
testing any code sent out).
Well, that turns out to be more complicated than initially thought.
Apparently, the msvc has a bit of a different idea of what check and
installcheck do with respect to temporary installs. For instance,
vcregress installcheck does not use psql from the installation but from
the build tree. vcregress check uses psql from the build tree but other
binaries (initdb, pg_ctl) from the temporary installation. It is hard
for me to straighten this out by just looking at the code. Attached is
a patch that shows the idea, but I can't easily take it further than that.
- {top-builddir, required_argument, NULL, 11},
+ {datadir, required_argument, NULL, 12},
In pg_regress.c datadir is a new option but it is used nowhere, so it
could be as well removed.
Yeah, that's an oversight that is easily corrected.
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index bd3dd2c..1e09c74 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -94,7 +94,7 @@ sub installcheck
my @args = (
../../../$Config/pg_regress/pg_regress,
--dlpath=.,
- --psqldir=../../../$Config/psql,
+ --bindir=../../../$Config/psql,
--schedule=${schedule}_schedule,
--encoding=SQL_ASCII,
--no-locale);
@@ -106,39 +106,56 @@ sub installcheck
sub check
{
+ my $cwd = getcwd();
+
+ system($0, 'install.pl', $cwd/tmp_install);
+ my $status = $? 8;
+ exit $status if $status;
+
+ $ENV{PATH} = $cwd/tmp_install/bin;$cwd/tmp_install/lib;$ENV{PATH};
+
my @args = (
../../../$Config/pg_regress/pg_regress,
--dlpath=.,
- --psqldir=../../../$Config/psql,
+ --bindir=,
--schedule=${schedule}_schedule,
--encoding=SQL_ASCII,
--no-locale,
- --temp-install=./tmp_check,
- --top-builddir=\$topdir\);
+ --temp-instance=./tmp_check);
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
system(@args);
- my $status = $? 8;
+ $status = $? 8;
exit $status if $status;
}
sub ecpgcheck
{
chdir $startdir;
+
system(msbuild ecpg_regression.proj /p:config=$Config);
my $status = $? 8;
exit $status if $status;
+
+ my $cwd = getcwd();
+
+ system($0, 'install.pl', $cwd/tmp_install);
+ $status = $? 8;
+ exit $status if $status;
+
+ $ENV{PATH} = $cwd/tmp_install/bin;$cwd/tmp_install/lib;$ENV{PATH};
+
chdir $topdir/src/interfaces/ecpg/test;
$schedule = ecpg;
my @args = (
../../../../$Config/pg_regress_ecpg/pg_regress_ecpg,
- --psqldir=../../../$Config/psql,
+ --bindir=,
--dbname=regress1,connectdb,
--create-role=connectuser,connectdb,
--schedule=${schedule}_schedule,
--encoding=SQL_ASCII,
--no-locale,
- --temp-install=./tmp_chk,
+ --temp-instance=./tmp_chk,
--top-builddir=\$topdir\);
push(@args, $maxconn) if $maxconn;
system(@args);
@@ -153,7 +170,7 @@ sub isolationcheck
../../../$Config/pg_isolation_regress);
my @args = (
../../../$Config/pg_isolation_regress/pg_isolation_regress,
- --psqldir=../../../$Config/psql,
+ --bindir=../../../$Config/psql,
--inputdir=.,
--schedule=./isolation_schedule);
push(@args, $maxconn) if $maxconn;
@@ -202,7 +219,7 @@ sub plcheck
print Checking $lang\n;
my @args = (
../../../$Config/pg_regress/pg_regress,
- --psqldir=../../../$Config/psql,
+ --bindir=../../../$Config/psql,
--dbname=pl_regression, @lang_args, @tests);
system(@args);
my $status = $? 8;
@@ -237,7 +254,7 @@ sub contribcheck
my @opts = fetchRegressOpts();
my @args = (
../../$Config/pg_regress/pg_regress,
- --psqldir=../../$Config/psql,
+ --bindir=../../$Config/psql,
--dbname=contrib_regression, @opts, @tests);
system(@args);
my $status = $? 8;
--
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] Join push-down support for foreign tables
Hi Ashutosh, Thanks for finding out what we oversight. Here is still a problem because the new 'relids' field is not updated on setrefs.c (scanrelid is incremented by rtoffset here). It is easy to shift the bitmapset by rtoffset, however, I also would like to see another approach. My idea adds 'List *fdw_sub_paths' field in ForeignPath to inform planner underlying foreign-scan paths (with scanrelid 0). The create_foreignscan_plan() will call create_plan_recurse() to construct plan nodes based on the path nodes being attached. Even though these foreign-scan nodes are not actually executed, setrefs.c can update scanrelid in usual way and ExplainPreScanNode does not need to take exceptional handling on Foreign/CustomScan nodes. In addition, it allows to keep information about underlying foreign table scan, even if planner will need some other information in the future version (not only relids). How about your thought? -- NEC OSS Promotion Center / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com -Original Message- From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Ashutosh Bapat Sent: Friday, March 06, 2015 7:26 PM To: Kaigai Kouhei(海外 浩平) Cc: Shigeru Hanada; Robert Haas; PostgreSQL-development Subject: Re: [HACKERS] Join push-down support for foreign tables Hi Kaigai-san, Hanada-san, Attached please find a patch to print the column names prefixed by the relation names. I haven't tested the patch fully. The same changes will be needed for CustomPlan node specific code. Now I am able to make sense out of the Output information postgres=# explain verbose select * from ft1 join ft2 using (val); QUERY PLAN --- --- Foreign Scan (cost=100.00..125.60 rows=2560 width=12) Output: ft1.val, ft1.val2, ft2.val2 Remote SQL: SELECT r.a_0, r.a_1, l.a_1 FROM (SELECT val, val2 FROM public.lt) l (a_0, a_1) INNER JOIN (SELECT val, val2 FROM public.lt) r (a_0, a_1) ON ((r.a_0 = l.a_0)) (3 rows) On Fri, Mar 6, 2015 at 6:41 AM, Kouhei Kaigai kai...@ak.jp.nec.com wrote: Actually val and val2 come from public.lt in r side, but as you say it's too difficult to know that from EXPLAIN output. Do you have any idea to make the Output item more readable? A fundamental reason why we need to have symbolic aliases here is that postgres_fdw has remote query in cstring form. It makes implementation complicated to deconstruct/construct a query that is once constructed on the underlying foreign-path level. If ForeignScan keeps items to construct remote query in expression node form (and construction of remote query is delayed to beginning of the executor, probably), we will be able to construct more human readable remote query. However, I don't recommend to work on this great refactoring stuff within the scope of join push-down support project. Thanks, -- NEC OSS Promotion Center / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com -Original Message- From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Shigeru Hanada Sent: Thursday, March 05, 2015 10:00 PM To: Ashutosh Bapat Cc: Kaigai Kouhei(海外 浩平); Robert Haas; PostgreSQL-development Subject: Re: [HACKERS] Join push-down support for foreign tables Hi Ashutosh, thanks for the review. 2015-03-04 19:17 GMT+09:00 Ashutosh Bapat ashutosh.ba...@enterprisedb.com: In create_foreignscan_path() we have lines like - 1587 pathnode-path.param_info = get_baserel_parampathinfo(root, rel, 1588 required_outer); Now, that the same function is being used for creating foreign scan paths for joins, we should be calling get_joinrel_parampathinfo() on a join rel and get_baserel_parampathinfo() on base rel. Got it. Please let me check the difference. The patch seems to handle all the restriction clauses in the same way. There are two kinds of restriction clauses - a. join quals (specified using ON clause; optimizer might move them to the other class if that doesn't affect correctness) and b. quals on join relation (specified in the WHERE clause, optimizer might move them to the other class if that doesn't affect correctness). The quals in a are applied while the join is being computed whereas those in b are applied after the join is computed. For example, postgres=# select * from lt; val | val2 -+-- 1 |2 1 |3
Re: [HACKERS] Strange debug message of walreciver?
On Sun, Mar 8, 2015 at 8:13 PM, Tatsuo Ishii is...@postgresql.org wrote:
On Sat, Mar 7, 2015 at 10:18 PM, Tatsuo Ishii is...@postgresql.org
wrote:
When I set log_min_messages to debug5 and looked into walreciver log,
I saw this:
3600 2015-03-08 09:47:38 JST DEBUG: sendtime 2015-03-08
09:47:38.31493+09 receipttime 2015-03-08 09:47:38.315027+09 replication
apply delay -1945478837 ms tran sfer latency 0 ms
The replication apply delay -1945478837 ms part looks strange
because the delay is below 0. The number is formatted as %d in elog
call, and I suspect this is kind of integer overflow.
Looking at GetReplicationApplyDelay() in walreceiverfuncs.c I noticed
that
the integer overflow occurs because sometimes the return of the
GetCurrentChunkReplayStartTime() is 0 (zero).
I added an elog into GetReplicationApplyDelay to check this info:
DEBUG: GetReplicationApplyDelay 479099832 seconds, 352 milliseconds,
(0.00, 479099832352083.00)
DEBUG: sendtime 2015-03-08 00:17:12.351987-03 receipttime 2015-03-08
00:17:12.352043-03 replication apply delay -1936504800 ms transfer
latency
0 ms
DEBUG: GetReplicationApplyDelay 479099841 seconds, 450 milliseconds,
(0.00, 479099841450320.00)
DEBUG: sendtime 2015-03-08 00:17:21.45018-03 receipttime 2015-03-08
00:17:21.450294-03 replication apply delay -1936495702 ms transfer
latency
0 ms
Right. Until walreceiver gets the first WAL record to replay,
xlogctl-currentChunkStartTime remains 0.
Maybe we should check before and return zero from
GetReplicationApplyDelay.
The attached patch implement it.
Your patch regards 0 replication apply delay in the case above which
is confusing if the delay is actually 0.
What about something like this to explicitly stats the delay data is
not available?
elog(DEBUG2, sendtime %s receipttime %s replication apply delay (N/A)
transfer latency %d ms,
Makes sense. Attached patch implement what you suggested.
Regards,
--
Fabrízio de Royes Mello
Consultoria/Coaching PostgreSQL
Timbira: http://www.timbira.com.br
Blog: http://fabriziomello.github.io
Linkedin: http://br.linkedin.com/in/fabriziomello
Twitter: http://twitter.com/fabriziomello
Github: http://github.com/fabriziomello
diff --git a/src/backend/replication/walreceiver.c b/src/backend/replication/walreceiver.c
index bfbc02f..9c7710f 100644
--- a/src/backend/replication/walreceiver.c
+++ b/src/backend/replication/walreceiver.c
@@ -1191,15 +1191,26 @@ ProcessWalSndrMessage(XLogRecPtr walEnd, TimestampTz sendTime)
{
char *sendtime;
char *receipttime;
+ int applyDelay;
/* Copy because timestamptz_to_str returns a static buffer */
sendtime = pstrdup(timestamptz_to_str(sendTime));
receipttime = pstrdup(timestamptz_to_str(lastMsgReceiptTime));
- elog(DEBUG2, sendtime %s receipttime %s replication apply delay %d ms transfer latency %d ms,
- sendtime,
- receipttime,
- GetReplicationApplyDelay(),
- GetReplicationTransferLatency());
+ applyDelay = GetReplicationApplyDelay();
+
+ /* apply delay is not available */
+ if (applyDelay == -1)
+ elog(DEBUG2, sendtime %s receipttime %s replication apply delay (N/A) transfer latency %d ms,
+ sendtime,
+ receipttime,
+ GetReplicationTransferLatency());
+ else
+ elog(DEBUG2, sendtime %s receipttime %s replication apply delay %d ms transfer latency %d ms,
+ sendtime,
+ receipttime,
+ applyDelay,
+ GetReplicationTransferLatency());
+
pfree(sendtime);
pfree(receipttime);
}
diff --git a/src/backend/replication/walreceiverfuncs.c b/src/backend/replication/walreceiverfuncs.c
index 496605f..b26f5fc 100644
--- a/src/backend/replication/walreceiverfuncs.c
+++ b/src/backend/replication/walreceiverfuncs.c
@@ -314,7 +314,8 @@ GetWalRcvWriteRecPtr(XLogRecPtr *latestChunkStart, TimeLineID *receiveTLI)
}
/*
- * Returns the replication apply delay in ms
+ * Returns the replication apply delay in ms or -1
+ * if the apply delay info is not available
*/
int
GetReplicationApplyDelay(void)
@@ -328,6 +329,8 @@ GetReplicationApplyDelay(void)
long secs;
int usecs;
+ TimestampTz chunckReplayStartTime;
+
SpinLockAcquire(walrcv-mutex);
receivePtr = walrcv-receivedUpto;
SpinLockRelease(walrcv-mutex);
@@ -337,7 +340,12 @@ GetReplicationApplyDelay(void)
if (receivePtr == replayPtr)
return 0;
- TimestampDifference(GetCurrentChunkReplayStartTime(),
+ chunckReplayStartTime = GetCurrentChunkReplayStartTime();
+
+ if (chunckReplayStartTime == 0)
+ return -1;
+
+ TimestampDifference(chunckReplayStartTime,
GetCurrentTimestamp(),
secs, usecs);
--
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] Bootstrap DATA is a pita
On 03/08/2015 10:11 PM, Alvaro Herrera wrote: Tom Lane wrote: Andres Freund and...@2ndquadrant.com writes: And even if it turns out to actually be bothersome, you can help yourself by passing -U 5/setting diff.context = 5 or something like that. Um. Good luck with getting every patch submitter to do that. Can we do it centrally somehow? I don't believe there is provision for setting diff.context on a per file basis. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Rethinking the parameter access hooks for plpgsql's benefit
We already knew that plpgsql's setup_param_list() is a bit of a
performance hog. Trying to improve that was the primary motivation
behind commit f4e031c662a6b600b786c4849968a099c58fcce7 (the
bms_next_member business), though that was just micro-optimization
rather than any fundamental rethink of how things are done.
Some benchmarking at Salesforce has shown that there's a really
significant penalty in plpgsql functions that have a lot of local
variables, because large ndatums increases the size of the
ParamListInfo array that has to be allocated (and zeroed!) for
each expression invocation.
Thinking about this, it struck me that the real problem is that
the parameter list APIs are still too stuck in the last century,
in that they're optimized for static lists of parameters which
is not what plpgsql needs at all. In fact, if we redefined the
usage of ParamFetchHook so that it would be called on every parameter
fetch, plpgsql could be perfectly happy with just *one* ParamListInfo
slot that it would re-fill each time. This would entirely eliminate
the need for any per-expression-execution ParamListInfo allocation,
because a function could just use one single-entry ParamListInfo array
for all purposes.
The attached proposed patch represents an exploration of this idea.
I've also attached a SQL script with some simple functions for testing
its performance. The first one is for exploring what happens with more
or fewer local variables. I compared current HEAD against the patch
with asserts off, for a call like SELECT timingcheck(1000);.
It looks like this (times in ms):
HEADpatch
no extra variables 86958390
10 extra variables 92188419
30 extra variables 94248255
100 extra variables 94338202
These times are only repeatable to within a few percent, but they do show
that there is a gain rather than loss of performance even in the base
case, and that the patched code's performance is pretty much independent
of the number of local variables whereas HEAD isn't.
The case where the patch potentially loses is where a single expression
contains multiple references to the same plpgsql variable, since with
HEAD, additional references to a variable don't incur any extra trips
through the ParamFetchHook, whereas with the patch they do. I set up
the reusecheck10() and reusecheck5() functions to see how bad that is.
HEADpatch
5 uses of same variable 41053962
10 uses of same variable57386076
So somewhere between 5 and 10 uses of the same variable in a single
expression, you start to lose.
I claim that that's a very unlikely scenario and so this patch should
be an unconditional win for just about everybody.
The patch does create an API break for any code that is using
ParamFetchHooks, but it's an easy fix (just return the address of
the array element you stored data into), and any decent C compiler
will complain about the function type mismatch so the API break
should not go unnoticed.
Objections? Even better ideas?
regards, tom lane
diff --git a/src/backend/executor/execCurrent.c b/src/backend/executor/execCurrent.c
index 1c8be25..1504c92 100644
*** a/src/backend/executor/execCurrent.c
--- b/src/backend/executor/execCurrent.c
*** fetch_cursor_param_value(ExprContext *ec
*** 216,226
if (paramInfo
paramId 0 paramId = paramInfo-numParams)
{
! ParamExternData *prm = paramInfo-params[paramId - 1];
/* give hook a chance in case parameter is dynamic */
! if (!OidIsValid(prm-ptype) paramInfo-paramFetch != NULL)
! (*paramInfo-paramFetch) (paramInfo, paramId);
if (OidIsValid(prm-ptype) !prm-isnull)
{
--- 216,228
if (paramInfo
paramId 0 paramId = paramInfo-numParams)
{
! ParamExternData *prm;
/* give hook a chance in case parameter is dynamic */
! if (paramInfo-paramFetch != NULL)
! prm = (*paramInfo-paramFetch) (paramInfo, paramId, 0);
! else
! prm = paramInfo-params[paramId - 1];
if (OidIsValid(prm-ptype) !prm-isnull)
{
diff --git a/src/backend/executor/execQual.c b/src/backend/executor/execQual.c
index d94fe58..f558d40 100644
*** a/src/backend/executor/execQual.c
--- b/src/backend/executor/execQual.c
*** ExecEvalParamExtern(ExprState *exprstate
*** 1137,1147
if (paramInfo
thisParamId 0 thisParamId = paramInfo-numParams)
{
! ParamExternData *prm = paramInfo-params[thisParamId - 1];
/* give hook a chance in case parameter is dynamic */
! if (!OidIsValid(prm-ptype) paramInfo-paramFetch != NULL)
! (*paramInfo-paramFetch) (paramInfo, thisParamId);
if (OidIsValid(prm-ptype))
{
--- 1137,1149
if (paramInfo
thisParamId 0 thisParamId = paramInfo-numParams)
{
! ParamExternData *prm;
/* give hook a chance in case parameter is dynamic */
! if
Re: [HACKERS] Join push-down support for foreign tables
On Mon, Mar 9, 2015 at 5:46 AM, Kouhei Kaigai kai...@ak.jp.nec.com wrote: Hi Ashutosh, Thanks for finding out what we oversight. Here is still a problem because the new 'relids' field is not updated on setrefs.c (scanrelid is incremented by rtoffset here). It is easy to shift the bitmapset by rtoffset, however, I also would like to see another approach. I just made it work for explain, but other parts still need work. Sorry about that. If we follow INDEX_VAR, we should be able to get there. My idea adds 'List *fdw_sub_paths' field in ForeignPath to inform planner underlying foreign-scan paths (with scanrelid 0). The create_foreignscan_plan() will call create_plan_recurse() to construct plan nodes based on the path nodes being attached. Even though these foreign-scan nodes are not actually executed, setrefs.c can update scanrelid in usual way and ExplainPreScanNode does not need to take exceptional handling on Foreign/CustomScan nodes. In addition, it allows to keep information about underlying foreign table scan, even if planner will need some other information in the future version (not only relids). How about your thought? I am not sure about keeping planner nodes, which are not turned into execution nodes. There's no precedence for that in current code. It could be risky. -- NEC OSS Promotion Center / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com -Original Message- From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Ashutosh Bapat Sent: Friday, March 06, 2015 7:26 PM To: Kaigai Kouhei(海外 浩平) Cc: Shigeru Hanada; Robert Haas; PostgreSQL-development Subject: Re: [HACKERS] Join push-down support for foreign tables Hi Kaigai-san, Hanada-san, Attached please find a patch to print the column names prefixed by the relation names. I haven't tested the patch fully. The same changes will be needed for CustomPlan node specific code. Now I am able to make sense out of the Output information postgres=# explain verbose select * from ft1 join ft2 using (val); QUERY PLAN --- --- Foreign Scan (cost=100.00..125.60 rows=2560 width=12) Output: ft1.val, ft1.val2, ft2.val2 Remote SQL: SELECT r.a_0, r.a_1, l.a_1 FROM (SELECT val, val2 FROM public.lt) l (a_0, a_1) INNER JOIN (SELECT val, val2 FROM public.lt) r (a_0, a_1) ON ((r.a_0 = l.a_0)) (3 rows) On Fri, Mar 6, 2015 at 6:41 AM, Kouhei Kaigai kai...@ak.jp.nec.com wrote: Actually val and val2 come from public.lt in r side, but as you say it's too difficult to know that from EXPLAIN output. Do you have any idea to make the Output item more readable? A fundamental reason why we need to have symbolic aliases here is that postgres_fdw has remote query in cstring form. It makes implementation complicated to deconstruct/construct a query that is once constructed on the underlying foreign-path level. If ForeignScan keeps items to construct remote query in expression node form (and construction of remote query is delayed to beginning of the executor, probably), we will be able to construct more human readable remote query. However, I don't recommend to work on this great refactoring stuff within the scope of join push-down support project. Thanks, -- NEC OSS Promotion Center / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com -Original Message- From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Shigeru Hanada Sent: Thursday, March 05, 2015 10:00 PM To: Ashutosh Bapat Cc: Kaigai Kouhei(海外 浩平); Robert Haas; PostgreSQL-development Subject: Re: [HACKERS] Join push-down support for foreign tables Hi Ashutosh, thanks for the review. 2015-03-04 19:17 GMT+09:00 Ashutosh Bapat ashutosh.ba...@enterprisedb.com: In create_foreignscan_path() we have lines like - 1587 pathnode-path.param_info = get_baserel_parampathinfo(root, rel, 1588 required_outer); Now, that the same function is being used for creating foreign scan paths for joins, we should be calling get_joinrel_parampathinfo() on a join rel and get_baserel_parampathinfo() on base rel. Got it. Please let me check the difference. The patch seems to handle all the restriction clauses in the same way. There are two kinds of restriction clauses - a. join quals (specified using ON clause; optimizer might move them to the other
Re: [HACKERS] TABLESAMPLE patch
On Sat, Mar 7, 2015 at 10:37 PM, Petr Jelinek p...@2ndquadrant.com wrote: On 05/03/15 09:21, Amit Kapila wrote: On Tue, Feb 17, 2015 at 3:29 AM, Petr Jelinek p...@2ndquadrant.com mailto:p...@2ndquadrant.com wrote: I didn't add the whole page visibility caching as the tuple ids we get from sampling methods don't map well to the visibility info we get from heapgetpage (it maps to the values in the rs_vistuples array not to to its indexes). Commented about it in code also. I think we should set pagemode for system sampling as it can have dual benefit, one is it will allow us caching tuples and other is it can allow us pruning of page which is done in heapgetpage(). Do you see any downside to it? Double checking for tuple visibility is the only downside I can think of. That will happen if we use heapgetpage and the way currently code is written in patch, however we can easily avoid double checking if we don't call heapgetpage and rather do the required work at caller's place. Plus some added code complexity of course. I guess we could use binary search on rs_vistuples (it's already sorted) so that info won't be completely useless. Not sure if worth it compared to normal visibility check, but not hard to do. It seems to me that it is better to avoid locking/unlocking buffer wherever possible. I personally don't see the page pruning in TABLESAMPLE as all that important though, considering that in practice it will only scan tiny portion of a relation and usually one that does not get many updates (in practice the main use-case is BI). In that case, I think it should be acceptable either way, because if there are less updates then anyway it won't incur any cost of doing the pruning. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] Wrong error message in REINDEX command
Sawada Masahiko sawada.m...@gmail.com writes: I got wrong error message when I did REINDEX SYSTEM command in transaction as follows. It should say ERROR: REINDEX SYSTEM cannot run inside a transaction block Attached patch fixes it. Hm, yeah, looks like ReindexObject() has a similar disease internally (not to mention being very inappropriately named itself...) 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] get_object_address support for additional object types
Alvaro, * Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: This is extracted from the DDL deparse series. These patches add get_object_address support for the following object types: - user mappings - default ACLs - operators and functions of operator families I took a (relatively quick) look through these patches. Subject: [PATCH 1/3] deparse/core: get_object_address support user mappings [...] I thought this looked fine. One minor nit-pick is that the function added doesn't have a single comment, but it's a pretty short too. Subject: [PATCH 2/3] deparse/core: get_object_address support default ACLs [...] + char *stuff; Nit-pick, but 'stuff' isn't really a great variable name. :) Perhaps 'defacltype_name'? It's longer, sure, but it's not used a lot.. Subject: [PATCH 3/3] deparse/core: get_object_address support opfamily members @@ -661,7 +664,8 @@ get_object_address(ObjectType objtype, List *objname, List *objargs, ObjectAddress domaddr; char *constrname; - domaddr = get_object_address_type(OBJECT_DOMAIN, objname, missing_ok); + domaddr = get_object_address_type(OBJECT_DOMAIN, + list_head(objname), missing_ok); constrname = strVal(linitial(objargs)); address.classId = ConstraintRelationId; I don't really care for how all the get_object_address stuff uses lists for arguments instead of using straight-forward arguments, but it's how it's been done and I can kind of see the reasoning behind it. I'm not following why you're switching this case (get_object_address_type) to using a ListCell though..? I thought the rest of it looked alright. I agree it's a bit odd how the opfamily is handled but I agree with your assessment that there's not much better we can do with this object representation. Thanks! Stephen signature.asc Description: Digital signature
Re: [HACKERS] Bootstrap DATA is a pita
On 2015-03-04 10:25:58 -0500, Robert Haas wrote: Another advantage of this is that it would probably make git less likely to fumble a rebase. If there are lots of places in the file where we have the same 10 lines in a row with occasional variations, rebasing a patch could easily pick the the wrong place to reapply the hunk. I would personally consider a substantial increase in the rate of such occurrences as being a cure far, far worse than the disease. If you keep the entry for each function on just a couple of lines the chances of this happening are greatly reduced, because you're much likely to get a false match to surrounding context. I'm not particularly worried about this. Especially with attribute defaults it seems unlikely that you often have the same three surrounding lines in both directions in a similar region of the file. And even if it turns out to actually be bothersome, you can help yourself by passing -U 5/setting diff.context = 5 or something like that. 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
