Re: [HACKERS] BUG: pg_stat_statements query normalization issues with combined queries

2016-12-29 Thread Fabien COELHO


Hello Craig,

[...] It's touching every single utility statement type, which is not 
only pretty invasive in itself, but will break any pending patches 
that add more utility statement types.


Yep, but it is limited to headers and the break is trivial...


I'm not sure how else that part can be done.

We can add a common parent for all utility statement types, but that 
also touches all utility statements, and since we're working with pure 
C, it means 'type' won't be accessible as e.g. someCreateStmt.type, but 
only as something like ((UtilityStatement)someCreateStmt).type or 
someCreateStmt.hdr.type. [...] I don't think that's so bad, but it's not 
exactly less invasive.


I thought of this way of implementing that on the submitted version, I 
decided against because then it is less obvious what is directly in the 
structure, existing code may reference the tag field, and the number of 
header changes is the same, if a little less verbose.


[...] For fixing the information in pg_stat_statement, the location 
data must be transported from the parsed node to the query to the 
planned node, because the later two nodes types are passed to different 
hooks.


Yep.


Now the detail is that utility statements, which seems to be nearly all of
them but select/update/delete/insert, do not have plans: The statement
itself is its own plan... so there is no place to store the location &
length.


Yeah. I don't see any way around adding location info for utility
statements one way or the other.


If utility statements are done this way, that's 95% of all statements, so 
the point of doing the remaining 5% with Tom's neat intermediate node 
trick seems void, I think that it is better to have all of them the same 
way.



TBH I think that for utility statements, adding a member struct with
location info is the least-bad option. Something like:



typedef struct StmtLocation
{
   int offset;
   int length;
}


It is possible, but:


typedef struct CreateStmt
{
   NodeTag type;
   StmtLocation  location;
   
}


Name "location" is already used meaning the offset or the directory 
destination for create table space, that would create a third option.


For homogeneity, ISTM that keeping location & length directly and renaming 
the table space location is the simplest & most homogeneous option.


--
Fabien.


--
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] proposal: session server side variables

2016-12-29 Thread Fabien COELHO


I know this one. It can be empty, which a singleton cannot be. For a 
singleton table, you should have one and only one row, you cannot insert or 
delete, so this is only part of the real thing.


Surely we can do a bit better than that, if that's what you really want.
Create the table with an initial row and add a trigger preventing anything 
except update.


Yes.

I just meant that this is not available easily "out of the box", but it is 
obviously doable with some effort... which people would seldom do.


--
Fabien.


--
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] proposal: session server side variables

2016-12-29 Thread Fabien COELHO



Not sure if term "singleton" is valid in relation database.


The term is valid in mathematics: The relational model is an extension of 
set theory, eg the algebra includes set operators such as union, 
intersection, difference... In the set theory, a singleton designate is a 
exactly-one-element set. At least Wikipedia and my dwindling memories tell 
me so:-)


--
Fabien.


--
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] Potential data loss of 2PC files

2016-12-29 Thread Michael Paquier
On Thu, Dec 29, 2016 at 6:41 PM, Ashutosh Bapat
 wrote:
> I agree with this.
> If no prepared transactions were required to be fsynced
> CheckPointTwoPhase(), do we want to still fsync the directory?
> Probably not.
>
> May be you want to call fsync_fname(TWOPHASE_DIR, true); if
> serialized_xacts > 0.

Definitely true for the non-recovery code path. But not for restart
points as there is no GXACT entry created by the redo routine of 2PC
prepare. We could have a static counter tracking how many 2PC files
have been flushed since last restart point or not but I am not
convinced if that's worth the facility.

> The comment just states what has been done earlier in the function,
> which is documented in the prologue as well.
> /*
>  * Make sure that the content created is persistent on disk to prevent
>  * data loss in case of an OS crash or a power failure. Each 2PC file
>  * has been already created and flushed to disk individually by
>  * RecreateTwoPhaseFile() using the list of GXACTs available for
>  * normal processing as well as at recovery when replaying individually
>  * each XLOG_XACT_PREPARE record.
>  */
> Instead, you may want to just say "If we have flushed any 2PC files,
> flush the metadata in the pg_twophase directory."

That's not true for recovery. So I could go for something like that:
"If any 2PC files have been flushed, do the same for the parent
directory to make this information durable on disk. On recovery, issue
the fsync() anyway, individual 2PC files have already been flushed whe
replaying their respective XLOG_XACT_PREPARE record.

> Although, I thought that a simple case of creating a persistent table
> which requires creating a file also would need to flush the directory.
> I tried to locate the corresponding code to see if it also uses
> fsync_fname(). I couldn't locate the code. I have looked at
> mdcreate(), mdpreckpt(), mdpostckpt(). But may be that's not relevant
> here.

I have been playing with unlogged tables created, then followed by
checkpoints, and a VM plugged off but I could not see those files
getting away. That was on ext4, but like ext3 things can disappear if
the fsync() is forgotten on the parent directory.
-- 
Michael


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


[HACKERS] What is "index returned tuples in wrong order" for recheck supposed to guard against?

2016-12-29 Thread Regina Obe
I've been trying to troubleshoot the cause of this PostGIS recheck bug we
have reported by two people so far.  The last test was a nice simple
repeatable one that triggered the issue:

https://trac.osgeo.org/postgis/ticket/3418


from what I have seen this only affects cases where we are doing a distance
check between two points, which we actually don't need to enable recheck for
anyway, but trying to disable that seems like just shoving the real problem
under the covers.
Where it errors is this line 272 in src/backend/executor/nodeIndexscan

https://git.postgresql.org/gitweb/?p=postgresql.git;a=blob;f=src/backend/exe
cutor/nodeIndexscan.c;h=3143bd94ec4499fba94b41693538b785c4b32e6c;hb=HEAD#l27
2


  /*
 259  * Was the ORDER BY value returned by the index accurate?
The
 260  * recheck flag means that the index can return inaccurate
values,
 261  * but then again, the value returned for any particular
tuple
 262  * could also be exactly correct.  Compare the value
returned by
 263  * the index with the recalculated value.  (If the value
returned
 264  * by the index happened to be exact right, we can often
avoid
 265  * pushing the tuple to the queue, just to pop it back out
again.)
 266  */
 267 cmp = cmp_orderbyvals(node->iss_OrderByValues,
 268   node->iss_OrderByNulls,
 269   scandesc->xs_orderbyvals,
 270   scandesc->xs_orderbynulls,
 271   node);
 272 if (cmp < 0)
 273 elog(ERROR, "index returned tuples in wrong order");
 274 else if (cmp == 0)
 275 was_exact = true;
 276 else
 277 was_exact = false;

If things are out of order, why isn't just going to was_exact = false good
enough?

I'm not sure if the mistake is in our PostGIS code or something in
PostgreSQL recheck logic.
If I change the elog(ERROR ...) to a elog(NOTICE, the answers  are correct
and sort order is right.

Under what conditions would cmp return less than 0?  I tried following the
code in cmp_orderbyvals, but got lost
and trying to put elog notices in to see what the distance is returning (I
probably did it wrong), just ended up crashing by backend.


Thanks for any thoughts,
Regina





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


[HACKERS] Broken atomics code on PPC with FreeBSD 10.3

2016-12-29 Thread Tom Lane
In pursuit of knowledge about clock_gettime(), I installed FreeBSD 10.3
on a PowerBook I had laying about.  I was disappointed to find out that
HEAD fails regression tests on that platform:

*** /usr/home/tgl/pgsql/src/test/regress/expected/lock.out  Thu Dec 29 
19:37:50 2016
--- /usr/home/tgl/pgsql/src/test/regress/results/lock.out   Fri Dec 30 
00:31:01 2016
***
*** 63,70 
  -- atomic ops tests
  RESET search_path;
  SELECT test_atomic_ops();
!  test_atomic_ops 
! -
!  t
! (1 row)
! 
--- 63,66 
  -- atomic ops tests
  RESET search_path;
  SELECT test_atomic_ops();
! ERROR:  flag: set spuriously #2

==

After some digging I found out that the atomics code appears to believe
that the PPC platform has byte-wide atomics, which of course is nonsense.
That causes it to define pg_atomic_flag with the "char" variant, and
what we end up with is word-wide operations (lwarx and friends) operating
on a byte-wide struct.  Failure is not exactly astonishing; what's
surprising is that we don't get stack-clobber core dumps or such.
Undef'ing HAVE_GCC__SYNC_CHAR_TAS makes it work.

Perhaps it could be argued that there's a FreeBSD compiler bug here,
but what I'm wondering is why configure believes that this:

  [char lock = 0;
   __sync_lock_test_and_set(, 1);
   __sync_lock_release();])],

is going to draw a hard error on platforms without byte-wide atomics.
My sense of C semantics is that the best you could hope for is a warning
--- and a look at the config.log on this platform says we're not
even getting that.

In short, I'd put the blame here on a ridiculously optimistic configure
check.

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] Support for pg_receivexlog --format=plain|tar

2016-12-29 Thread Michael Paquier
On Thu, Dec 29, 2016 at 6:13 PM, Magnus Hagander  wrote:
> On Thu, Dec 29, 2016 at 12:35 AM, Michael Paquier
>  wrote:
>> On Wed, Dec 28, 2016 at 9:31 PM, Magnus Hagander 
>> wrote:
>> >> - I have switched the directory method to use a file pointer instead
>> >> of a file descriptor as gzwrite returns int as the number of
>> >> uncompressed bytes written.
>> >
>> > I don't really follow that reasoning :) Why does the directory method
>> > have
>> > to change to use a filepointer because of that?
>>
>> The only reason is that write() returns size_t and fwrite returns int,
>> while gzwrite() returns int. It seems more consistent to use fwrite()
>> in this case. Or we don't bother about my nitpicking and just cast
>> stuff.
>
>
> I can at least partially see that argument, but your patch doesn't actually
> use fwrite(), it uses write() with fileno()...

That was part of the one/two things I wanted to change before sending
a fresh patch.

> But also, on my platform (debian jessie), fwrite() returns size_t, and
> write() returns ssize_t. So those are apparently both different from what
> your platform does - which one did you get that one?

It looks like I misread the macos man pages previously. Thay actually
list ssize_t. I find a bit surprising the way gzwrite is designed. It
uses an input an unsigned integer and returns to caller a signed
integer, so this will never work with uncompressed buffers of sizes
higher than 2GB. There's little point to worry about that in
pg_receivexlog though, so let's just cast to ssize_t.

Attached is a simplified new version, I have kept the file descriptor
as originally done. Note that tests are actually difficult to work
out, there is no way to run in batch pg_receivexlog..
-- 
Michael


receivexlog-gzip-v2.patch
Description: application/stream

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


Re: [HACKERS] BUG: pg_stat_statements query normalization issues with combined queries

2016-12-29 Thread Craig Ringer
On 28 December 2016 at 20:11, Fabien COELHO  wrote:
>
> Hello Tom,
>
>> [...] It's touching every single utility statement type, which is not only
>> pretty invasive in itself, but will break any pending patches that add more
>> utility statement types.
>
>
> Yep, but it is limited to headers and the break is trivial...

I'm not sure how else that part can be done.

We can add a common parent for all utility statement types, but that
also touches all utility statements, and since we're working with pure
C, it means 'type' won't be accessible as e.g. someCreateStmt.type,
but only as something like ((UtilityStatement)someCreateStmt).type or
someCreateStmt.hdr.type .

e.g.

typedef struct UtilityStatement
{
   NodeTag type;
   int querytext_offset;
   int querytext_length;
};

typedef struct CreateStmt
{
UtilityStatement hdr;
RangeVar   *relation;   /* relation to create */

}

I don't think that's so bad, but it's not exactly less invasive.

>> And you've not followed through on the implications of adding those fields
>> in, for instance, src/backend/nodes/;
>
> Hmmm, probably you are pointing out structure read/write functions.

I'm sure that's his intent, yes.

> I would have hoped that such code would be automatically derived from the
> corresponding struct definition...

That would be nice. But no, unfortunately, we have no such preprocessing.

To do so would require that we either generate things like
parsenodes.h and the relevant parts of copyfuncs.c etc from a common
source, or have a toolchain that can ingest parsenodes.h and emit
suitable code. The latter means either "fragile, hideous Perl script"
or requiring something like LLVM or the gcc frontend libraries in the
toolchain to parse the C code and emit an intermediate representation
of the structures that we could then generate our functions from.

If you've come from the Java world you'd expect that we'd just
generate the copy and makenode stuff from introspection of the node
structs, or preprocess them or something, but it's not very practical.
They don't change enough to really make it worth it either.

> Hmmm. I've run into a tiny small ever so little detail in trying to apply
> this clever approach...
>
> For fixing the information in pg_stat_statement, the location data must be
> transported from the parsed node to the query to the planned node, because
> the later two nodes types are passed to different hooks.

Yep.

> Now the detail is that utility statements, which seems to be nearly all of
> them but select/update/delete/insert, do not have plans: The statement
> itself is its own plan... so there is no place to store the location &
> length.

Yeah. I don't see any way around adding location info for utility
statements one way or the other.

We could wrap every utility statement up in a container with the
location info, but that'd break every user of ProcessUtility_hook and
unless we put the container as the first by-value member of each
utility struct anyway, it'd mean extra allocations for each utility
statement. Not very appealing.

TBH I think that for utility statements, adding a member struct with
location info is the least-bad option. Something like:

/*
 * Location of the original querytext of an individual parsetree or
plan relative to the
 * start of the querytext. This is usually offset 0 and the length of
the querytext its self,
 * but in multi-statements and other cases where a single parse has
multiple products
 * hooks may need to know which part of the querytext corresponds to
which product
 * plan.
 *
 * offset and length are both -1 for plans with no corresponding
querytext, such as
 * additional/replacement queries emitted by the rewrite phase or
additional statements
 * produced as a result of processing some utility statements.
 *
typedef struct StmtLocation
{
int offset;
int length;
}

typedef struct CreateStmt
{
NodeTag type;
StmtLocation  location;

}


Personally I don't much care if we do it this way, or by embedding the
NodeTag too and creating a common UtilityStmt. The latter probably has
benefits for later extension, but is a bit more annoying in the
shorter term.

-- 
 Craig Ringer   http://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] Assignment of valid collation for SET operations on queries with UNKNOWN types.

2016-12-29 Thread Ashutosh Bapat
On Thu, Dec 29, 2016 at 8:18 PM, Tom Lane  wrote:
> Ashutosh Bapat  writes:
>> The way this patch has been written, it doesn't allow creating tables
>> with unknown type columns, which was allowed earlier.
>
> Yes, that's an intentional change; creating such tables (or views) has
> never been anything but a foot-gun.
>
> However, I thought the idea was to silently coerce affected columns from
> unknown to text.  This doesn't look like the behavior we want:

Do you mean to say that when creating a table with a column of unknown
type, that column type should be silently converted (there's nothing
to coerce when the table is being created) to text? instead of
throwing an error?

The patch does that when creating a view with constant literals, which
are known to be binary compatible with text and hence coercible. It
looks like any "unknown' type data should be coercible to text, so it
shouldn't matter whether those are constants or non-constant nodes.

>
>> You might want to add some testcases to test the error report e.g.
>> (not necessarily in the same form) create view sv as select
>> relname::unknown from pg_class;
>> ERROR:  column "relname" has type "unknown"
>
> regards, tom lane



-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


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


Re: [HACKERS] gettimeofday is at the end of its usefulness?

2016-12-29 Thread Tom Lane
Haribabu Kommi  writes:
> Attached a patch that replaces most of the getimeofday function calls,
> except timeofday(user callable) and GetCurrentTimestamp functions.

I looked at this for awhile and could not convince myself that it's
a good idea.  Trying to do s/gettimeofday/clock_gettime/g is not going
to do much for us except create portability headaches.  According
to my tests, clock_gettime is not noticeably faster than gettimeofday
on any platform, except that if you use nonstandard clockids like
CLOCK_REALTIME_COARSE then on *some* platforms it's a little bit quicker,
at the cost of being a great deal less precise.  But we'd have to research
the existence and effects of nonstandard clockids on every platform.
So AFAICS the only clear advantage to switching is the extra precision
available from clock_gettime.

But ... most of the places you've touched in this patch have neither any
need for sub-microsecond precision nor any great need to worry about
shaving a few ns off the time taken by the call.  As far as I can find,
the only place where it's actually worth our trouble to deal with it is
instr_time.h (ie, EXPLAIN ANALYZE and a few other uses).

So I think we should do something more like the attached.


One issue I did not resolve in this WIP patch is what to do with this
gem of abstraction violation in pgbench:

/* no, print raw transactions */
#ifndef WIN32

/* This is more than we really ought to know about instr_time */
if (skipped)
fprintf(logfile, "%d " INT64_FORMAT " skipped %d %ld %ld",
st->id, st->cnt, st->use_file,
(long) now->tv_sec, (long) now->tv_usec);
else
fprintf(logfile, "%d " INT64_FORMAT " %.0f %d %ld %ld",
st->id, st->cnt, latency, st->use_file,
(long) now->tv_sec, (long) now->tv_usec);
#else

/* On Windows, instr_time doesn't provide a timestamp anyway */
if (skipped)
fprintf(logfile, "%d " INT64_FORMAT " skipped %d 0 0",
st->id, st->cnt, st->use_file);
else
fprintf(logfile, "%d " INT64_FORMAT " %.0f %d 0 0",
st->id, st->cnt, latency, st->use_file);
#endif

We could either rip out the non-Windows code path entirely, or do
something about providing an honest elapsed-time measurement, perhaps
by doing INSTR_TIME_GET_DOUBLE() on the diff from run start to "now".
Given that we're calling fprintf, I doubt that the extra arithmetic
needed for that is a big problem.

regards, tom lane

diff --git a/configure b/configure
index 0f143a0..e5dd6fb 100755
*** a/configure
--- b/configure
*** if test "$ac_res" != no; then :
*** 9055,9060 
--- 9055,9116 
  
  fi
  
+ { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing clock_gettime" >&5
+ $as_echo_n "checking for library containing clock_gettime... " >&6; }
+ if ${ac_cv_search_clock_gettime+:} false; then :
+   $as_echo_n "(cached) " >&6
+ else
+   ac_func_search_save_LIBS=$LIBS
+ cat confdefs.h - <<_ACEOF >conftest.$ac_ext
+ /* end confdefs.h.  */
+ 
+ /* Override any GCC internal prototype to avoid an error.
+Use char because int might match the return type of a GCC
+builtin and then its argument prototype would still apply.  */
+ #ifdef __cplusplus
+ extern "C"
+ #endif
+ char clock_gettime ();
+ int
+ main ()
+ {
+ return clock_gettime ();
+   ;
+   return 0;
+ }
+ _ACEOF
+ for ac_lib in '' rt posix4; do
+   if test -z "$ac_lib"; then
+ ac_res="none required"
+   else
+ ac_res=-l$ac_lib
+ LIBS="-l$ac_lib  $ac_func_search_save_LIBS"
+   fi
+   if ac_fn_c_try_link "$LINENO"; then :
+   ac_cv_search_clock_gettime=$ac_res
+ fi
+ rm -f core conftest.err conftest.$ac_objext \
+ conftest$ac_exeext
+   if ${ac_cv_search_clock_gettime+:} false; then :
+   break
+ fi
+ done
+ if ${ac_cv_search_clock_gettime+:} false; then :
+ 
+ else
+   ac_cv_search_clock_gettime=no
+ fi
+ rm conftest.$ac_ext
+ LIBS=$ac_func_search_save_LIBS
+ fi
+ { $as_echo "$as_me:${as_lineno-$LINENO}: result: $ac_cv_search_clock_gettime" >&5
+ $as_echo "$ac_cv_search_clock_gettime" >&6; }
+ ac_res=$ac_cv_search_clock_gettime
+ if test "$ac_res" != no; then :
+   test "$ac_res" = "none required" || LIBS="$ac_res $LIBS"
+ 
+ fi
+ 
  # Solaris:
  { $as_echo "$as_me:${as_lineno-$LINENO}: checking for library containing fdatasync" >&5
  $as_echo_n "checking for library containing fdatasync... " >&6; }
*** fi
*** 12520,12526 
  LIBS_including_readline="$LIBS"
  LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'`
  
! for ac_func in cbrt dlopen fdatasync getifaddrs getpeerucred getrlimit mbstowcs_l memmove poll pstat pthread_is_threaded_np readlink setproctitle setsid shm_open symlink sync_file_range towlower utime utimes wcstombs wcstombs_l
  do :
as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh`
  

Re: [HACKERS] Faster methods for getting SPI results

2016-12-29 Thread Jim Nasby

On 12/27/16 9:10 PM, Craig Ringer wrote:

On 28 December 2016 at 09:58, Jim Nasby  wrote:


I've looked at this some more, and ITSM that the only way to do this without
some major surgery is to create a new type of Destination specifically for
SPI that allows for the execution of an arbitrary C function for each tuple
to be sent.


That sounds a lot more sensible than the prior proposals. Callback driven.


Here's what I've got right now. I haven't bothered with 
SPI_execute_callback() yet, and there's some missing sanity checks.


I'm not sure if the changes to CreateDestReceiver() are warranted or 
necessary, though it would at least give you sane defaults. My 
incomplete code that would make use of this currently does


CallbackState   callback;

	memcpy(callback.pub, CreateDestReceiver(DestSPICallback), 
sizeof(DestReceiver));

callback.pub.receiveSlot = PLy_CSreceive;
callback.pub.rStartup = PLy_CSStartup;
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)


--
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] background sessions

2016-12-29 Thread Peter Eisentraut
On 12/16/16 10:38 AM, Andrew Borodin wrote:
> 2016-12-16 20:17 GMT+05:00 Peter Eisentraut 
> :
>>> And one more thing... Can we have BackgroundSessionExecute() splitted
>>> into two parts: start query and wait for results?
>>> It would allow pg_background to reuse bgsession's code.
>>
>> Yes, I will look into that.
> 
> Thank you. I'm marking both patches as "Waiting for author", keeping
> in mind that pg_background is waiting for bgsessions.
> After updates I'll review these patches.

New patch, mainly with the function split as you requested above, not
much else changed.

For additional entertainment, I include patches that integrate
background sessions into dblink.  So dblink can open a connection to a
background session, and then you can use the existing dblink functions
to send queries, read results, etc.  People use dblink to make
self-connections to get autonomous subsessions, so this would directly
address that use case.  The 0001 patch is some prerequisite refactoring
to remove an ugly macro mess, which is useful independent of this.  0002
is the actual patch.

-- 
Peter Eisentraut  http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 95c0499907cfe49ff944dc19900d678e5dd8b5de Mon Sep 17 00:00:00 2001
From: Peter Eisentraut 
Date: Wed, 28 Dec 2016 12:00:00 -0500
Subject: [PATCH v2] Add background sessions

This adds a C API to run SQL statements in a background worker,
communicating by FE/BE protocol over a DSM message queue.  This can be
used to execute statements and transactions separate from the main
foreground session.

Also included is a PL/Python interface to this functionality.
---
 doc/src/sgml/bgsession.sgml | 272 +++
 doc/src/sgml/filelist.sgml  |   1 +
 doc/src/sgml/plpython.sgml  | 102 +++
 doc/src/sgml/postgres.sgml  |   1 +
 src/backend/commands/variable.c |   5 +
 src/backend/libpq/pqmq.c|  26 +-
 src/backend/storage/ipc/shm_mq.c|  19 +
 src/backend/tcop/Makefile   |   2 +-
 src/backend/tcop/bgsession.c| 911 
 src/backend/tcop/postgres.c |  24 +-
 src/include/commands/variable.h |   1 +
 src/include/libpq/pqmq.h|   1 +
 src/include/storage/shm_mq.h|   3 +
 src/include/tcop/bgsession.h|  30 +
 src/include/tcop/tcopprot.h |   9 +
 src/pl/plpython/Makefile|   2 +
 src/pl/plpython/expected/plpython_bgsession.out | 188 +
 src/pl/plpython/expected/plpython_test.out  |   7 +-
 src/pl/plpython/plpy_bgsession.c| 454 
 src/pl/plpython/plpy_bgsession.h|  18 +
 src/pl/plpython/plpy_main.h |   3 +
 src/pl/plpython/plpy_planobject.c   |   1 +
 src/pl/plpython/plpy_planobject.h   |   2 +
 src/pl/plpython/plpy_plpymodule.c   |   5 +
 src/pl/plpython/plpy_spi.c  |   7 +-
 src/pl/plpython/plpy_spi.h  |   3 +
 src/pl/plpython/sql/plpython_bgsession.sql  | 148 
 27 files changed, 2224 insertions(+), 21 deletions(-)
 create mode 100644 doc/src/sgml/bgsession.sgml
 create mode 100644 src/backend/tcop/bgsession.c
 create mode 100644 src/include/tcop/bgsession.h
 create mode 100644 src/pl/plpython/expected/plpython_bgsession.out
 create mode 100644 src/pl/plpython/plpy_bgsession.c
 create mode 100644 src/pl/plpython/plpy_bgsession.h
 create mode 100644 src/pl/plpython/sql/plpython_bgsession.sql

diff --git a/doc/src/sgml/bgsession.sgml b/doc/src/sgml/bgsession.sgml
new file mode 100644
index 00..ba5cb409d6
--- /dev/null
+++ b/doc/src/sgml/bgsession.sgml
@@ -0,0 +1,272 @@
+
+
+
+ Background Session API
+
+ 
+  The background session API is a C API for creating additional database
+  sessions in the background and running SQL statements in them.  A background
+  session behaves like a normal (foreground) session in that it has session
+  state, transactions, can run SQL statements, and so on.  Unlike a foreground
+  session, it is not connected directly to a client.  Instead the foreground
+  session can use this API to execute SQL statements and retrieve their
+  results.  Higher-level integrations, such as in procedural languages, can
+  make this functionality available to clients.  Background sessions are
+  independent from their foreground sessions in their session and transaction
+  state.  So a background session cannot see uncommitted data in foreground
+  sessions or vice versa, and there is no preferential treatment about
+  locking.  Like all sessions, background sessions are separate processes.
+  Foreground and background sessions communicate over shared memory messages
+  queues 

Re: [HACKERS] [sqlsmith] Short reads in hash indexes

2016-12-29 Thread Andreas Seltenreich
Amit Kapila writes:

> Can you please try with the patch posted on hash index thread [1] to
> see if you can reproduce any of these problems?
>
> [1] - 
> https://www.postgresql.org/message-id/CAA4eK1Kf6tOY0oVz_SEdngiNFkeXrA3xUSDPPORQvsWVPdKqnA%40mail.gmail.com

I'm no longer seeing the failed assertions nor short reads since these
patches are in.

regards,
Andreas


-- 
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] [sqlsmith] Crash reading pg_stat_activity

2016-12-29 Thread Andreas Seltenreich
Thomas Munro writes:

> [2. text/x-diff; fix-dsa-tranche-registration.patch]

Fuzzing with the patch applied hasn't triggered the crash so far.  It
did happen 5 times with the same amount of fuzzing before patching.

regards,
Andreas


-- 
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] Increase pltcl test coverage

2016-12-29 Thread Jim Nasby

On 10/31/16 3:24 PM, Jim Nasby wrote:

This patch increases test coverage for pltcl, from 70% to 83%. Aside
from that, the work on this uncovered 2 new bugs (the trigger return one
I just submitted, as well as a bug in the SRF/composite patch).


Rebased patch attached. Test coverage is now at 85% (by line count).

Original patch by Karl Lehenbauer. Work sponsored by FlightAware.
--
Jim Nasby, Data Architect, Blue Treble Consulting, Austin TX
Experts in Analytics, Data Architecture and PostgreSQL
Data in Trouble? Get it in Treble! http://BlueTreble.com
855-TREBLE2 (855-873-2532)
diff --cc src/pl/tcl/expected/pltcl_queries.out
index 3a9fef3447,9e3a0dcd0b..00
--- a/src/pl/tcl/expected/pltcl_queries.out
+++ b/src/pl/tcl/expected/pltcl_queries.out
@@@ -288,6 -350,20 +350,22 @@@ select tcl_argisnull(null)
   t
  (1 row)
  
+ -- should error
+ insert into trigger_test(test_argisnull) values(true);
+ NOTICE:  NEW: {}
+ NOTICE:  OLD: {}
+ NOTICE:  TG_level: STATEMENT
+ NOTICE:  TG_name: statement_trigger
+ NOTICE:  TG_op: INSERT
+ NOTICE:  TG_relatts: {{} i v {} test_skip test_return_null test_argisnull}
+ NOTICE:  TG_relid: bogus:12345
+ NOTICE:  TG_table_name: trigger_test
+ NOTICE:  TG_table_schema: public
+ NOTICE:  TG_when: BEFORE
+ NOTICE:  args: {42 {statement trigger}}
+ ERROR:  argisnull cannot be used in triggers
++select trigger_data();
++ERROR:  trigger functions can only be called as triggers
  -- Test spi_lastoid primitive
  create temp table t1 (f1 int);
  select tcl_lastoid('t1');
@@@ -303,64 -379,216 +381,277 @@@ select tcl_lastoid('t2') > 0
   t
  (1 row)
  
 --- Test quote
 +-- test some error cases
- CREATE FUNCTION tcl_error(OUT a int, OUT b int) AS $$return {$$ LANGUAGE 
pltcl;
- SELECT tcl_error();
++create function tcl_error(out a int, out b int) as $$return {$$ language 
pltcl;
++select tcl_error();
 +ERROR:  missing close-brace
- CREATE FUNCTION bad_record(OUT a text, OUT b text) AS $$return [list a]$$ 
LANGUAGE pltcl;
- SELECT bad_record();
++create function bad_record(out a text, out b text) as $$return [list a]$$ 
language pltcl;
++select bad_record();
 +ERROR:  column name/value list must have even number of elements
- CREATE FUNCTION bad_field(OUT a text, OUT b text) AS $$return [list a 1 b 2 
cow 3]$$ LANGUAGE pltcl;
- SELECT bad_field();
++create function bad_field(out a text, out b text) as $$return [list a 1 b 2 
cow 3]$$ language pltcl;
++select bad_field();
 +ERROR:  column name/value list contains nonexistent column name "cow"
 +-- test compound return
 +select * from tcl_test_cube_squared(5);
 + squared | cubed 
 +-+---
 +  25 |   125
 +(1 row)
 +
- -- test SRF
++-- test srf
 +select * from tcl_test_squared_rows(0,5);
 + x | y  
 +---+
 + 0 |  0
 + 1 |  1
 + 2 |  4
 + 3 |  9
 + 4 | 16
 +(5 rows)
 +
 +select * from tcl_test_sequence(0,5) as a;
 + a 
 +---
 + 0
 + 1
 + 2
 + 3
 + 4
 +(5 rows)
 +
 +select 1, tcl_test_sequence(0,5);
 + ?column? | tcl_test_sequence 
 +--+---
 +1 | 0
 +1 | 1
 +1 | 2
 +1 | 3
 +1 | 4
 +(5 rows)
 +
- CREATE FUNCTION non_srf() RETURNS int AS $$return_next 1$$ LANGUAGE pltcl;
++create function non_srf() returns int as $$return_next 1$$ language pltcl;
 +select non_srf();
 +ERROR:  return_next cannot be used in non-set-returning functions
- CREATE FUNCTION bad_record_srf(OUT a text, OUT b text) RETURNS SETOF record 
AS $$
++create function bad_record_srf(out a text, out b text) returns setof record 
as $$
 +return_next [list a]
- $$ LANGUAGE pltcl;
- SELECT bad_record_srf();
++$$ language pltcl;
++select bad_record_srf();
 +ERROR:  column name/value list must have even number of elements
- CREATE FUNCTION bad_field_srf(OUT a text, OUT b text) RETURNS SETOF record AS 
$$
++create function bad_field_srf(out a text, out b text) returns setof record as 
$$
 +return_next [list a 1 b 2 cow 3]
- $$ LANGUAGE pltcl;
- SELECT bad_field_srf();
++$$ language pltcl;
++select bad_field_srf();
 +ERROR:  column name/value list contains nonexistent column name "cow"
++-- test quote
+ select tcl_eval('quote foo bar');
+ ERROR:  wrong # args: should be "quote string"
+ select tcl_eval('quote [format %c 39]');
+  tcl_eval 
+ --
+  ''
+ (1 row)
+ 
+ select tcl_eval('quote [format %c 92]');
+  tcl_eval 
+ --
+  \\
+ (1 row)
+ 
+ -- Test argisnull
+ select tcl_eval('argisnull');
+ ERROR:  wrong # args: should be "argisnull argno"
+ select tcl_eval('argisnull 14');
+ ERROR:  argno out of range
+ select tcl_eval('argisnull abc');
+ ERROR:  expected integer but got "abc"
+ -- Test return_null
+ select tcl_eval('return_null 14');
+ ERROR:  wrong # args: should be "return_null "
+ -- should error
+ insert into trigger_test(test_return_null) values(true);
+ NOTICE:  NEW: {}
+ NOTICE:  OLD: {}
+ NOTICE:  TG_level: STATEMENT
+ NOTICE:  TG_name: statement_trigger
+ NOTICE:  TG_op: INSERT
+ NOTICE:  

Re: [HACKERS] gettimeofday is at the end of its usefulness?

2016-12-29 Thread Tom Lane
Thomas Munro  writes:
> On Tue, Dec 27, 2016 at 10:34 AM, Tom Lane  wrote:
>> However, it seems that these impressive results date back only to
>> June 2012, cf
>> https://github.com/freebsd/freebsd/commit/13a9f42818f6b89a72b3e40923be809b490400d8
>> and at least as of that commit, only x86 and x86_64 had the fast
>> clock_gettime code.  Older FreeBSD, or FreeBSD on another architecture,
>> is likely to be a lot worse.  But I lack an installation to try.

> That commit is in every 'production' and 'legacy' release of
> FreeBSD[1], meaning as far back as 9.3 (expected to be EoL in the next
> few days), because it landed in 9.2 (EoL).

I'm unclear on whether there's any significant number of people running
out-of-support *BSD releases.  If it's not something we have to worry
about, fine.

> That leaves the following architectures without
> fast-path time functions:

> macaque:freebsd munro$ git grep 'trivial-vdso_tc.c'
> lib/libc/mips/sys/Makefile.inc:SRCS+=   trivial-vdso_tc.c
> lib/libc/powerpc/Makefile.inc:SRCS+=trivial-vdso_tc.c
> lib/libc/powerpc64/Makefile.inc:SRCS+=  trivial-vdso_tc.c
> lib/libc/powerpcspe/Makefile.inc:SRCS+= trivial-vdso_tc.c
> lib/libc/riscv/sys/Makefile.inc:SRCS+=  trivial-vdso_tc.c
> lib/libc/sparc64/Makefile.inc:SRCS+=trivial-vdso_tc.c

Yeah, I just finished getting results from FreeBSD 10.3 on PPC
(1.33GHz G4 laptop): gettimeofday takes about 1180 ns and clock_gettime
about 1200 ns.  That difference seems to be repeatable, but since it's
only 2% I'm not too fussed about it.  Interestingly, it's very easy
to tell that it is entering the kernel, because time(1) shows a
significant fraction of system time:

$ time ./testclock
0 bogus readings
1 distinct readings
  117.96 real26.80 user90.31 sys

The same test on platforms with vDSO support shows zero system time.

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] gettimeofday is at the end of its usefulness?

2016-12-29 Thread Thomas Munro
On Tue, Dec 27, 2016 at 10:34 AM, Tom Lane  wrote:
> I also tried FreeBSD 11.0 on another Mac (2.3GHz x86_64),
> and found that gettimeofday as well as basically all their
> clock_gettime variants run in 27 to 28 ns; and clock_gettime
> reliably delivers full precision, except for CLOCK_SECOND which
> is intentionally truncated to 1s precision.  So there would be
> no need to work with anything but CLOCK_MONOTONIC here.
>
> However, it seems that these impressive results date back only to
> June 2012, cf
> https://github.com/freebsd/freebsd/commit/13a9f42818f6b89a72b3e40923be809b490400d8
> and at least as of that commit, only x86 and x86_64 had the fast
> clock_gettime code.  Older FreeBSD, or FreeBSD on another architecture,
> is likely to be a lot worse.  But I lack an installation to try.

That commit is in every 'production' and 'legacy' release of
FreeBSD[1], meaning as far back as 9.3 (expected to be EoL in the next
few days), because it landed in 9.2 (EoL).  ARM support landed in
FreeBSD 11.0[2].  That leaves the following architectures without
fast-path time functions:

macaque:freebsd munro$ git grep 'trivial-vdso_tc.c'
lib/libc/mips/sys/Makefile.inc:SRCS+=   trivial-vdso_tc.c
lib/libc/powerpc/Makefile.inc:SRCS+=trivial-vdso_tc.c
lib/libc/powerpc64/Makefile.inc:SRCS+=  trivial-vdso_tc.c
lib/libc/powerpcspe/Makefile.inc:SRCS+= trivial-vdso_tc.c
lib/libc/riscv/sys/Makefile.inc:SRCS+=  trivial-vdso_tc.c
lib/libc/sparc64/Makefile.inc:SRCS+=trivial-vdso_tc.c

[1] https://www.freebsd.org/releases/
[2] 
https://github.com/freebsd/freebsd/commit/80e8626b434515d16b3576174438526755336810

-- 
Thomas Munro
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] gettimeofday is at the end of its usefulness?

2016-12-29 Thread Tom Lane
Florian Weimer  writes:
> * Tom Lane:
>> On Linux (RHEL6, 2.4GHz x86_64), I find that gettimeofday(),
>> clock_gettime(CLOCK_MONOTONIC), and clock_gettime(CLOCK_REALTIME)
>> all take about 40ns.  Of course gettimeofday() only has 1us resolution,
>> but the other two have perhaps 10ns resolution (I get no duplicate
>> readings in a tight loop).

> Isn't this very specific to kernel and glibc versions, depending on
> things like CONFIG_HZ settings and what level of vDSO support has been
> backported?

No doubt, but I have yet to find a platform where clock_gettime() exists
but performs worse than gettimeofday().  Do you know of one?

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] proposal: session server side variables

2016-12-29 Thread Andrew Dunstan



On 12/29/2016 02:23 PM, Fabien COELHO wrote:



There is a singleton table :)

create table foo(x integer unique not null default 1 check(x = 1), y 
integer);

insert into foo(y) values(100);
analyze foo;


I know this one. It can be empty, which a singleton cannot be. For a 
singleton table, you should have one and only one row, you cannot 
insert or delete, so this is only part of the real thing.



Surely we can do a bit better than that, if that's what you really want. 
Create the table with an initial row and add a trigger preventing 
anything except update.



--
Andrew Dunstanhttps://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] gettimeofday is at the end of its usefulness?

2016-12-29 Thread Florian Weimer
* Tom Lane:

> On Linux (RHEL6, 2.4GHz x86_64), I find that gettimeofday(),
> clock_gettime(CLOCK_MONOTONIC), and clock_gettime(CLOCK_REALTIME)
> all take about 40ns.  Of course gettimeofday() only has 1us resolution,
> but the other two have perhaps 10ns resolution (I get no duplicate
> readings in a tight loop).  Other documented clockids include
> CLOCK_REALTIME_COARSE: about 10ns to read, but only 1ms resolution
> CLOCK_MONOTONIC_COARSE: about 12ns to read, but only 1ms resolution
> CLOCK_MONOTONIC_RAW: full resolution but very slow, ~145ns to read
> So CLOCK_MONOTONIC seems to be the thing to use here.  It won't buy
> us anything speed-wise but the extra resolution will be nice.
> However, we need to do more research to see if this holds true on
> other popular distros.

Isn't this very specific to kernel and glibc versions, depending on
things like CONFIG_HZ settings and what level of vDSO support has been
backported?


-- 
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] proposal: session server side variables

2016-12-29 Thread Pavel Stehule
2016-12-29 20:23 GMT+01:00 Fabien COELHO :

>
> There is a singleton table :)
>>
>> create table foo(x integer unique not null default 1 check(x = 1), y
>> integer);
>> insert into foo(y) values(100);
>> analyze foo;
>>
>
> I know this one. It can be empty, which a singleton cannot be. For a
> singleton table, you should have one and only one row, you cannot insert or
> delete, so this is only part of the real thing.


subselect is "singleton" - it returns one row every time.

Not sure if term "singleton" is valid in relation database.


>
>
> For example - MySQL @var is volatile - can be changed in query - that's
>> mean, you cannot use it as const for planner :(
>>
>
> Indeed, because of the ":=" within in a SELECT query, the variable is
> updated at each round. Yuk.
>
> --
> Fabien.
>


Re: [HACKERS] proposal: session server side variables

2016-12-29 Thread Fabien COELHO



There is a singleton table :)

create table foo(x integer unique not null default 1 check(x = 1), y integer);
insert into foo(y) values(100);
analyze foo;


I know this one. It can be empty, which a singleton cannot be. For a 
singleton table, you should have one and only one row, you cannot insert 
or delete, so this is only part of the real thing.



For example - MySQL @var is volatile - can be changed in query - that's
mean, you cannot use it as const for planner :(


Indeed, because of the ":=" within in a SELECT query, the variable is 
updated at each round. Yuk.


--
Fabien.


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


Re: [HACKERS] [PATCH] Rename pg_switch_xlog to pg_switch_wal

2016-12-29 Thread Stephen Frost
Cynthia,

Please don't top-post on the PG mailing lists but rather write responses
in-line.

* Cynthia Shang (cynthia.sh...@crunchydata.com) wrote:
> I have never heard of coding standards where naming conventions required a 
> function/variable name match a directory or file name. It seems that would be 
> restrictive. 

We aren't discussing a coding standard, we're talking about changing the
references from 'xlog' to 'wal' to match the change we did to the
directory, to be consistent.

> I'm not trying to pick a fight, I just think the pros should outweigh the 
> cons when choosing a path forward. In this case I see lots of cons and one 
> partial pro; partial because renaming only user facing functions and 
> documentation will create inconsistency within the Postgres code for all of 
> the following. It sounds as if your minds are already made up, which saddens 
> me but I will say nothing further on the matter.

All backwards incompatible changes are judgement calls and people are
certainly welcome to have different opinions.  I have a pretty strong
feeling about this particular change being worthwhile and also pretty
long overdue.

[... list of references to xlog in the code ...]

No need to include a list, I can find them myself. :)

Nearly all of the references listed are internal functions or references
in the code.  Those don't need to be changed, just the user-facing
pieces, which is quite a bit less.

The doxygen website is generated directly off of the code and isn't
user-facing documentation.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] [PATCH] Rename pg_switch_xlog to pg_switch_wal

2016-12-29 Thread Cynthia Shang
I have never heard of coding standards where naming conventions required a 
function/variable name match a directory or file name. It seems that would be 
restrictive. 

I'm not trying to pick a fight, I just think the pros should outweigh the cons 
when choosing a path forward. In this case I see lots of cons and one partial 
pro; partial because renaming only user facing functions and documentation will 
create inconsistency within the Postgres code for all of the following. It 
sounds as if your minds are already made up, which saddens me but I will say 
nothing further on the matter.


xlog 

 - logstreamer_param
xlog.c 
xlog.h 
XLOG_BACKUP_END 

 - pg_control.h
XLOG_BACKUP_LABEL 

 - pg_standby.c
xlog_blcksz 

 - ControlFileData
XLOG_BLCKSZ_K 

 - pg_test_fsync.c
XLOG_BRIN_CREATE_INDEX 

 - brin_xlog.h
XLOG_BRIN_INIT_PAGE 

 - brin_xlog.h
XLOG_BRIN_INSERT 

 - brin_xlog.h
XLOG_BRIN_OPMASK 

 - brin_xlog.h
XLOG_BRIN_REVMAP_EXTEND 

 - brin_xlog.h
XLOG_BRIN_REVMAP_VACUUM 

 - brin_xlog.h
XLOG_BRIN_SAMEPAGE_UPDATE 

 - brin_xlog.h
XLOG_BRIN_UPDATE 

 - brin_xlog.h
XLOG_BTREE_DELETE 

 - nbtree.h
XLOG_BTREE_INSERT_LEAF 

 - nbtree.h
XLOG_BTREE_INSERT_META 

 - nbtree.h
XLOG_BTREE_INSERT_UPPER 

 - nbtree.h
XLOG_BTREE_MARK_PAGE_HALFDEAD 

 - nbtree.h
XLOG_BTREE_NEWROOT 

 - nbtree.h
XLOG_BTREE_REUSE_PAGE 

 - nbtree.h
XLOG_BTREE_SPLIT_L 

 - nbtree.h
XLOG_BTREE_SPLIT_L_ROOT 

 - nbtree.h
XLOG_BTREE_SPLIT_R 

 - nbtree.h
XLOG_BTREE_SPLIT_R_ROOT 

 - nbtree.h
XLOG_BTREE_UNLINK_PAGE 

 - nbtree.h
XLOG_BTREE_UNLINK_PAGE_META 

 - nbtree.h
XLOG_BTREE_VACUUM 

 - nbtree.h
XLOG_CHECKPOINT_ONLINE 

 - pg_control.h
XLOG_CHECKPOINT_SHUTDOWN 

 - pg_control.h
XLOG_CONTROL_FILE 
xlog_internal.h
XLOG_DATA 
pg_standby.c
XLOG_DBASE_CREATE 

 - dbcommands_xlog.h
XLOG_DBASE_DROP 

 - dbcommands_xlog.h
xlog_desc 
xlog_dir 
XLOG_END_OF_RECOVERY 

 - pg_control.h
xlog_fn.h 

Re: [HACKERS] [PATCH] Rename pg_switch_xlog to pg_switch_wal

2016-12-29 Thread Stephen Frost
Cynthia,

* Cynthia Shang (cynthia.sh...@crunchydata.com) wrote:
> 1) I agree with Michael that we should make this change backward compatible. 
> It would help PostgreSQL image if we did not break everyone's code. It costs 
> businesses money to rewrite code (e.g. middle tier software, backup tools, 
> etc), test and redeploy to their customers. 

While I agree that we don't want to break client code or to make
backwards incompatible changes without good cause, in this case, it's
definitely a good cause and it makes sense to have things be consistent
and that includes changing these functions.

We make backwards-incompatible changes with each major release, which is
part of why we support older versions of PG for as long as we do- to
give PG users time to make any necessary changes for the new version of
PG.  One could argue that we shouldn't ever make a backwards
incompatible change because it will break an existing user's code and
cost users time and effort to rewrite that code, but the flip side of
that is that the extra code and complexity results in its own
maintenance burdens for the code and the project moving forward.

Generally speaking, we've also found that backwards compatibility
'features' end up having a much longer life than they should.
Ultimately, the best way forward tends to be either make the backwards
incompatible change or don't make the change at all.  We've already
agreed on this particular change, and with good reason, so the way
forward is to make the rest of the changes, not to go half-way or to try
and provide some backwards compatibility complexity.

> 2) We decided to rename the pg_xlog directory because people were deleting it 
> when disks were getting full thinking it was just unimportant logging data; I 
> get that. I'm a little unclear why we need to change the functions - it would 
> be less painful to our users and less risky if we just left them as is. Could 
> someone please elaborate why this change is necessary? I'm just trying to 
> understand that.

It would be inconsistent to change the directory name without also
changing the documentation, functions, and other user-facing pieces.

Thanks!

Stephen


signature.asc
Description: Digital signature


Re: [HACKERS] Improving RLS planning

2016-12-29 Thread Dean Rasheed
On 29 December 2016 at 15:55, Tom Lane  wrote:
> Dean Rasheed  writes:
>> On 28 December 2016 at 19:12, Tom Lane  wrote:
>>> [ getting back to this patch finally... ]  I made the suggested change
>>> to that test case, and what I see is a whole lot of "NOTICE: snooped value
>>> = whatever" outputs.
>>>
>>> I'd leave it as shown in the attached diff fragment, except that I'm
>>> a bit worried about possible platform dependency of the output.  The
>>> hashing occurring in the subplans shouldn't affect output order, but
>>> I'm not sure if we want a test output like this or not.  Thoughts?
>
>> How about replacing "a = 3" with "a < 7 AND a != 6". That then
>> exercises more of the possible types of behaviour for quals: The "a <
>> 7" qual is pushed down and used as an index condition. The "a != 6"
>> qual is pushed down and used as a filter, because it's cheap and
>> leakproof. The leakproof() qual isn't pushed down on cost grounds. The
>> snoop() qual isn't pushed down on security grounds. Both snoop() and
>> leakproof() are used as filters, along with "a != 6", and a SB subplan
>> qual. "a != 6" is executed first because it has a security_level of 0,
>> and is cheaper than the subplan. snoop() is executed later, despite
>> being cheaper than the other filter quals, because it has a higher
>> security_level, and leakproof() is executed last because it has the
>> same security level as snoop() but is more expensive.
>
> Will do, although I think that the next test case (the one with "a = 8")
> already shows most of those behaviors.
>

Except that it doesn't have a cheap leakproof qual like "a != 6", not
handled automatically by the index, that order_qual_clauses() can
assign security_level = 0 to, and then move to the start of the list.

I think it's probably worth having a clause like that in one of the
tests, but it could perhaps be added to the "a = 8" test, if you
wanted to drop the "a = 3" test.

Regards,
Dean


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


Re: [HACKERS] [PATCH] Rename pg_switch_xlog to pg_switch_wal

2016-12-29 Thread Cynthia Shang
1) I agree with Michael that we should make this change backward compatible. It 
would help PostgreSQL image if we did not break everyone's code. It costs 
businesses money to rewrite code (e.g. middle tier software, backup tools, 
etc), test and redeploy to their customers. 

2) We decided to rename the pg_xlog directory because people were deleting it 
when disks were getting full thinking it was just unimportant logging data; I 
get that. I'm a little unclear why we need to change the functions - it would 
be less painful to our users and less risky if we just left them as is. Could 
someone please elaborate why this change is necessary? I'm just trying to 
understand that.

Thanks,
-Cynthia
-- 
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] make more use of RoleSpec struct

2016-12-29 Thread Peter Eisentraut
On 12/28/16 9:53 AM, Alvaro Herrera wrote:
> Peter Eisentraut wrote:
>> Here is a small cleanup patch to make more use of the RoleSpec
>> struct/node throughout the parser to avoid casts and make the code more
>> self-documenting.
> 
> This makes sense to me.  I started to do this at some point when I was
> writing RoleSpec; eventually got annoyed about fixing the fallout (I was
> working to get somebody else's patch committed) and went back to how it
> is.  Glad to see it done.
> 
> The only functional issue might be the removal of the IsA() checks.  If
> we don't cast any Node before passing it to any of those functions,
> there should be no problem because any misfeasance will be reported as a
> compile-time warning.  Perhaps it's worth adding IsA() checks in loops
> such as the one in roleSpecsToIds().

Committed with an extra Assert there for good measure.

-- 
Peter Eisentraut  http://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 RLS planning

2016-12-29 Thread Tom Lane
Dean Rasheed  writes:
> On 28 December 2016 at 19:12, Tom Lane  wrote:
>> [ getting back to this patch finally... ]  I made the suggested change
>> to that test case, and what I see is a whole lot of "NOTICE: snooped value
>> = whatever" outputs.
>> 
>> I'd leave it as shown in the attached diff fragment, except that I'm
>> a bit worried about possible platform dependency of the output.  The
>> hashing occurring in the subplans shouldn't affect output order, but
>> I'm not sure if we want a test output like this or not.  Thoughts?

> How about replacing "a = 3" with "a < 7 AND a != 6". That then
> exercises more of the possible types of behaviour for quals: The "a <
> 7" qual is pushed down and used as an index condition. The "a != 6"
> qual is pushed down and used as a filter, because it's cheap and
> leakproof. The leakproof() qual isn't pushed down on cost grounds. The
> snoop() qual isn't pushed down on security grounds. Both snoop() and
> leakproof() are used as filters, along with "a != 6", and a SB subplan
> qual. "a != 6" is executed first because it has a security_level of 0,
> and is cheaper than the subplan. snoop() is executed later, despite
> being cheaper than the other filter quals, because it has a higher
> security_level, and leakproof() is executed last because it has the
> same security level as snoop() but is more expensive.

Will do, although I think that the next test case (the one with "a = 8")
already shows most of those behaviors.

Maybe this one's just redundant and we should drop it?

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] Compiler warnings

2016-12-29 Thread Joe Conway
On 12/06/2016 01:59 PM, Robert Haas wrote:
> On Tue, Dec 6, 2016 at 3:46 PM, Stephen Frost  wrote:
>> Good thought, thanks!
>>
>> Updated patch attached with that change and I also added an Assert() to
>> GetCachedPlan(), in case that code gets whacked around later and somehow
>> we end up falling through without actually setting *plan.
>>
>> Thoughts?
> 
> wfm
> 

Shouldn't this be back-patched? The plancache warning goes back through
9.2 (at least) and the lwlocks warning through 9.5 (or maybe it was 9.4).

Joe

-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development



signature.asc
Description: OpenPGP digital signature


Re: [HACKERS] FW: VACUUM FULL Error

2016-12-29 Thread Tom Lane
"Hayes, Patrick"  writes:
> Any suggestion how to get around this issue I am having with vacuum command 
> I’m running on 8.1 version of prostgre SQL.

You realize, I hope, that 8.1 has been out of support for more than six
years.

> The VACUUM FULL command seems to get stuck on vacuuming 
> "pg_catalog.pg_largeobject" (last message for Verbose)

If that table is very large --- check with, eg,
select pg_size_pretty(pg_relation_size('pg_largeobject'));
then VACUUM FULL is going to take a heck of a long time, particularly
with the old implementation that was used in 8.1.  But if your objective
is to return disk space to the OS, you may not have much choice; plain
VACUUM doesn't try very hard to do that.

> Error message reads as follows:
> ERROR:  out of memory
> DETAIL:  Failed on request of size 134217728.

I'm assuming that you are saying that VACUUM FULL fails with that, which
is not what "getting stuck" seems to mean otherwise.

If that happens to be equal to your current maintenance_work_mem setting,
you could probably dodge the problem by reducing maintenance_work_mem.
That would make it even slower :-( but at least you'd have hope of
completing eventually.

Personally I'd think very hard about going the dump-and-restore route and
updating to a somewhat modern version of Postgres while you're at it.
There are an awful lot of known bugs in 8.1, even assuming that you're
on the last minor release 8.1.23.

Updating to an OS that's still supported by its maker would be a bright
move as well.

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] Improving RLS planning

2016-12-29 Thread Dean Rasheed
On 28 December 2016 at 19:12, Tom Lane  wrote:
> Stephen Frost  writes:
>> * Dean Rasheed (dean.a.rash...@gmail.com) wrote:
>>> Hmm. I've not read any of the new code yet, but the fact that this
>>> test now reduces to a one-time filter makes it effectively useless as
>>> a test of qual evaluation order because it has deduced that it doesn't
>>> need to evaluate them. I would suggest replacing the qual with
>>> something that can't be reduced, perhaps "2*a = 6".
>
>> That's a good thought, I agree.
>
> [ getting back to this patch finally... ]  I made the suggested change
> to that test case, and what I see is a whole lot of "NOTICE: snooped value
> = whatever" outputs.
>
> I'd leave it as shown in the attached diff fragment, except that I'm
> a bit worried about possible platform dependency of the output.  The
> hashing occurring in the subplans shouldn't affect output order, but
> I'm not sure if we want a test output like this or not.  Thoughts?
>

How about replacing "a = 3" with "a < 7 AND a != 6". That then
exercises more of the possible types of behaviour for quals: The "a <
7" qual is pushed down and used as an index condition. The "a != 6"
qual is pushed down and used as a filter, because it's cheap and
leakproof. The leakproof() qual isn't pushed down on cost grounds. The
snoop() qual isn't pushed down on security grounds. Both snoop() and
leakproof() are used as filters, along with "a != 6", and a SB subplan
qual. "a != 6" is executed first because it has a security_level of 0,
and is cheaper than the subplan. snoop() is executed later, despite
being cheaper than the other filter quals, because it has a higher
security_level, and leakproof() is executed last because it has the
same security level as snoop() but is more expensive.

Thus the test is more likely to highlight any future changes to the
pushdown/ordering rules. The output is also neater, because nothing
ends up being printed by snoop(), although of course it is OK for
values greater than 5 to be printed.

Regards,
Dean


-- 
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] Assignment of valid collation for SET operations on queries with UNKNOWN types.

2016-12-29 Thread Tom Lane
Ashutosh Bapat  writes:
> The way this patch has been written, it doesn't allow creating tables
> with unknown type columns, which was allowed earlier.

Yes, that's an intentional change; creating such tables (or views) has
never been anything but a foot-gun.

However, I thought the idea was to silently coerce affected columns from
unknown to text.  This doesn't look like the behavior we want:

> You might want to add some testcases to test the error report e.g.
> (not necessarily in the same form) create view sv as select
> relname::unknown from pg_class;
> ERROR:  column "relname" has type "unknown"

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] VACUUM FULL Error

2016-12-29 Thread Hayes, Patrick


Hi there
Can this be forwarded to someone who can assist with this query?
Thank you, Patrick Hayes
From: Hayes, Patrick
Sent: 29 December 2016 12:26
To: 'pgsql-hackers@postgresql.org' 
Subject: FW: VACUUM FULL Error

Hi there
Any suggestion how to get around this issue I am having with vacuum command I’m 
running on 8.1 version of prostgre SQL.
The VACUUM FULL command seems to get stuck on vacuuming 
"pg_catalog.pg_largeobject" (last message for Verbose)
 Now attempting below  - but not hopeful that it will complete successfully.
> VACUUM VERBOSE pg_catalog.pg_largeobject;
With initial Message
INFO:  vacuuming "pg_catalog.pg_largeobject"
How long should I wait for this to complete – if it ever does? It has currently 
been running for over 30 minutes.

Refer to the forwarded message below for additional information.

My fallback is that an archive of the existing DB (almost 2 TBytes) has been 
made and verified (via VEEAM Clone process). It contains all of the historical 
records the need to be retained in a read-only DB. The only option I seem to 
have is to drop the DB and start with a blank canvas. Not an option I want to 
take as I am not postgre SQL expect.

Help!!!

From: Hayes, Patrick
Sent: 29 December 2016 10:53
To: 'pgadmin-supp...@postgresql.org' 
>
Subject: VACUUM FULL Error

Hi pgAdmin support,

Recently truncated tables in Postgre SQL DB and now unable to free up disk 
space on server with the VACUUM FULL command.
The DB is 1.91TBytes in size.
Server is running Win 2008 R2 Standard Operating System.
Postgre SQL is version 8.1

Error message reads as follows:
ERROR:  out of memory
DETAIL:  Failed on request of size 134217728.

Adding memory to server does not seem to make any difference. Re-starting the 
server makes no difference. The VACUUM command always ends in the same record 
134217728.
Is there any way to selectively VACUUM tables – divide and conquer approach.
Thank you in advance for any support you can provide

Patrick Hayes
[cid:image001.jpg@01D261CF.0E36C280]



[HACKERS] FW: VACUUM FULL Error

2016-12-29 Thread Hayes, Patrick
Hi there
Any suggestion how to get around this issue I am having with vacuum command I’m 
running on 8.1 version of prostgre SQL.
The VACUUM FULL command seems to get stuck on vacuuming 
"pg_catalog.pg_largeobject" (last message for Verbose)
 Now attempting below  - but not hopeful that it will complete successfully.
> VACUUM VERBOSE pg_catalog.pg_largeobject;
With initial Message
INFO:  vacuuming "pg_catalog.pg_largeobject"
How long should I wait for this to complete – if it ever does? It has currently 
been running for over 30 minutes.

Refer to the forwarded message below for additional information.

My fallback is that an archive of the existing DB (almost 2 TBytes) has been 
made and verified (via VEEAM Clone process). It contains all of the historical 
records the need to be retained in a read-only DB. The only option I seem to 
have is to drop the DB and start with a blank canvas. Not an option I want to 
take as I am not postgre SQL expect.

Help!!!
From: Hayes, Patrick
Sent: 29 December 2016 10:53
To: 'pgadmin-supp...@postgresql.org' 
Subject: VACUUM FULL Error

Hi pgAdmin support,

Recently truncated tables in Postgre SQL DB and now unable to free up disk 
space on server with the VACUUM FULL command.
The DB is 1.91TBytes in size.
Server is running Win 2008 R2 Standard Operating System.
Postgre SQL is version 8.1

Error message reads as follows:
ERROR:  out of memory
DETAIL:  Failed on request of size 134217728.

Adding memory to server does not seem to make any difference. Re-starting the 
server makes no difference. The VACUUM command always ends in the same record 
134217728.
Is there any way to selectively VACUUM tables – divide and conquer approach.
Thank you in advance for any support you can provide

Patrick Hayes
[cid:image001.jpg@01D261C1.B01E8BE0]



Re: [HACKERS] proposal: session server side variables

2016-12-29 Thread Fabien COELHO


Hello Pavel,


There are two concepts - both can be implemented, and used (can be used
together).


That is one point I would like to ascertain clearly and explicitely, so 
having various designs side by side, eg in the wiki page, would help if 
and where they interact.


The second point I am keen on discussing is how the proposed designs 
provide a solution to different use cases, and at what cost.


I've added sections about use cases (I have listed 3) and how they could 
be supported in the wiki page.


Both these concepts has some advantage and some disadvantages. It is 
hard to expect, so there is possible full agreement - because everybody 
has different preferences.


Sure.


I understand so for you can be your proposal more readable, but for me,
your design of usage and security looks not well.


Yep, there are pros and cons to all proposals. I wish they are listed 
somewhere, and possibly discussed, because some pros/cons depends on 
some detailed features.


It is acceptable without PRIVATE flags and similar flags. It is not 
designed be secure.


Indeed. I've taken this point somehow into account and changed my proposal 
so that session variables are private by default, and now I'm not even 
sure that there should exist public session variables at all...


(MySQL has nothing similar, I don't know if MSSQL has some, but probably 
not). Ok. We have different priorities. For you is not usual so in one 
session there can be more more times switch of secure context. It is 
usual for me, and for applications what I write.


I have added a section in the wiki to present succintely existing stuff in 
other products.



Could you put your ideal (final) design proposition on the wiki page?

yes, I'll do it.


Good!

--
Fabien.


--
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] Indirect indexes

2016-12-29 Thread Tom Lane
Alvaro Herrera  writes:
> One exception: in htup_details.h I changed the definition of
> HeapTupleHeaderIsHeapOnly() so that it returns a true boolean rather
> than a random bit in a wider integer.  Without this change, my compiler
> complains when the result is passed as a bool argument into a function.

That should probably get committed as a separate bug fix, and
back-patched.  Since HEAP_ONLY_TUPLE is 0x8000, the result of the existing
macro would actually fail to fit into a bool variable, meaning we have a
real coding hazard here.  It would be nasty if code developed in HEAD
failed when back-patched because of that.

Andres ran around and fixed a bunch of these hazards before, but
evidently missed this one.

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] proposal: session server side variables

2016-12-29 Thread Pavel Stehule
2016-12-29 14:41 GMT+01:00 Pavel Stehule :

>
>
> 2016-12-29 14:25 GMT+01:00 Fabien COELHO :
>
>>
>> I newer talked about persistent data. I talked about persistent metadata.
>>>
>>
>> Sure, I finally understood that detail. Now if I hear "persistent
>> variable", I by default understand that both metadata and data are
>> persistent... It requires some effort to understand the subtelty.
>>
>> I really don't propose any possible substitution of tables (relations). I
>>> newer did it.
>>>
>>
>> Sure.
>>
>> The used terminology is not 100% clean and natural - maybe better name is
>>> "global temporary unshared untransactional unrelational storage" -
>>>
>>
>> Hmmm. Too long:-) But these properties need to be spelled out.
>>
>> [...] I don't see any sense to have two similar storages or two redundant
>>> access methods - not in PostgreSQL level.
>>>
>>
>> Then say so in the wiki in the cons.
>>
>> Personnaly, I'm not sure. Maybe having a clean way of declaring a one-row
>> "singleton" table enforced by postgresql would be enough.
>
>
> There is a singleton table :)
>
> create table foo(x integer unique not null default 1 check(x = 1), y
>  integer);
> insert into foo(y) values(100);
> analyze foo;
>
> The storage is not important and is not interesting - any different behave
> for persistent objects different than MVCC can be big surprise for users.
>

our sequences -  simple, persistent, and not ACID - I found lot of people
that cannot accept it - not quickly


>
> What is interesting are getter functions - they can be volatile or
> stable/immutable - what can be interesting, because then the value can be
> used by planner.
>
> For example - MySQL @var is volatile - can be changed in query - that's
> mean, you cannot use it as const for planner :( - the behave will be same
> (with same risks to performance) like using plpgsql variable in query.
>
> With getter functions you can do bigger game.
>
> Regards
>
> Pavel
>
>
>>
>> --
>> Fabien.
>>
>
>


Re: [HACKERS] proposal: session server side variables

2016-12-29 Thread Pavel Stehule
2016-12-29 14:25 GMT+01:00 Fabien COELHO :

>
> I newer talked about persistent data. I talked about persistent metadata.
>>
>
> Sure, I finally understood that detail. Now if I hear "persistent
> variable", I by default understand that both metadata and data are
> persistent... It requires some effort to understand the subtelty.
>
> I really don't propose any possible substitution of tables (relations). I
>> newer did it.
>>
>
> Sure.
>
> The used terminology is not 100% clean and natural - maybe better name is
>> "global temporary unshared untransactional unrelational storage" -
>>
>
> Hmmm. Too long:-) But these properties need to be spelled out.
>
> [...] I don't see any sense to have two similar storages or two redundant
>> access methods - not in PostgreSQL level.
>>
>
> Then say so in the wiki in the cons.
>
> Personnaly, I'm not sure. Maybe having a clean way of declaring a one-row
> "singleton" table enforced by postgresql would be enough.


There is a singleton table :)

create table foo(x integer unique not null default 1 check(x = 1), y
 integer);
insert into foo(y) values(100);
analyze foo;

The storage is not important and is not interesting - any different behave
for persistent objects different than MVCC can be big surprise for users.

What is interesting are getter functions - they can be volatile or
stable/immutable - what can be interesting, because then the value can be
used by planner.

For example - MySQL @var is volatile - can be changed in query - that's
mean, you cannot use it as const for planner :( - the behave will be same
(with same risks to performance) like using plpgsql variable in query.

With getter functions you can do bigger game.

Regards

Pavel


>
> --
> Fabien.
>


Re: [HACKERS] proposal: session server side variables

2016-12-29 Thread Fabien COELHO



I newer talked about persistent data. I talked about persistent metadata.


Sure, I finally understood that detail. Now if I hear "persistent 
variable", I by default understand that both metadata and data are 
persistent... It requires some effort to understand the subtelty.


I really don't propose any possible substitution of tables (relations). 
I newer did it.


Sure.


The used terminology is not 100% clean and natural - maybe better name is
"global temporary unshared untransactional unrelational storage" -


Hmmm. Too long:-) But these properties need to be spelled out.

[...] I don't see any sense to have two similar storages or two 
redundant access methods - not in PostgreSQL level.


Then say so in the wiki in the cons.

Personnaly, I'm not sure. Maybe having a clean way of declaring a one-row 
"singleton" table enforced by postgresql would be enough.


--
Fabien.


--
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] Assignment of valid collation for SET operations on queries with UNKNOWN types.

2016-12-29 Thread Ashutosh Bapat
The way this patch has been written, it doesn't allow creating tables
with unknown type columns, which was allowed earlier. That breaks
backward compatibility. Users, who have created such tables will face
problems while loading dumps from earlier versions. pg_upgrade might
be an issue, but we may modify it to convert those columns to text.
Given that a column with unknown type is pretty much useless unless
casted to some other type, there may not be many such users out there.
But we should probably add a notice in release notes.

+-- check coercion of UNKNOWN type to text for literal constants
+create view v as select 'abc' a;
+create view v11 as select 'def' a;
+select a from v UNION select a from v11;
+

The comment says that it's testing coercion of unknown to text, but
nothing in the test guarantees that constant literals were casted to
text. The union could give the expected result if code merging the two
views knew how to handle unknown types. Instead \d v or \d v11 would
be a better test to test what the comment says. If we want to test
such a union the test is fine, but the comment needs to change.

For a materialized view you may have to modify
transformCreateTableAsStmt() to modify at targetlist of the query
similar to DefineVirtualRelation(), but I think that can be done as a
separate patch.

You might want to add some testcases to test the error report e.g.
(not necessarily in the same form) create view sv as select
relname::unknown from pg_class;
ERROR:  column "relname" has type "unknown"

On Wed, Dec 14, 2016 at 3:32 PM, Rahila Syed  wrote:
> Hello,
>
> Thank you for comments.
>
>>There is a similar code pattern for materialized views, see
>>create_ctas_nodata() where the attribute list is built
> create_ctas_nodata() is for creation of materialized views WITH NO DATA.
> For other materialized views and CREATE TABLE AS, column definitions are
> built in
> intorel_startup() function which has different code from that of CREATE VIEW
> which
> the patch deals with.
>
> Limiting the scope of the patch to include changing the type of literal
> constants
> to text only for plain views. Also, error out when column with UNKNOWN type
> is
> being created for other relations like tables and materialized views.
>
>>And actually, shouldn't this be just a plain error?
> Changed it to error in the attached patch.
>
>>Your patch has no regression tests, surely you want some to stress
>>this code path
> Added regression tests in the attached patch.
>
> Also adding this patch to CF 2017-01
>
> Thank you,
> Rahila Syed
>
>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>



-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


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


Re: [HACKERS] proposal: session server side variables

2016-12-29 Thread Pavel Stehule
2016-12-29 11:42 GMT+01:00 Fabien COELHO :

>
> yes, I'll do it.
>>>
>>
>> But I'll remove some strange ideas.
>>
>
> Why?
>
> I rather expect that you would comment that you find them strange and
> argue why: there is no reason to "remove" a concept idea as such at least
> early in a discussion process...
>
> Why persistent variables?
>>
>
> Because *you* want persistent session variables... I did not invent it, I
> just removed the "session" word and generalized the concept.
>

I newer talked about persistent data. I talked about persistent metadata. I
really don't propose any possible substitution of tables (relations). I
newer did it.

The used terminology is not 100% clean and natural - maybe better name is
"global temporary unshared untransactional unrelational storage" - when I
use a terminology related to temporary tables.

I don't see any sense to have two similar storages or two redundant access
methods - not in PostgreSQL level.


>
> Sometimes one wants to store sets, sometimes one wants to only store value.
>
> Please, one argument. We have tables. What is wrong on tables?
>>
>
> Nothing is wrong as such. Cons arguments are: the syntax is verbose just
> for one scalar, and the one-row property is currently not easily enforced
> by pg, AFAIK.
>
> Note that I'm not claiming that it should be implemented, but if some kind
> of half-persistent variables are implemented, I think it should be
> consistent with possibly fully-persistent variable as well, even if they
> are not implemented immediately, or ever.


There is small language barrier :) - I am thinking about features, what we
should to implement, not what can be implemented. There is lot of
possibilities, that we can find in universe. But only few has
technical/practical sense.

SQL is verbose language - I like it - because it different - the developer
have to know when he is working with database, with persistent data. The
cost of access to data is significantly higher. When this information is
hidden from developer, then the result is terrible slow.

Regards

Pavel



>
>
> Anything what will be persistent will have similar performance like tables.
>>
>
> Yes, sure. It is a different use case. Argue in the wiki!
>
> --
> Fabien.
>


Re: [HACKERS] proposal: session server side variables

2016-12-29 Thread Fabien COELHO


[Oops, resent, wrong from address, please accept my apologies]

Hello Pavel,


There are two concepts - both can be implemented, and used (can be used
together).


That is one point I would like to ascertain clearly and explicitely, so having 
various designs side by side, eg in the wiki page, would help if and where they 
interact.


The second point I am keen on discussing is how the proposed designs provide a 
solution to different use cases, and at what cost.


I've added sections about use cases (I have listed 3) and how they could be 
supported in the wiki page.


Both these concepts has some advantage and some disadvantages. It is hard to 
expect, so there is possible full agreement - because everybody has different 
preferences.


Sure.


I understand so for you can be your proposal more readable, but for me,
your design of usage and security looks not well.


Yep, there are pros and cons to all proposals. I wish they are listed 
somewhere, and possibly discussed, because some pros/cons depends on some 
detailed features.


It is acceptable without PRIVATE flags and similar flags. It is not designed 
be secure.


Indeed. I've taken this point somehow into account and changed my proposal so 
that session variables are private by default, and now I'm not even sure that 
there should exist public session variables at all...


(MySQL has nothing similar, I don't know if MSSQL has some, but probably 
not). Ok. We have different priorities. For you is not usual so in one 
session there can be more more times switch of secure context. It is usual 
for me, and for applications what I write.


I have added a section in the wiki to present succintely existing stuff in 
other products.



Could you put your ideal (final) design proposition on the wiki page?

yes, I'll do it.


Good!

--
Fabien.



--
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] proposal: session server side variables

2016-12-29 Thread Fabien COELHO



yes, I'll do it.


But I'll remove some strange ideas.


Why?

I rather expect that you would comment that you find them strange and 
argue why: there is no reason to "remove" a concept idea as such at least 
early in a discussion process...



Why persistent variables?


Because *you* want persistent session variables... I did not invent it, 
I just removed the "session" word and generalized the concept.


Sometimes one wants to store sets, sometimes one wants to only store 
value.



Please, one argument. We have tables. What is wrong on tables?


Nothing is wrong as such. Cons arguments are: the syntax is verbose just 
for one scalar, and the one-row property is currently not easily enforced 
by pg, AFAIK.


Note that I'm not claiming that it should be implemented, but if some kind 
of half-persistent variables are implemented, I think it should be 
consistent with possibly fully-persistent variable as well, even if they 
are not implemented immediately, or ever.


Anything what will be persistent will have similar performance like 
tables.


Yes, sure. It is a different use case. Argue in the wiki!

--
Fabien.


--
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] proposal: session server side variables

2016-12-29 Thread Pavel Stehule
2016-12-29 10:32 GMT+01:00 Pavel Stehule :

>
>
> 2016-12-29 10:11 GMT+01:00 Fabien COELHO :
>
>>
>> There is big difference - you concept missing any safe point. You have to
>>> specify same information more times.
>>>
>>
>> Not necessarily, and if so maybe twice. I'm ok to recognize that it is a
>> difference between both approaches, and an inconvenient of the one I'm
>> proposing. There also see inconvenients to the other design as well, so
>> there will not be any perfect solution, IMO. That is the point of
>> discussing.
>
>
>>
>> I am sorry, this discussion is in cycle - there is no sense to continue.
>>>
>>
>> If the only open option is to agree with your initial design, then
>> obviously this is not a path for reaching a consensus.
>>
>
> There are two concepts - both can be implemented, and used (can be used
> together). Both these concepts has some advantage and some disadvantages.
> It is hard to expect, so there is possible full agreement - because
> everybody has different preferences.
>
> I understand so for you can be your proposal more readable, but for me,
> your design of usage and security looks not well. It is acceptable without
> PRIVATE flags and similar flags. It is not designed be secure. (MySQL has
> nothing similar, I don't know if MSSQL has some, but probably not). Ok. We
> have different priorities. For you is not usual so in one session there can
> be more more times switch of secure context. It is usual for me, and for
> applications what I write.
>
>
>>
>> Could you put your ideal (final) design proposition on the wiki page?
>> That would avoid repeating the same cyclic arguments, they would be written
>> only once...
>
>
> yes, I'll do it.
>

But I'll remove some strange ideas. Why persistent variables?

Please, one argument. We have tables. What is wrong on tables?

Anything what will be persistent will have similar performance like tables.

Regards

Pavel


>
> Regards
>
> Pavel
>
>
>>
>>
>> --
>> Fabien.
>>
>
>


Re: [HACKERS] Potential data loss of 2PC files

2016-12-29 Thread Ashutosh Bapat
On Thu, Dec 22, 2016 at 7:00 AM, Michael Paquier
 wrote:
> Hi all,
>
> 2PC files are created using RecreateTwoPhaseFile() in two places currently:
> - at replay on a XLOG_XACT_PREPARE record.
> - At checkpoint with CheckPointTwoPhase().
>
> Now RecreateTwoPhaseFile() is careful to call pg_fsync() to be sure
> that the 2PC files find their way into disk. But one piece is missing:
> the parent directory pg_twophase is not fsync'd. At replay this is
> more sensitive if there is a PREPARE record followed by a checkpoint
> record. If there is a power failure after the checkpoint completes
> there is a risk to lose 2PC status files here.
>
> It seems to me that we really should have CheckPointTwoPhase() call
> fsync() on pg_twophase to be sure that no files are lost here. There
> is no point to do this operation in RecreateTwoPhaseFile() as if there
> are many 2PC transactions to replay performance would be impacted, and
> we don't care about the durability of those files until a checkpoint
> moves the redo pointer. I have drafted the patch attached to address
> this issue.

I agree with this.
If no prepared transactions were required to be fsynced
CheckPointTwoPhase(), do we want to still fsync the directory?
Probably not.

May be you want to call fsync_fname(TWOPHASE_DIR, true); if
serialized_xacts > 0.

The comment just states what has been done earlier in the function,
which is documented in the prologue as well.
/*
 * Make sure that the content created is persistent on disk to prevent
 * data loss in case of an OS crash or a power failure. Each 2PC file
 * has been already created and flushed to disk individually by
 * RecreateTwoPhaseFile() using the list of GXACTs available for
 * normal processing as well as at recovery when replaying individually
 * each XLOG_XACT_PREPARE record.
 */
Instead, you may want to just say "If we have flushed any 2PC files,
flush the metadata in the pg_twophase directory."

Although, I thought that a simple case of creating a persistent table
which requires creating a file also would need to flush the directory.
I tried to locate the corresponding code to see if it also uses
fsync_fname(). I couldn't locate the code. I have looked at
mdcreate(), mdpreckpt(), mdpostckpt(). But may be that's not relevant
here.


>
> I am adding that as well to the next CF for consideration.
>
> Thoughts?
> --
> Michael
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>



-- 
Best Wishes,
Ashutosh Bapat
EnterpriseDB Corporation
The Postgres Database Company


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


Re: [HACKERS] proposal: session server side variables

2016-12-29 Thread Pavel Stehule
2016-12-29 10:11 GMT+01:00 Fabien COELHO :

>
> There is big difference - you concept missing any safe point. You have to
>> specify same information more times.
>>
>
> Not necessarily, and if so maybe twice. I'm ok to recognize that it is a
> difference between both approaches, and an inconvenient of the one I'm
> proposing. There also see inconvenients to the other design as well, so
> there will not be any perfect solution, IMO. That is the point of
> discussing.


>
> I am sorry, this discussion is in cycle - there is no sense to continue.
>>
>
> If the only open option is to agree with your initial design, then
> obviously this is not a path for reaching a consensus.
>

There are two concepts - both can be implemented, and used (can be used
together). Both these concepts has some advantage and some disadvantages.
It is hard to expect, so there is possible full agreement - because
everybody has different preferences.

I understand so for you can be your proposal more readable, but for me,
your design of usage and security looks not well. It is acceptable without
PRIVATE flags and similar flags. It is not designed be secure. (MySQL has
nothing similar, I don't know if MSSQL has some, but probably not). Ok. We
have different priorities. For you is not usual so in one session there can
be more more times switch of secure context. It is usual for me, and for
applications what I write.


>
> Could you put your ideal (final) design proposition on the wiki page? That
> would avoid repeating the same cyclic arguments, they would be written only
> once...


yes, I'll do it.

Regards

Pavel


>
>
> --
> Fabien.
>


Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups

2016-12-29 Thread Craig Ringer
On 29 December 2016 at 16:51, Craig Ringer  wrote:
> On 28 December 2016 at 22:16, Petr Jelinek  
> wrote:
>
>> About the patch, it looks good to me for master with the minor exception
>> that:
>>> + appendStringInfo(buf, "pageno %d, xid %u",
>>> + trunc.pageno, trunc.oldestXid);
>>
>> This should probably say oldestXid instead of xid in the text description.
>
> Agreed.

Slightly amended attached.

-- 
 Craig Ringer   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
From e1dd1a711d65425df71db63d06ce6b6b934f12a8 Mon Sep 17 00:00:00 2001
From: Craig Ringer 
Date: Wed, 28 Dec 2016 21:23:59 +0800
Subject: [PATCH] Fix a minor race between commit_ts slru truncation and
 lookups

vac_truncate_clog was truncating the commit timestamp SLRU before
it advanced the oldest xid limit for commit timestamp lookups. This
created a small race window where a concurrent pg_xact_commit_timestamp
calling TransactionIdGetCommitTsData(...) could check the oldest
xid after the SLRU page containing it is truncated away but before
the threshold is updated.

Fix this by advancing the lower bound before removing the relevant SLRU pages.

A larger race window also existed on a standby. The lower bound for commit
timetamp validity was only advanced on a standby when we replayed a checkpoint.
This could happen a long time after the commit timestamp SLRU truncation.

This race only affects XIDs that vac_truncate_clog has determined are no longer
referenced in the system, so the minimum datfrozenxid has advanced past them.
It's never going to be triggered by code that looks up commit timestamps of
rows it's just read during an in-progress transaction. Also, the worst outcome
should the race be triggered is an I/O error from the SLRU code (where the
commit ts lookup should more correctly return null). So this race is pretty
harmless.

The nearly identical clog code has the same race, but it doesn't matter there
since there's no way for users to pass arbitrary XIDs to look them up in clog.
---
 src/backend/access/rmgrdesc/committsdesc.c |  8 +---
 src/backend/access/transam/commit_ts.c | 23 +++
 src/backend/commands/vacuum.c  |  3 ++-
 src/include/access/commit_ts.h |  8 
 4 files changed, 30 insertions(+), 12 deletions(-)

diff --git a/src/backend/access/rmgrdesc/committsdesc.c b/src/backend/access/rmgrdesc/committsdesc.c
index 527e5dc..34553d5 100644
--- a/src/backend/access/rmgrdesc/committsdesc.c
+++ b/src/backend/access/rmgrdesc/committsdesc.c
@@ -33,10 +33,12 @@ commit_ts_desc(StringInfo buf, XLogReaderState *record)
 	}
 	else if (info == COMMIT_TS_TRUNCATE)
 	{
-		int			pageno;
+		xl_commit_ts_truncate trunc;
 
-		memcpy(, rec, sizeof(int));
-		appendStringInfo(buf, "%d", pageno);
+		memcpy(, rec, SizeOfCommitTsTruncate);
+
+		appendStringInfo(buf, "pageno %d, oldestXid %u",
+			trunc.pageno, trunc.oldestXid);
 	}
 	else if (info == COMMIT_TS_SETTS)
 	{
diff --git a/src/backend/access/transam/commit_ts.c b/src/backend/access/transam/commit_ts.c
index a5b270c..86ac1d6 100644
--- a/src/backend/access/transam/commit_ts.c
+++ b/src/backend/access/transam/commit_ts.c
@@ -113,7 +113,7 @@ static bool CommitTsPagePrecedes(int page1, int page2);
 static void ActivateCommitTs(void);
 static void DeactivateCommitTs(void);
 static void WriteZeroPageXlogRec(int pageno);
-static void WriteTruncateXlogRec(int pageno);
+static void WriteTruncateXlogRec(int pageno, TransactionId oldestXid);
 static void WriteSetTimestampXlogRec(TransactionId mainxid, int nsubxids,
 		 TransactionId *subxids, TimestampTz timestamp,
 		 RepOriginId nodeid);
@@ -824,7 +824,7 @@ TruncateCommitTs(TransactionId oldestXact)
 		return;	/* nothing to remove */
 
 	/* Write XLOG record */
-	WriteTruncateXlogRec(cutoffPage);
+	WriteTruncateXlogRec(cutoffPage, oldestXact);
 
 	/* Now we can remove the old CommitTs segment(s) */
 	SimpleLruTruncate(CommitTsCtl, cutoffPage);
@@ -910,10 +910,15 @@ WriteZeroPageXlogRec(int pageno)
  * Write a TRUNCATE xlog record
  */
 static void
-WriteTruncateXlogRec(int pageno)
+WriteTruncateXlogRec(int pageno, TransactionId oldestXid)
 {
+	xl_commit_ts_truncate xlrec;
+
+	xlrec.pageno = pageno;
+	xlrec.oldestXid = oldestXid;
+
 	XLogBeginInsert();
-	XLogRegisterData((char *) (), sizeof(int));
+	XLogRegisterData((char *) (), SizeOfCommitTsTruncate);
 	(void) XLogInsert(RM_COMMIT_TS_ID, COMMIT_TS_TRUNCATE);
 }
 
@@ -967,17 +972,19 @@ commit_ts_redo(XLogReaderState *record)
 	}
 	else if (info == COMMIT_TS_TRUNCATE)
 	{
-		int			pageno;
+		xl_commit_ts_truncate trunc;
 
-		memcpy(, XLogRecGetData(record), sizeof(int));
+		memcpy(, XLogRecGetData(record), SizeOfCommitTsTruncate);
+
+		AdvanceOldestCommitTsXid(trunc.oldestXid);
 
 		/*
 		 * During XLOG replay, latest_page_number isn't set up yet; insert a
 		 * 

Re: [HACKERS] Support for pg_receivexlog --format=plain|tar

2016-12-29 Thread Magnus Hagander
On Thu, Dec 29, 2016 at 12:35 AM, Michael Paquier  wrote:

> On Wed, Dec 28, 2016 at 9:31 PM, Magnus Hagander 
> wrote:
> > Conditional tests? It probably wouldn't hurt to have them, but that
> would be
> > something more generic (like we'd need something to actually validate it
> --
> > but it would make sense to have a test that, with compression enabled,
> would
> > verify if the uncompressed file turns out to be exactly 16Mb for
> example).
>
> Looking at if the build is compiled with libz via SQL or using
> pg_config is the way to go here. A minimum is doable.
>
> >> A couple of things to be aware of though:
> >> - gzopen, gzwrite and gzclose are used to handle the gz files. That's
> >> unconsistent with the tar method that is based on mainly deflate() and
> >> more low level routines.
> >
> > But chosen for simplicity, I assume?
>
> Yep. That's quite in-line with the current code.
>

Agreed.



> >> - I have switched the directory method to use a file pointer instead
> >> of a file descriptor as gzwrite returns int as the number of
> >> uncompressed bytes written.
> >
> > I don't really follow that reasoning :) Why does the directory method
> have
> > to change to use a filepointer because of that?
>
> The only reason is that write() returns size_t and fwrite returns int,
> while gzwrite() returns int. It seems more consistent to use fwrite()
> in this case. Or we don't bother about my nitpicking and just cast
> stuff.
>

I can at least partially see that argument, but your patch doesn't actually
use fwrite(), it uses write() with fileno()...

But also, on my platform (debian jessie), fwrite() returns size_t, and
write() returns ssize_t. So those are apparently both different from what
your platform does - which one did you get that one?

(gzwrite does return int)

-- 
 Magnus Hagander
 Me: http://www.hagander.net/
 Work: http://www.redpill-linpro.com/


Re: [HACKERS] proposal: session server side variables

2016-12-29 Thread Fabien COELHO



There is big difference - you concept missing any safe point. You have to
specify same information more times.


Not necessarily, and if so maybe twice. I'm ok to recognize that it is a 
difference between both approaches, and an inconvenient of the one I'm 
proposing. There also see inconvenients to the other design as well, so 
there will not be any perfect solution, IMO. That is the point of 
discussing.



I am sorry, this discussion is in cycle - there is no sense to continue.


If the only open option is to agree with your initial design, then 
obviously this is not a path for reaching a consensus.


Could you put your ideal (final) design proposition on the wiki page? That 
would avoid repeating the same cyclic arguments, they would be written 
only once...


--
Fabien.


--
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] proposal: session server side variables

2016-12-29 Thread Pavel Stehule
2016-12-29 9:57 GMT+01:00 Fabien COELHO :

>
> Please, could you remove the part of the mail you are not responding to
> and just keep the relevant part?
>
> Whatever the features and syntax, you can always shoot yourself in the
>>> foot.
>>>
>>
>> I disagree
>>
>
> Hmmm... I have succeeded in shotting myself in the foot with possibly
> every feature of every language I have used. This is called experience...
> in the end you do know how NOT to do things:-)
>
> - some concepts are more robust, other less.
>>
>
> Sure. The use-case under discussion is about ONE session variable holding
> an expensive to compute security status which can be consulted by other
> functions. I think that probably one can have it right with both
> approaches, even if it is on the second try...


The robustness in not only about content, but about code maintenance,
possibility to quickly search errors or better don't do errors



>
>
> --
> Fabien.
>


Re: [HACKERS] proposal: session server side variables

2016-12-29 Thread Fabien COELHO


Please, could you remove the part of the mail you are not responding to 
and just keep the relevant part?



Whatever the features and syntax, you can always shoot yourself in the
foot.


I disagree


Hmmm... I have succeeded in shotting myself in the foot with possibly 
every feature of every language I have used. This is called experience... 
in the end you do know how NOT to do things:-)



- some concepts are more robust, other less.


Sure. The use-case under discussion is about ONE session variable holding 
an expensive to compute security status which can be consulted by other 
functions. I think that probably one can have it right with both 
approaches, even if it is on the second try...


--
Fabien.


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


Re: [HACKERS] [PATCH] Fix minor race in commit_ts SLRU truncation vs lookups

2016-12-29 Thread Craig Ringer
On 28 December 2016 at 22:16, Petr Jelinek  wrote:

> About the patch, it looks good to me for master with the minor exception
> that:
>> + appendStringInfo(buf, "pageno %d, xid %u",
>> + trunc.pageno, trunc.oldestXid);
>
> This should probably say oldestXid instead of xid in the text description.

Agreed.

> About back-patching, I wonder if standby could be modified to move
> oldestXid based on pageno reverse calculation, but it's going to be
> slightly ugly.

Not worth it IMO.

-- 
 Craig Ringer   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


Re: [HACKERS] proposal: session server side variables

2016-12-29 Thread Pavel Stehule
2016-12-29 9:46 GMT+01:00 Fabien COELHO :

>
> CREATE FUNCTION setup_user(TEXT, TEXT)
>>>   RETURNS BOOLEAN SECURITY DEFINER AS $$
>>>
>>
> CREATE FUNCTION isUserAuditor()
>>>RETURNS BOOLEAN SECURITY DEFINER AS $$
>>>
>>
>> so what is worse - I did one new entry in pg_class and one entry in
>> pg_attributes. You wrote two entries in pg_proc function - more you have
>> to
>> ensure consistency of these functions.
>>
>
> You are not comparing the same perimeter, the setup_user() function is
> necessary to both approaches for the described use case where a read-only
> value is needed:
>
> With your approach:
>
>   1. CREATE VARIABLE secure_stuff SESSION SCOPE ...
>   2. REVOKE/GRANT ... on VARIABLE secure_stuff
>   3. CREATE FUNCTION setup_user(...)
>
> With this approach:
>
>   1. CREATE FUNCTION access_secure_stuff(...)
>   2. REVOKE/GRANT ... on FUNCTION access_secure_stuff
>   3. CREATE FUNCTION setup_user(...)
>
> The REVOKE/GRANT are basically the same on VARIABLE and on FUNCTION.
>
> So it is not really that different as far as catalog entry count is
> concerned.
>
> The benefit is that it avoids a special concept and use a more generic
> one, i.e. basic session variables.
>

There is big difference - you concept missing any safe point. You have to
specify same information more times.

I am sorry, this discussion is in cycle - there is no sense to continue.

Regards

Pavel



>
> The added cost is that a two line function must be written, which does not
> look like a big issue to implement a pretty special use case.
>
> --
> Fabien.
>


Re: [HACKERS] proposal: session server side variables

2016-12-29 Thread Fabien COELHO



CREATE FUNCTION setup_user(TEXT, TEXT)
  RETURNS BOOLEAN SECURITY DEFINER AS $$



CREATE FUNCTION isUserAuditor()
   RETURNS BOOLEAN SECURITY DEFINER AS $$


so what is worse - I did one new entry in pg_class and one entry in
pg_attributes. You wrote two entries in pg_proc function - more you have to
ensure consistency of these functions.


You are not comparing the same perimeter, the setup_user() function is 
necessary to both approaches for the described use case where a read-only 
value is needed:


With your approach:

  1. CREATE VARIABLE secure_stuff SESSION SCOPE ...
  2. REVOKE/GRANT ... on VARIABLE secure_stuff
  3. CREATE FUNCTION setup_user(...)

With this approach:

  1. CREATE FUNCTION access_secure_stuff(...)
  2. REVOKE/GRANT ... on FUNCTION access_secure_stuff
  3. CREATE FUNCTION setup_user(...)

The REVOKE/GRANT are basically the same on VARIABLE and on FUNCTION.

So it is not really that different as far as catalog entry count is 
concerned.


The benefit is that it avoids a special concept and use a more generic 
one, i.e. basic session variables.


The added cost is that a two line function must be written, which does not 
look like a big issue to implement a pretty special use case.


--
Fabien.


--
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] proposal: session server side variables

2016-12-29 Thread Pavel Stehule
2016-12-29 9:36 GMT+01:00 Fabien COELHO :

>
> Hello Pavel,
>
> Hmmm... I know a little bit about that field, ISTM that you are speaking
>>> of the current capabilities of a particular static analysis tool, but I'm
>>> not sure that the tool capabilities could not be enhanced to manage more
>>> things.
>>>
>>
>> It cannot be - the static analyze is limited to function scope - in static
>> analyze you don't know a order of calls.
>>
>
> I have been doing interprocedural static analysis for the last 25 years,
> and I can assure you that those techniques are not limited to the scope of
> a function. As for global variables, I agree that you may proove more
> things about them if you know the order of calls.
>
> The private session variables I suggested have a fixed (static) name, and
>>> their type could be infered by a static analysis tool, eg:
>>>   ...
>>>   DECLARE @foo BOOLEAN PRIVATE;
>>>   -- I know that there is private a boolean variable "@foo" of unknown
>>> value
>>>   SET @foo = TRUE;
>>>   --- I know that @foo is true...
>>>   ...
>>>
>>
>> This is not too friendly
>>
>
> Friendly is subjective. ISTM That it gets the job done with minimal syntax
> and implementation. It avoids getter/setter functions which are unfriendly
> to me.rst


getter/setter functions are implementation in first step. I spoke, so this
step is not last.


>
>
> - you have to repeat DECLARE in every function.
>>
>
> That is the same in nearly all programming languages, you have to declare
> external variables somehow before using them, that is life.
>
> Some declarations could be avoided if an unknown variable is assumed to
> have untyped value NULL by default.
>
> What is somebody somewhere write @fooo
>>
>
> NULL ? Unkown variable error ?
>
> or use DECLARE @foo integer instead.
>>
>
> It would not matter if it is someone else, because @foo would be their
> private version. If it is yourself, an error could be raised if a session
> variable is found to be declared with two distinct types. A static analysis
> tool would be able to detect that as well.
>
> There is big space for errors.
>>
>
> Whatever the features and syntax, you can always shoot yourself in the
> foot.
>

I disagree - some concepts are more robust, other less.

Regards

Pavel


>
> I have open a wiki page to help with this discussion:
>
> https://wiki.postgresql.org/wiki/Variable_Design
>
> --
> Fabien.
>


Re: [HACKERS] proposal: session server side variables

2016-12-29 Thread Fabien COELHO


Hello Pavel,


Hmmm... I know a little bit about that field, ISTM that you are speaking
of the current capabilities of a particular static analysis tool, but I'm
not sure that the tool capabilities could not be enhanced to manage more
things.


It cannot be - the static analyze is limited to function scope - in static
analyze you don't know a order of calls.


I have been doing interprocedural static analysis for the last 25 years, 
and I can assure you that those techniques are not limited to the scope of 
a function. As for global variables, I agree that you may proove more 
things about them if you know the order of calls.



The private session variables I suggested have a fixed (static) name, and
their type could be infered by a static analysis tool, eg:
  ...
  DECLARE @foo BOOLEAN PRIVATE;
  -- I know that there is private a boolean variable "@foo" of unknown
value
  SET @foo = TRUE;
  --- I know that @foo is true...
  ...


This is not too friendly


Friendly is subjective. ISTM That it gets the job done with minimal syntax 
and implementation. It avoids getter/setter functions which are unfriendly 
to me.



- you have to repeat DECLARE in every function.


That is the same in nearly all programming languages, you have to declare 
external variables somehow before using them, that is life.


Some declarations could be avoided if an unknown variable is assumed to 
have untyped value NULL by default.



What is somebody somewhere write @fooo


NULL ? Unkown variable error ?


or use DECLARE @foo integer instead.


It would not matter if it is someone else, because @foo would be their 
private version. If it is yourself, an error could be raised if a session 
variable is found to be declared with two distinct types. A static 
analysis tool would be able to detect that as well.



There is big space for errors.


Whatever the features and syntax, you can always shoot yourself in the 
foot.


I have open a wiki page to help with this discussion:

https://wiki.postgresql.org/wiki/Variable_Design

--
Fabien.


--
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] proposal: session server side variables

2016-12-29 Thread Fabien COELHO


Hello Jim,


Yes if the private variable should be accessed. If the variable is
private, then it is private and nothing is needed. Idem for public.


Why force the extra busywork? Just allow for them to be public.


There is no extra busywork, having possibly private session variables cost 
nearly nothing, and I'm arguing that they could be enough for Pavel 
special use case.


For that matter, if we're going to start introducing private objects, that 
certainly needs to be thought through.


Yep. It is a discussion.


One can get the permissions on special session variable wrong as well...
I do not see how it differs.


It's a lot harder to mess up an explicit grant than it is to mess up object 
ownership.


ISTM that it rather depends on the default permissions on the object... If 
a variable object is publicly accessible by default, then you have to take 
care about revoke & grant and you can mess up with it. Moreover, it also 
suggest that session variables could be private by default.



 *@db> SELECT secfunc2(); -- returns: "postgres - fabien" from both
sides...


Perhaps I've got the restrictions on SECDEF wrong, but I know there's 
problems with it. Certainly one issue is you can't change roles back to the 
calling user.


It is the same with suid executable on Unix, I do not see as an issue.

Maybe they would be workable in this case, but it's just a bunch of extra 
busywork for the user that serves no real purpose.


Pavel special case is the purpose.

Then IMHO what needs to happen is to have a discussion on actual syntax 
instead of calling into question the value of the feature.


I think that the discussion should be *both* about syntax and semantics.

Following this thread has been very painful because the communications 
have not been very clear.


Yep. The fact that I initially missed the "half persistence" property of 
Pavel's design has not helped.


I suggest to move the discussion on the following wiki page that I have 
just bootstrapped:


https://wiki.postgresql.org/wiki/Variable_Design

--
Fabien


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