Re: [HACKERS] Parallel build with MSVC

[Christian Ullrich]

* Michael Paquier wrote:


On Wed, Apr 27, 2016 at 5:15 PM, Christian Ullrich  wrote:

OK then, hopefully last round attached.


Thanks. Those are fine in my view. It is hard to tell if a committer
is going to have a look at that soon, so I think that it would be
wiser to add that to the next CF so as those patches don't fall into
oblivion.


Done.

--
Christian



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

[HACKERS] relocation truncated to fit: citus build failure on s390x

[Christoph Berg]

[Cc'ing -hackers]

I said:
> That said, there's a build failure on s390x:
> 
> https://buildd.debian.org/status/fetch.php?pkg=citus&arch=s390x&ver=5.0.1-1&stamp=1461670278
> 
> gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement 
> -Wendif-labels -Wmissing-format-attribute
> -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -g 
> -g -O2 -fstack-protector-strong -Wformat
> -Werror=format-security -I/usr/include/mit-krb5 -fPIC -pie  -fpic -g -O2 
> -fstack-protector-strong -Wformat
> -Werror=format-security  -Wall -Wextra -Wno-unused-parameter 
> -Wno-sign-compare -Wno-missing-field-initializers
> -Wno-clobbered -Wdeclaration-after-statement -Wendif-labels 
> -Wmissing-format-attribute -Wmissing-declarations
> -Wmissing-prototypes -shared -o citus.so ./shared_library_init.o 
> commands/create_distributed_table.o
> commands/transmit.o executor/multi_utility.o executor/multi_router_executor.o 
> executor/multi_server_executor.o
> executor/multi_real_time_executor.o executor/multi_executor.o 
> executor/multi_task_tracker_executor.o
> executor/multi_client_executor.o master/master_stage_protocol.o 
> master/master_metadata_utility.o
> master/master_delete_protocol.o master/master_repair_shards.o 
> master/master_create_shards.o
> master/worker_node_manager.o master/master_node_protocol.o 
> planner/multi_physical_planner.o
> planner/multi_master_planner.o planner/multi_explain.o 
> planner/multi_logical_planner.o planner/multi_planner.o
> planner/multi_join_order.o planner/modify_planner.o 
> planner/multi_logical_optimizer.o relay/relay_event_utility.o
> test/generate_ddl_commands.o test/connection_utils.o test/prune_shard_list.o 
> test/distribution_metadata.o
> test/test_helper_functions.o test/connection_cache.o test/fake_fdw.o 
> test/create_shards.o utils/listutils.o
> utils/citus_readfuncs_94.o utils/ruleutils_94.o utils/multi_resowner.o 
> utils/resource_lock.o utils/citus_outfuncs.o
> utils/metadata_cache.o utils/connection_cache.o utils/citus_read.o 
> utils/citus_ruleutils.o utils/citus_nodefuncs.o
> utils/citus_readfuncs_95.o utils/ruleutils_95.o 
> worker/worker_partition_protocol.o worker/task_tracker_protocol.o
> worker/task_tracker.o worker/worker_file_access_protocol.o 
> worker/worker_data_fetch_protocol.o
> worker/worker_merge_protocol.o -L/usr/lib/s390x-linux-gnu -Wl,-z,relro 
> -Wl,-z,now -Wl,--as-needed -L/usr/lib/mit-krb5
> -L/usr/lib/s390x-linux-gnu/mit-krb5  -Wl,--as-needed -Wl,-z,relro  
> -L/usr/lib/s390x-linux-gnu -lpq
> utils/ruleutils_95.o: In function `simple_quote_literal':
> /«PKGBUILDDIR»/src/backend/distributed/utils/ruleutils_95.c:6114:(.text+0x596):
>  relocation truncated to fit:
> R_390_GOT12 against undefined symbol `standard_conforming_strings'
> 
> I've had that problem before, but forgot most of it. I think the
> problem is that "-fPIC -pie  -fpic" isn't consistent and should
> probably be "-fPIC -PIE", but I might be totally wrong. (I'll try to
> dig up some docs.)

Re: Andres Freund 2016-04-28 <20160427230516.44fuzdxlifch2...@alap3.anarazel.de>
> > Just talked to a s390x porter, the flags used should be -fPIC -pie",
> > i.e. without the lower case version.
> 
> Afaics, that's citus independent, and coming from postgres code.
> Makefile.port:
> ifeq "$(findstring sparc,$(host_cpu))" "sparc"
> CFLAGS_SL = -fPIC
> else
> CFLAGS_SL = -fpic
> endif
> 
> I'm not clear why citus triggers this, when other extensions don't?

Maybe it's simply because citus.so is bigger than all the other
extension .so files:

   -fpic
   Generate position-independent code (PIC) suitable for use
   in a shared library, if supported for the target machine.
   Such code accesses all constant addresses through a global
   offset table (GOT).  The dynamic loader resolves the GOT
   entries when the program starts (the dynamic loader is not
   part of GCC; it is part of the operating system).  If the
   GOT size for the linked executable exceeds a machine-
   specific maximum size, you get an error message from the
   linker indicating that -fpic does not work; in that case,
   recompile with -fPIC instead.  (These maximums are 8k on
   the SPARC and 32k on the m68k and RS/6000.  The 386 has no
   such limit.)

   Position-independent code requires special support, and
   therefore works only on certain machines.  For the 386, GCC
   supports PIC for System V but not for the Sun 386i.  Code
   generated for the IBM RS/6000 is always
   position-independent.

   When this flag is set, the macros "__pic__" and "__PIC__"
   are defined to 1.

   -fPIC
   If supported for the target machine, emit
   position-independent code, suitable for dynamic linking and
   avoiding any limit on the size of the global offset table.
   This option makes a difference on the m68k,

Re: [HACKERS] Support for N synchronous standby servers - take 2

[Kyotaro HORIGUCHI]

Hello,

At Wed, 27 Apr 2016 18:05:26 -0400, Tom Lane  wrote in 
<3167.1461794...@sss.pgh.pa.us>
> Kyotaro HORIGUCHI  writes:
> > Sorry, I have attached an empty patch. This is another one that should
> > be with content.
> 
> I pushed this after whacking it around some, and cleaning up some
> sort-of-related problems in the syncrep parser/lexer.

Thank you for pushing this (with improvements) and improvements
of synchronous_standby_names. I agree to the discussion that
standby names should have restriction not to break possible
extension to be happen near future.

> There remains a point that I'm not very happy about, which is the code
> in check_synchronous_standby_names to emit a WARNING if the num_sync
> setting is too large.  That's a pretty bad compromise: we should either
> decide that the case is legal or that it is not.  If it's legal, people
> who are correctly using the case will not thank us for logging a WARNING
> every single time the postmaster gets a SIGHUP (and those who aren't using
> it correctly will have their systems freezing up, warning or no warning).
> If it's not legal, we should make it an error not a warning.

This specification makes the code a bit complex and makes the
document a bit less understandable. It seems to me somewhat
suspicious that allowing duplcate (potentially synchronous)
walrecivers is so useful as to justify such disadvantages.

In spite of this, my inclination is also the same as the
following:p rather than making the behavior consistent and clear.

> My inclination is to just rip out the warning.

Is there anyone object to removing the warining?

> But I wonder whether the
> desire to have one doesn't imply that the semantics are poorly chosen
> and should be revisited.

We already have abandoned a bit of backward compatibility in this
feature.

regards,

-- 
Kyotaro Horiguchi
NTT Open Source Software Center


-- 
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] pgindent

[Bruce Momjian]

On Wed, Apr 27, 2016 at 11:38:57PM -0400, Robert Haas wrote:
> > On what grounds do you claim the buildfarm result is unstable?
> > I've been using that for a long time and it works fine.  Moreover,
> > ignoring that data is a bad idea because it reflects platform-specific
> > variations in the set of typedefs that are known.  If you build a
> > typedefs list based only on what works on your machine, it likely
> > won't work for other people.
> 
> /me shrugs
> 
> Well, let's get the list, then, and compare it to what's in the file
> now.  How do we do that exactly?

The URL is in the file src/tools/pgindent/README:

  5) Download the typedef file from the buildfarm:

wget -O src/tools/pgindent/typedefs.list 
http://buildfarm.postgresql.org/cgi-bin/typedefs.pl

   (see http://www.pgbuildfarm.org/cgi-bin/typedefs.pl?show_list for a full 
list of typedefs,
also 
http://adpgtech.blogspot.com/2015/05/running-pgindent-on-non-core-code-or.html)

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


-- 
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] Sync timezone code with upstream release tzcode2016c

[Bruce Momjian]

On Sun, Mar 27, 2016 at 05:14:44PM -0400, Tom Lane wrote:
> Well, that was just about as tedious as I feared it might be, but
> attached is a patch for $SUBJECT.  We should apply this, and
> probably eventually back-patch it, but it'd be wise to let it
> age awhile in HEAD first.  Is anyone interested in reviewing it,
> or shall I just push it and see what the buildfarm thinks?
> 
> A note about comparing this to upstream: I found that the best
> way to do that was to run the IANA source files through a sed
> filter like this:
> 
> sed -r \
>   -e 's/^([   ]*)\*\*([   ])/\1 *\2/' \
>   -e 's/^([   ]*)\*\*$/\1 */' \
>   -e 's|^\*/| */|' \
>   -e 's/register //g' \
>   -e 's/int_fast32_t/int32/g' \
>   -e 's/int_fast64_t/int64/g' \
>   -e 's/struct tm\b/struct pg_tm/g' \
>   -e 's/\btime_t\b/pg_time_t/g' \
> 
> and then pgindent them.  (If you pgindent without this, it'll make
> a hash of their preferred block-comment format with double **'s.
> As long as I had to do that, I figured I could make the filter
> deal with substituting typedef names and getting rid of their
> overenthusiasm for "register".)

Is this documented for use next time?

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


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

Re: [HACKERS] Use %u to print user mapping's umid and userid

[Etsuro Fujita]

On 2016/03/14 17:56, Ashutosh Bapat wrote:

On Mon, Mar 14, 2016 at 1:29 PM, Etsuro Fujita
mailto:fujita.ets...@lab.ntt.co.jp>> wrote:



 /*
  * Build the fdw_private list that will be available to the
executor.
  * Items in the list must match order in enum FdwScanPrivateIndex.
  */
 fdw_private = list_make4(makeString(sql.data),
  retrieved_attrs,
  makeInteger(fpinfo->fetch_size),
  makeInteger(foreignrel->umid));

I don't think it's correct to use makeInteger for the foreignrel's umid.



As long as we are using makeInteger() and inVal() pair to set and
extract the values, it should be fine.


Yeah, but my concern about this is eg, print plan if debugging (ie, 
debug_print_plan=on); the umid OID will be printed with the %ld 
specifier, so in some platform, the OID might be printed wrongly.  Maybe 
I'm missing something, though.


Sorry for the long delay.

Best regards,
Etsuro Fujita




--
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] amroutine->amsupport from numeric to defined constants

[Michael Paquier]

On Tue, Apr 26, 2016 at 10:17 PM, Nikolay Shaplov
 wrote:
> While working with postgres code, I found that for gist index number of
> amsupport procs are defined two times. First in src/include/access/gist.h
>
> #define GISTNProcs  9
>
> and second in src/backend/access/gist/gist.c
>
> amroutine->amsupport = 9;
>
> I think this number should be specified only once, in .h file.

Yep, that sounds true.

> So I would offer a patch for all access methods. That changes amsupport and
> amstrategies from numbers to defined constants. (I've added two of them where
> they were missing)

It may be a good idea to make something similar for contrib/bloom if >
0 values are defined for amstrategies or amsupport for consistency.
-- 
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] Unused macros in contrib code

[Daniel Gustafsson]

While doing some archaeology I came across a couple of macros in pgstattuple
and pageinspect contrib code which seems to have been unused for some time.
CHECK_PAGE_OFFSET_RANGE is unused in both modules and commit 6405842 which
introduced contrib/pageinspect moved the code making CHECK_RELATION_BLOCK_RANGE
in pgstattuple unused.

For cleanliness sake it seems reasonable to remove these, patch attached which
passes make check (regular and in contrib).

cheers ./daniel



contrib_macros.diff
Description: Binary data

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

Re: [HACKERS] GIN data corruption bug(s) in 9.6devel

[Teodor Sigaev]

The more I think about it, the more I think gin is just an innocent
bystander, for which I just happen to have a particularly demanding
test.  I think something about snapshots and wrap-around may be
broken.


After 10 hours of running I've got
1587  XX000 2016-04-28 05:57:09.964 MSK:ERROR:  unexpected chunk number 0 
(expected 1) for toast value 10192986 in pg_toast_16424
1587  XX000 2016-04-28 05:57:09.964 MSK:CONTEXT:  automatic analyze of table 
"teodor.public.foo"
1587  0 2016-04-28 05:57:09.974 MSK:LOG:  JJ transaction ID wrap limit is 
2147484514, limited by database with OID 16384



I've pushed v5 of gin-alone-cleanup patch, many thanks to all participants.
Will work on backpatch

--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/


--
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] amroutine->amsupport from numeric to defined constants

[Teodor Sigaev]

I think this number should be specified only once, in .h file.

Yep, that sounds true.
It may be a good idea to make something similar for contrib/bloom if >
0 values are defined for amstrategies or amsupport for consistency.


Thank you, pushed.

--
Teodor Sigaev   E-mail: teo...@sigaev.ru
   WWW: http://www.sigaev.ru/


--
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] pgindent

[Robert Haas]

On Thu, Apr 28, 2016 at 7:39 AM, Bruce Momjian  wrote:
> On Wed, Apr 27, 2016 at 11:38:57PM -0400, Robert Haas wrote:
>> > On what grounds do you claim the buildfarm result is unstable?
>> > I've been using that for a long time and it works fine.  Moreover,
>> > ignoring that data is a bad idea because it reflects platform-specific
>> > variations in the set of typedefs that are known.  If you build a
>> > typedefs list based only on what works on your machine, it likely
>> > won't work for other people.
>>
>> /me shrugs
>>
>> Well, let's get the list, then, and compare it to what's in the file
>> now.  How do we do that exactly?
>
> The URL is in the file src/tools/pgindent/README:
>
>   5) Download the typedef file from the buildfarm:
>
> wget -O src/tools/pgindent/typedefs.list 
> http://buildfarm.postgresql.org/cgi-bin/typedefs.pl
>
>(see http://www.pgbuildfarm.org/cgi-bin/typedefs.pl?show_list for a full 
> list of typedefs,
> also 
> http://adpgtech.blogspot.com/2015/05/running-pgindent-on-non-core-code-or.html)

I compared the result of running pgindent with the typedefs.list file
as updated by me manually with the result of running pgindent using
the buildfarm list and ... the buildfarm list is better.  Shows what I
know.  Should we go ahead and commit the current version of that file
as src/tools/pgindent/typedefs.list, then?

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL 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] Rename max_parallel_degree?

[Robert Haas]

On Wed, Apr 27, 2016 at 1:05 PM, Daniel Verite  wrote:
> Robert Haas wrote:
>> Of course, we could make this value 1-based rather than 0-based, as
>> Peter Geoghegan suggested a while back.  But as I think I said at the
>> time, I think that's more misleading than helpful.  The leader
>> participates in the parallel plan, but typically does far less of the
>> work beneath the Gather node than the other nodes involved in the
>> query, often almost none.  In short, the leader is special.
>> Pretending that it's just another process involved in the parallel
>> group isn't doing anyone a favor.
>
> FWIW, that's not how it looks from the outside (top or vmstat).
> I'm ignorant about how parallel tasks are assigned in the planner,
> but when trying various values for max_parallel_degree and running
> simple aggregates on large tables on a single 4 core CPU doing
> nothing else, I'm only ever seeing max_parallel_degree+1 processes
> indiscriminately at work, often in the same state (R running or
> D waiting for disk).

Right, but they're probably not doing the SAME work.  You can look at
EXPLAIN (ANALYZE, VERBOSE, BUFFERS) to see.  Of course, all the work
above the Gather node is being done by the leader, but the stuff below
the Gather node often has a bit of participation from the leader, but
is mostly the workers.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL 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] Sync timezone code with upstream release tzcode2016c

[Tom Lane]

Bruce Momjian  writes:
> On Sun, Mar 27, 2016 at 05:14:44PM -0400, Tom Lane wrote:
>> A note about comparing this to upstream: I found that the best
>> way to do that was to run the IANA source files through a sed
>> filter like this: ...

> Is this documented for use next time?

Yes, see the README.

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] pgindent

[Tom Lane]

Robert Haas  writes:
> I compared the result of running pgindent with the typedefs.list file
> as updated by me manually with the result of running pgindent using
> the buildfarm list and ... the buildfarm list is better.  Shows what I
> know.  Should we go ahead and commit the current version of that file
> as src/tools/pgindent/typedefs.list, then?

Yes, if it's what you plan to use for pgindent'ing.

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] [BUGS] Breakage with VACUUM ANALYSE + partitions

[Andres Freund]

On 2016-04-28 17:41:29 +0100, Thom Brown wrote:
> I've noticed another breakage, which I can reproduce consistently.
> 
> createdb pgbench
> pgbench -i -s 100 --unlogged-tables pgbench
> psql -f pgbench_partitions.sql pgbench
> vacuumdb -z pgbench
> createdb test
> 
> Which produces:
> 
> createdb: database creation failed: ERROR:  checkpoint request failed
> HINT:  Consult recent messages in the server log for details.

Interesting. Any chance you could verify if this actually related to
either the fix or the commit that was to blame for the previous issue?

Because

> user=thom,db=postgres,client=[local] STATEMENT:  CREATE DATABASE test;
> 2016-04-28 17:36:00 BST [18108]: [46-1] user=,db=,client= DEBUG:
>  performing replication slot checkpoint
> 2016-04-28 17:36:00 BST [18105]: [158-1] user=,db=,client= DEBUG:  server
> process (PID 18582) exited with exit code 0
> 2016-04-28 17:36:08 BST [18108]: [47-1] user=,db=,client= DEBUG:  could not
> fsync file "base/24581/24594.1" but retrying: No such file or directory
> 2016-04-28 17:36:08 BST [18108]: [48-1] user=,db=,client= ERROR:  could not
> fsync file "base/24581/24594.1": No such file or directory
> 2016-04-28 17:36:08 BST [18605]: [17-1]
> user=thom,db=postgres,client=[local] ERROR:  checkpoint request failed
> 2016-04-28 17:36:08 BST [18605]: [18-1]
> user=thom,db=postgres,client=[local] HINT:  Consult recent messages in the
> server log for details.
> 2016-04-28 17:36:08 BST [18605]: [19-1]

Doesn't really sound like it's necessarily related to the previous problem.

- Andres


-- 
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] Add jsonb_compact(...) for whitespace-free jsonb to text

[Ryan Pedela]

On Tue, Apr 26, 2016 at 10:49 AM, Stephen Frost  wrote:

> * Ryan Pedela (rped...@datalanche.com) wrote:
> > On Sun, Apr 24, 2016 at 4:02 PM, Sehrope Sarkuni 
> wrote:
> > > The default text representation of jsonb adds whitespace in between
> > > key/value pairs (after the colon ":") and after successive properties
> > > (after the comma ","):
>
> [...]
>
> > > It'd be nice to have a stable text representation of a jsonb value with
> > > minimal whitespace. The latter would also save a few bytes per record
> in
> > > text output formats, on the wire, and in backups (ex: COPY ... TO
> STDOUT).
> >
> > +1
> >
> > I cannot comment on the patch itself, but I welcome jsonb_compact() or
> some
> > way to get JSON with no inserted whitespace.
>
> As I mentioned to Sehrope on IRC, at least for my 2c, if you want a
> compact JSON format to reduce the amount of traffic over the wire or to
> do things with on the client side, we should probably come up with a
> binary format, rather than just hack out the whitespace.  It's not like
> representing numbers using ASCII characters is terribly efficient
> either.


Why build a Ferrari when a skateboard would suffice? Besides, that doesn't
help one of the most common cases for JSONB: REST APIs.

Now that PG fully supports JSON, a user can use PG to construct the JSON
payload of a REST API request. Then the web server would simply be a
pass-through for the JSON payload. I personally have this use case, it is
not hypothetical. However currently, a user must parse the JSON string from
PG and re-stringify it to minimize the whitespace. Given that HTTP is
text-based, removing all extraneous whitespace is the best way to compress
it, and on top of that you can do gzip compression. Unless you are
suggesting that the binary format is just a gzipped version of the
minimized text format, I don't see how a binary format helps at all in the
REST API case.

In addition, every JSON implementation I have ever seen fully minimizes
JSON by default. PG appears to deviate from standard practice for no
apparent reason.

Re: [HACKERS] Add jsonb_compact(...) for whitespace-free jsonb to text

[David G. Johnston]

On Thu, Apr 28, 2016 at 10:00 AM, Ryan Pedela 
wrote:

> On Tue, Apr 26, 2016 at 10:49 AM, Stephen Frost 
> wrote:
>
>> * Ryan Pedela (rped...@datalanche.com) wrote:
>> > On Sun, Apr 24, 2016 at 4:02 PM, Sehrope Sarkuni 
>> wrote:
>> > > The default text representation of jsonb adds whitespace in between
>> > > key/value pairs (after the colon ":") and after successive properties
>> > > (after the comma ","):
>>
>> [...]
>>
>> > > It'd be nice to have a stable text representation of a jsonb value
>> with
>> > > minimal whitespace. The latter would also save a few bytes per record
>> in
>> > > text output formats, on the wire, and in backups (ex: COPY ... TO
>> STDOUT).
>> >
>> > +1
>> >
>> > I cannot comment on the patch itself, but I welcome jsonb_compact() or
>> some
>> > way to get JSON with no inserted whitespace.
>>
>> As I mentioned to Sehrope on IRC, at least for my 2c, if you want a
>> compact JSON format to reduce the amount of traffic over the wire or to
>> do things with on the client side, we should probably come up with a
>> binary format, rather than just hack out the whitespace.  It's not like
>> representing numbers using ASCII characters is terribly efficient
>> either.
>
>
> Why build a Ferrari when a skateboard would suffice? Besides, that doesn't
> help one of the most common cases for JSONB: REST APIs.
>

​I'm agreeing with this sentiment.  This isn't an either-or situation so
argue the white-space removal on its own merits.  The fact that we might
implement a binary representation in the future doesn't, for me, influence
whether we make this white-space change now.
​


>
> Now that PG fully supports JSON, a user can use PG to construct the JSON
> payload of a REST API request. Then the web server would simply be a
> pass-through for the JSON payload. I personally have this use case, it is
> not hypothetical.
>
However currently, a user must parse the JSON string from PG and
> re-stringify it to minimize the whitespace. Given that HTTP is text-based,
> removing all extraneous whitespace is the best way to compress it, and on
> top of that you can do gzip compression.
>

Can you clarify what you mean by "and on top of that you can do gzip
compression"?​​

Unless you are suggesting that the binary format is just a gzipped version
> of the minimized text format, I don't see how a binary format helps at all
> in the REST API case.
>
>
No, I'm pretty sure you still end up with uncompressed text in the
application layer.​


> In addition, every JSON implementation I have ever seen fully minimizes
> JSON by default. PG appears to deviate from standard practice for no
> apparent reason.
>

Sorry to nit-pick but that's called convention - the standard is likely
silent on this point.  And its not like this was done in a vacuum - why is
this only coming up now and not, say, during the beta?  Is it surprising
that this seemingly easy-to-overlook dynamic was not explicitly addressed
by the author and reviewer of the patch, especially when implementation of
said feature consisted of a lot more things of greater import and impact?

David J.

Re: [HACKERS] Add jsonb_compact(...) for whitespace-free jsonb to text

[David Fetter]

On Wed, Apr 27, 2016 at 05:05:18PM -0400, Stephen Frost wrote:
> * Merlin Moncure (mmonc...@gmail.com) wrote:
> > On Tue, Apr 26, 2016 at 11:49 AM, Stephen Frost  wrote:
> > > As I mentioned to Sehrope on IRC, at least for my 2c, if you want a
> > > compact JSON format to reduce the amount of traffic over the wire or to
> > > do things with on the client side, we should probably come up with a
> > > binary format, rather than just hack out the whitespace.  It's not like
> > > representing numbers using ASCII characters is terribly efficient
> > > either.
> > 
> > -1
> > 
> > This will benefit pretty much nobody unless you are writing a hand
> > crafted C application that consumes and processes the data directly.
> 
> That's not accurate.  All that's needed is for the libraries which
> either wrap libpq or re-implement it to be updated to understand the
> format and then convert the data into whatever structure makes sense for
> the language (or library that the language includes for working with
> JSON data).
> 
> One of the unfortunate realities with JSON is that there isn't a
> terribly good binary representation, afaik.  As I understand it, BSON
> doesn't support all the JSON structures that we do; if it did, I'd
> suggest we provide a way to convert our structure into BSON.

How about MessagePack?
http://msgpack.org/index.html

Cheers,
David.
-- 
David Fetter  http://fetter.org/
Phone: +1 415 235 3778  AIM: dfetter666  Yahoo!: dfetter
Skype: davidfetter  XMPP: david.fet...@gmail.com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate


-- 
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] 9.6 and fsync=off

[Greg Stark]

On Wed, Apr 27, 2016 at 10:58 AM, Craig Ringer  wrote:
> Now, we can't rename fsync to disable_crash_safety=on or
> corrupt_my_database=on. But the comment needs changing.


Fwiw we've done similar things in the past. We can provide
backwards-compatibility support for "fsync" but make the setting
appear as "crash_safety" or whatever in pg_settings and in the default
postgres.conf. The only downside is that tools or scripts that
retrieve all the settings might break or miss that setting.

-- 
greg


-- 
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] 9.6 and fsync=off

[Simon Riggs]

On 27 April 2016 at 17:04, Craig Ringer  wrote:

> On 27 April 2016 at 21:44, Tom Lane  wrote:
>
>> Petr Jelinek  writes:
>> > +1 (Abhijit's wording with data loss changed to data corruption)
>>
>> I'd suggest something like
>>
>> #fsync = on # flush data to disk for crash
>> safety
>> # (turning this off can cause
>> # unrecoverable data corruption!)
>>
>>
> Looks good.
>
> The docs on fsync are already good, it's just a matter of making people
> think twice and actually look at them.
>

If fsync=off and you turn it on, does it fsync anything at that point?

Or does it mean only that future fsyncs will occur?

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] [BUGS] Breakage with VACUUM ANALYSE + partitions

[Fabien COELHO]

createdb: database creation failed: ERROR:  checkpoint request failed
HINT:  Consult recent messages in the server log for details.


Attached is a simpler/faster version, based on the previous script, I just 
added a CHECKPOINT after the EXPLAIN ANALYSE.


It fails on head with:

 HINT:  Consider increasing the configuration parameter "max_wal_size".
 ERROR:  canceling autovacuum task
 CONTEXT:  automatic vacuum of table "fabien.public.pgbench_accounts"
 ERROR:  canceling autovacuum task
 CONTEXT:  automatic analyze of table "fabien.public.pgbench_accounts_1"
 ERROR:  canceling autovacuum task
 CONTEXT:  automatic analyze of table "fabien.public.pgbench_accounts_2"
 ERROR:  could not fsync file "base/16384/16397.1": No such file or directory
 ERROR:  checkpoint request failed
 HINT:  Consult recent messages in the server log for details.
 STATEMENT:  CHECKPOINT;

I tried it on pg prior to the flushing patches and it did not fail, so it 
seems to be related somehow. Probably the fix is similar, just some 
conditions to check when dealing with a file which has been removed by a 
large DELETE.


--
Fabien.

xxx.sh
Description: Bourne shell script


xxx.sql
Description: application/sql

-- 
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] Add jsonb_compact(...) for whitespace-free jsonb to text

[Alvaro Herrera]

David G. Johnston wrote:
> On Thu, Apr 28, 2016 at 10:00 AM, Ryan Pedela 
> wrote:

> > In addition, every JSON implementation I have ever seen fully minimizes
> > JSON by default. PG appears to deviate from standard practice for no
> > apparent reason.
> 
> Sorry to nit-pick but that's called convention - the standard is likely
> silent on this point.  And its not like this was done in a vacuum - why is
> this only coming up now and not, say, during the beta?  Is it surprising
> that this seemingly easy-to-overlook dynamic was not explicitly addressed
> by the author and reviewer of the patch, especially when implementation of
> said feature consisted of a lot more things of greater import and impact?

Actually we did have someone come up with a patch to "normalize" how
JSON stuff was output, because our code seems to do it in three
different, inconsistent ways.  And our response was for them to get the
heck outta here, because we're so much in love with our current
practice that we don't need to refactor the three implementations into a
single one.

Personally I don't see any reason we need to care one bit about how the
irrelevant whitespace is laid out, in other words why shouldn't we just
strip them all out?  Surely there's no backwards compatibility argument
there.  If somebody wants to see a nicely laid out structure they can
use the prettification function.

-- 
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


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

Re: [HACKERS] 9.6 and fsync=off

[David G. Johnston]

On Thursday, April 28, 2016, Simon Riggs  wrote:

> On 27 April 2016 at 17:04, Craig Ringer  > wrote:
>
>> On 27 April 2016 at 21:44, Tom Lane > > wrote:
>>
>>> Petr Jelinek >> > writes:
>>> > +1 (Abhijit's wording with data loss changed to data corruption)
>>>
>>> I'd suggest something like
>>>
>>> #fsync = on # flush data to disk for crash
>>> safety
>>> # (turning this off can cause
>>> # unrecoverable data corruption!)
>>>
>>>
>> Looks good.
>>
>> The docs on fsync are already good, it's just a matter of making people
>> think twice and actually look at them.
>>
>
> If fsync=off and you turn it on, does it fsync anything at that point?
>
> Or does it mean only that future fsyncs will occur?
>
>
http://www.postgresql.org/docs/current/static/runtime-config-wal.html

4th paragraph in the fsync section.

David J.

Re: [HACKERS] 9.6 and fsync=off

[Simon Riggs]

On 28 April 2016 at 22:30, David G. Johnston 
wrote:

> On Thursday, April 28, 2016, Simon Riggs  wrote:
>
>> On 27 April 2016 at 17:04, Craig Ringer  wrote:
>>
>>> On 27 April 2016 at 21:44, Tom Lane  wrote:
>>>
 Petr Jelinek  writes:
 > +1 (Abhijit's wording with data loss changed to data corruption)

 I'd suggest something like

 #fsync = on # flush data to disk for crash
 safety
 # (turning this off can cause
 # unrecoverable data
 corruption!)


>>> Looks good.
>>>
>>> The docs on fsync are already good, it's just a matter of making people
>>> think twice and actually look at them.
>>>
>>
>> If fsync=off and you turn it on, does it fsync anything at that point?
>>
>> Or does it mean only that future fsyncs will occur?
>>
>>
> http://www.postgresql.org/docs/current/static/runtime-config-wal.html
>
> 4th paragraph in the fsync section.
>

Thanks. I've never touched that parameter!  But I could have read the docs.

-- 
Simon Riggshttp://www.2ndQuadrant.com/

PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Re: [HACKERS] 9.6 and fsync=off

[Andres Freund]

On 2016-04-28 21:32:37 +0200, Simon Riggs wrote:
> On 27 April 2016 at 17:04, Craig Ringer  wrote:
> 
> > On 27 April 2016 at 21:44, Tom Lane  wrote:
> >
> >> Petr Jelinek  writes:
> >> > +1 (Abhijit's wording with data loss changed to data corruption)
> >>
> >> I'd suggest something like
> >>
> >> #fsync = on # flush data to disk for crash
> >> safety
> >> # (turning this off can cause
> >> # unrecoverable data corruption!)
> >>
> >>
> > Looks good.
> >
> > The docs on fsync are already good, it's just a matter of making people
> > think twice and actually look at them.
> >
> 
> If fsync=off and you turn it on, does it fsync anything at that point?
> 
> Or does it mean only that future fsyncs will occur?

Abhijit had a patch implementing automatically running fsync whenever
reenabled IIRC. Abhijit?

Andres


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

[HACKERS] Is pg_control file crashsafe?

[Alex Ignatov]

Hello everyone!
We have some issue with truncated pg_control file on Windows after power 
failure.My questions is : 1) Is pg_control protected from say , power crash or 
partial write? 2) How PG update pg_control? By writing in it or writing in some 
temp file and after that rename it to pg_control to be atomic?3) Can PG have  
multiple pg_control copy to be more fault tolerant?
PS During some experiments we found that at present time there is no any method 
to do crash recovery with "restored" version of pg_control (based on some 
manipulations with pg_resetxlog ). Only by using pg_resetxlog and setting it 
parameters to values taken from wal file (pg_xlogdump)we can at least start PG 
and saw that PG state is at the moment of last check point. But we have no real 
confidence that PG is in consistent state(also docs on pg_resetxlogs told us 
about it too)

Alex IgnatovPostgres Professional: http://www.postgrespro.comRussian Postgres 
Company

Re: [HACKERS] Add jsonb_compact(...) for whitespace-free jsonb to text

[Sehrope Sarkuni]

On Wed, Apr 27, 2016 at 7:09 PM, David G. Johnston <
david.g.johns...@gmail.com> wrote:

> On Sun, Apr 24, 2016 at 3:02 PM, Sehrope Sarkuni 
> wrote:
>
>> Attached is a *very* work in progress patch that adds a
>> jsonb_compact(jsonb)::text function. It generates a text representation
>> without extra whitespace but does not yet try to enforce a stable order of
>> the properties within a jsonb value.
>>
>
> ​I think that having a jsonb_compact function that complements the
> existing jsonb_pretty function is a well scoped and acceptable​ feature.  I
> do not believe that it should also take on the role of canonicalization.
>
> I'd suggest that any discussions regarding stability of jsonb output be
> given its own thread.
>

I'm fine with removing the stability aspect. I think it's nice-to-have but
it definitely complicates things and has longer term consequences.


> That topic also seems separate from a discussion on how to implement a
> binary transport protocol for jsonb.
>

Defining a binary format for jsonb is definitely out of scope.


> ​Lastly would be whether we change our default text representation so that
> users utilizing COPY get the added benefit of a maximally minimized text
> representation.
>

I see this applying to both COPY and the text format on the wire. The
latter has the added benefit that it works with existing clients without
any driver changes.

Outside of being a bit more pleasant in psql, I don't see a point in the
added whitespace for jsonb::text. Even in psql it only helps with small
fields as anything big isn't really legible without indenting it via
jsonb_pretty(...).


> As an aside on the last topic, has there ever been considered to have a
> way to specify a serialization function to use for a given type (or maybe
> column) specified in a copy command?
>
> Something like: COPY [...] WITH (jsonb USING jsonb_compact)
>
> I'm thinking this would hard and undesirable given the role copy plays and
> limited intelligence that it has in order to maximize its efficiency in
> fulfilling its role.
>
> Backups get compressed already so bandwidth seems the bigger goal there.
> Otherwise I'd say that we lack any kind of overwhelming evidence that
> making such a change would be warranted.
>
> While these are definitely related topics it doesn't seem like any are
> pre-requisites for the others.  I think this thread is going to become hard
> to follow and trail off it continues to try and address all of these topics
> randomly as people see fit to reply.  And it will quickly become hard for
> anyone to jump in and understand the topics at hand.
>

That's a really cool idea but agree it's way out of scope for this.

I had a related idea, maybe something similar could be done for psql to set
a jsonb output format. That way you could automatically prettify jsonb
fields client side.

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/

[HACKERS] [sqlsmith] Crash in apply_projection_to_path

[Andreas Seltenreich]

Hi,

the following query against the regression database crashes master as of
23b09e15.

select 1 from depth0 inner join depth1 on (depth0.c = depth1.c)
 where depth0.c @@ depth1.c limit 1;

regards,
Andreas

Program terminated with signal SIGSEGV, Segmentation fault.
#0  create_projection_path (root=root@entry=0x6918e60,
rel=0x7f7f7f7f7f7f7f7f, subpath=0x69d6de8, target=target@entry=0x69d7428)
at pathnode.c:2160
2160pathnode->path.parallel_safe = rel->consider_parallel &&
#0  create_projection_path (root=root@entry=0x6918e60, rel=0x7f7f7f7f7f7f7f7f, 
subpath=0x69d6de8, target=target@entry=0x69d7428) at pathnode.c:2160
#1  0x0067841e in apply_projection_to_path (root=0x6918e60, 
rel=0x69d5e18, path=0x69d6ee0, target=0x69d7428) at pathnode.c:2251
#2  0x0065ff20 in grouping_planner (root=0x22e1850, 
root@entry=0x6918e60, inheritance_update=56 '8', inheritance_update@entry=0 
'\000', tuple_fraction=2615.2579411764709, tuple_fraction@entry=0) at 
planner.c:1737
#3  0x00661f44 in subquery_planner (glob=glob@entry=0x6918dc8, 
parse=parse@entry=0x235e3b0, parent_root=parent_root@entry=0x0, 
hasRecursion=hasRecursion@entry=0 '\000', 
tuple_fraction=tuple_fraction@entry=0) at planner.c:758
#4  0x00662e2b in standard_planner (parse=0x235e3b0, cursorOptions=256, 
boundParams=0x0) at planner.c:307
#5  0x006f40ed in pg_plan_query (querytree=0x235e3b0, 
cursorOptions=256, boundParams=0x0) at postgres.c:798
#6  0x006f41e4 in pg_plan_queries (querytrees=, 
cursorOptions=256, boundParams=0x0) at postgres.c:857
#7  0x006f5c93 in exec_simple_query (query_string=) at 
postgres.c:1022


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

Re: [HACKERS] pg_xlog -> pg_xjournal?

[Bruce Momjian]

Are we going to rename pg_xlog or pg_clog for 9.6?

---

On Sun, May 31, 2015 at 10:44:54PM +0200, Joel Jacobson wrote:
> On Sun, May 31, 2015 at 7:46 PM, Tom Lane  wrote:
> > Hm.  I think the impact on third-party backup tools would be rather bad,
> > but there's a simple modification of the idea that might fix that:
> > just always create pg_xlog as a symlink to pg_xjournal during initdb.
> > Anybody who blindly removes pg_xlog won't have done anything
> > irreversible.  We could deprecate pg_xlog and stop creating the symlink
> > after a few releases, once third-party tools have had a reasonable
> > amount of time to adjust.
> 
> I like the solution. Simple and effective.
> +1
> 
> > In the end though, this is a lot of thrashing for a problem that
> > only comes up rarely ...
> 
> It happens often enough for the problem to be the first mentioned
> use-case of pg_resetxlog at Stack Overflow:
> 
> "pg_resetxlog is a tool of last resort for getting your database
> running again after:
> 1. You deleted files you shouldn't have from pg_xlog;"
> 
> (http://stackoverflow.com/questions/12897429/what-does-pg-resetxlog-do-and-how-does-it-work)
> 
> Preventing failure in the case of faults is of course one of the
> primary objectives of any database.
> 
> 
> -- 
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


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

Re: [HACKERS] pg_xlog -> pg_xjournal?

[Andres Freund]

On 2016-04-28 19:23:26 -0400, Bruce Momjian wrote:
> 
> Are we going to rename pg_xlog or pg_clog for 9.6?

If we do so, we should do it at a good bit earlier in the cycle imo.

Andres


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

Re: [HACKERS] pg_xlog -> pg_xjournal?

[Bruce Momjian]

On Thu, Apr 28, 2016 at 04:30:39PM -0700, Andres Freund wrote:
> On 2016-04-28 19:23:26 -0400, Bruce Momjian wrote:
> > 
> > Are we going to rename pg_xlog or pg_clog for 9.6?
> 
> If we do so, we should do it at a good bit earlier in the cycle imo.

Well, we talked about it in May of 2015, but didn't do anything.  Was
there some big WAL user API change in this release that encouraged the
idea of the change?

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


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

Re: [HACKERS] pg_upgrade 9.6->9.6: column "amtype" does not exist

[Bruce Momjian]

On Fri, Apr  1, 2016 at 05:55:58PM -0400, Tom Lane wrote:
> Alvaro Herrera  writes:
> > The reason for the failure is that pg_dump knows that 9.6 installations
> > have the amtype column -- but on your older devel 9.6 install, it
> > doesn't exist.  To fix it we would have to compare catalog versions in
> > pg_dump rather than major version, but we don't that anywhere.  Anyway
> > many upgrades could fail depending on other patches because of the same
> > reason.  I don't think we need to fix this.
> 
> We've never expected pg_dump to be able to handle intermediate development
> versions.  It's enough of a maintenance burden that it knows how to dump
> from all previous stable branches.

Pg_upgrade does handle catalog version changes.  I don't think we ever
anticipated catalog changes that would throw errors in pg_dump, which
pg_upgrade relies on.  I am saying that someday we might need pg_dump to
track catalog versions.  However, I am not even sure how pg_dump would
check the catalog version.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


-- 
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 in apply_projection_to_path

[Tom Lane]

Andreas Seltenreich  writes:
> the following query against the regression database crashes master as of
> 23b09e15.

> select 1 from depth0 inner join depth1 on (depth0.c = depth1.c)
>  where depth0.c @@ depth1.c limit 1;

What's going on here is that add_partial_path is recycling a now-dominated
partial path without regard for the fact that there's already a GatherPath
pointing at that old partial path.  Later use of the GatherPath tries to
make use of its now-dangling subpath pointer.

I'd be inclined to think that it's silly to build GatherPaths in advance
of having finalized the list of partial paths for a rel.

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] pg_xlog -> pg_xjournal?

[Tom Lane]

Bruce Momjian  writes:
> Are we going to rename pg_xlog or pg_clog for 9.6?

NO.  We don't even have a patch for this, much less one that's been
through any review.  This suggestion is at least two months too late.

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] Postgres 9.6 scariest patch tournament

[Noah Misch]

On Mon, Apr 18, 2016 at 03:37:21PM -0300, Alvaro Herrera wrote:
> The PostgreSQL Project needs you!
> 
> The Release Management Team would like your input regarding the patch or
> patches which, in your opinion, are the most likely sources of major
> bugs or instabilities in PostgreSQL 9.6.
> 
> Please submit your answers before May 1st using this form:
> https://docs.google.com/forms/d/1xNNqhXC116wCMnomqGz9RQ7OuVwZqAcEre7iiU6pT20/viewform

Reminder: the survey closes this weekend.  Identify those scary patches!


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

Re: [HACKERS] pg_xlog -> pg_xjournal?

[Bruce Momjian]

On Thu, Apr 28, 2016 at 10:07:40PM -0400, Tom Lane wrote:
> Bruce Momjian  writes:
> > Are we going to rename pg_xlog or pg_clog for 9.6?
> 
> NO.  We don't even have a patch for this, much less one that's been
> through any review.  This suggestion is at least two months too late.

My larger question was, was 9.6 an ideal time to do this, and if so, why
did this issue not get done.  If 9.6 wasn't in some way ideal, we can do
it in 9.7.

-- 
  Bruce Momjian  http://momjian.us
  EnterpriseDB http://enterprisedb.com

+ As you are, so once was I. As I am, so you will be. +
+ Ancient Roman grave inscription +


-- 
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 in apply_projection_to_path

[Robert Haas]

On Thu, Apr 28, 2016 at 10:06 PM, Tom Lane  wrote:
> Andreas Seltenreich  writes:
>> the following query against the regression database crashes master as of
>> 23b09e15.
>
>> select 1 from depth0 inner join depth1 on (depth0.c = depth1.c)
>>  where depth0.c @@ depth1.c limit 1;
>
> What's going on here is that add_partial_path is recycling a now-dominated
> partial path without regard for the fact that there's already a GatherPath
> pointing at that old partial path.  Later use of the GatherPath tries to
> make use of its now-dangling subpath pointer.
>
> I'd be inclined to think that it's silly to build GatherPaths in advance
> of having finalized the list of partial paths for a rel.

Uh ... yeah, that shouldn't be happening.  I obviously screwed something up.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL 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] pg_xlog -> pg_xjournal?

[Craig Ringer]

On 29 April 2016 at 10:12, Bruce Momjian  wrote:


>
> My larger question was, was 9.6 an ideal time to do this, and if so, why
> did this issue not get done.  If 9.6 wasn't in some way ideal, we can do
> it in 9.7.
>
>
Doing it at the very beginning of the release cycle seems like a good idea.

I just helped another person yesterday who deleted their pg_xlog.


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

[HACKERS] UNION ALL - Var attno

[sri harsha]

Hi,

   Assume the following query ,

(SELECT a * 1 , b FROM TABLE_1) UNION ALL (SELECT a *1 , b FROM TABLE_2);

In this query , attribute number of the VARs are 141 and 2 respectively !!
What is the reason for this ??

I am trying to implement a FDW , so i need attribute numbers to fetch the
respective columns from my data store . Due to this change in attribute
numbers in the case of UNION ALL , this query doesn't provide the necessary
output .

But in the plan its given as 1 and 2 respectively . So is there a mapping
which PG maintains to convert from 141 to the original attribute number ??


Thanks,
Harsha

Re: [HACKERS] VS 2015 support in src/tools/msvc

[Andrew Dunstan]

On 04/25/2016 03:11 AM, Michael Paquier wrote:

On Mon, Apr 25, 2016 at 4:29 AM, Christian Ullrich  wrote:

Andrew wrote:

OK, here's my final version of the patch, which I will apply in 24 hours

or so unless there is an objection.

Thanks Andrew for the updated patch!


This one doesn't matter, but just for perfection's sake:

+#if (_MSC_VER >= 1900)
+   uint32  cp;
+   WCHAR   wctype[80];
+
+   memset(wctype, 0, 80 * sizeof(WCHAR));
+   MultiByteToWideChar(CP_ACP, 0, ctype, -1, wctype, 80);

The maximum length is documented as 85 characters, also:

:
'Note   Your application must use the constant [LOCALE_NAME_MAX_LENGTH] for
the maximum locale name length, instead of hard-coding the value "85".'

Just an addition on top of the comments of Christian..

+#pragma warning(push)
+#pragma warning(disable : 4091)
  #include 
+#pragma warning(pop)
It seems to me that we had better protect those pragmas with _MSC_VER

= 1900. The crash dump facility could be used by MinGW as well, no?




I think we're being overcautious here.  But to keep people happy I have 
protected it with "#ifdef _MSC_VER".


latest patch attached. I have also cleaned up the docs some, and removed 
references to the now redundant msysGit.


cheers

andrew


diff --git a/doc/src/sgml/install-windows.sgml b/doc/src/sgml/install-windows.sgml
index 74265fc..4e92cc9 100644
--- a/doc/src/sgml/install-windows.sgml
+++ b/doc/src/sgml/install-windows.sgml
@@ -19,10 +19,10 @@
  
   There are several different ways of building PostgreSQL on
   Windows. The simplest way to build with
-  Microsoft tools is to install Visual Studio Express 2013
+  Microsoft tools is to install Visual Studio Express 2015
   for Windows Desktop and use the included
   compiler. It is also possible to build with the full
-  Microsoft Visual C++ 2005 to 2013.
+  Microsoft Visual C++ 2005 to 2015.
   In some cases that requires the installation of the
   Windows SDK in addition to the compiler.
  
@@ -77,19 +77,26 @@
   Visual Studio Express or some versions of the
   Microsoft Windows SDK. If you do not already have a
   Visual Studio environment set up, the easiest
-  ways are to use the compilers from Visual Studio Express 2013
+  ways are to use the compilers from Visual Studio Express 2015
   for Windows Desktop or those in the Windows SDK
   7.1, which are both free downloads from Microsoft.
  
 
  
-  PostgreSQL is known to support compilation using the compilers shipped with
+  Both 32-bit and 64-bit builds are possible with the Microsoft Compiler suite.
+  32-bit PostgreSQL buils are possible with
   Visual Studio 2005 to
-  Visual Studio 2013 (including Express editions),
+  Visual Studio 2015 (including Express editions),
   as well as standalone Windows SDK releases 6.0 to 7.1.
-  64-bit PostgreSQL builds are only supported with
+  64-bit PostgreSQL builds are supported with
   Microsoft Windows SDK version 6.0a to 7.1 or
-  Visual Studio 2008 and above.
+  Visual Studio 2008 and above. Compilation
+  is supported down to Windows XP and
+  Windows Server 2003 when building with
+  Visual Studio 2005 to
+  Visual Studio 2013. Building with
+  Visual Studio 2015 is supported down to
+  Windows Vista and Windows Server 2008.
  
 
  
@@ -210,9 +217,7 @@ $ENV{PATH}=$ENV{PATH} . ';c:\some\where\bison\bin';
   Both Bison and Flex
   are included in the msys tool suite, available
   from http://www.mingw.org/wiki/MSYS";> as part of the
-  MinGW compiler suite. You can also get
-  msys as part of
-  msysGit from http://git-scm.com/";>.
+  MinGW compiler suite.
  
 
  
@@ -221,9 +226,7 @@ $ENV{PATH}=$ENV{PATH} . ';c:\some\where\bison\bin';
   PATH environment variable in buildenv.pl unless
   they are already in PATH. In the case of MinGW, the directory is the
   \msys\1.0\bin subdirectory of your MinGW
-  installation directory. For msysGit, it's the bin
-  directory in your Git install directory. Do not add the MinGW compiler
-  tools themselves to PATH.
+  installation directory.
  
 
  
diff --git a/src/backend/port/win32/crashdump.c b/src/backend/port/win32/crashdump.c
index ec7c325..92e23cb 100644
--- a/src/backend/port/win32/crashdump.c
+++ b/src/backend/port/win32/crashdump.c
@@ -41,7 +41,21 @@
 #define WIN32_LEAN_AND_MEAN
 #include 
 #include 
+/*
+ * Some versions of the MS SDK contain "typedef enum { ... } ;" which the MS
+ * compiler quite sanely complains about. Well done, Microsoft.
+ * This pragma disables the warning just while we include the header.
+ * The pragma is known to work with all (as at the time of writing) supported
+ * versions of MSVC.
+ */
+#ifdef _MSC_VER
+#pragma warning(push)
+#pragma warning(disable : 4091)
+#endif
 #include 
+#ifdef _MSC_VER
+#pragma warning(pop)
+#endif
 
 /*
  * Much of the following code is based on CodeProject and MSDN examples,
dif

Re: [HACKERS] UNION ALL - Var attno

[Tom Lane]

sri harsha  writes:
>Assume the following query ,
> (SELECT a * 1 , b FROM TABLE_1) UNION ALL (SELECT a *1 , b FROM TABLE_2);

> In this query , attribute number of the VARs are 141 and 2 respectively !!

I doubt it.

Maybe you're looking at something that's not a Var, possibly an OpExpr,
but assuming it's a Var?  The fact that 141 is the pg_proc OID of int4mul
lends considerable weight to this suspicion ...

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] VS 2015 support in src/tools/msvc

[Tom Lane]

Andrew Dunstan  writes:
> latest patch attached. I have also cleaned up the docs some, and removed 
> references to the now redundant msysGit.

Please don't do stuff like this:

@@ -232,6 +265,10 @@ win32_langinfo(const char *ctype)
if (r != NULL)
sprintf(r, "CP%s", codepage);
}
+
+#if (_MSC_VER >= 1900)
+   }
+#endif
 #endif
 
return r;

I'm not very sure what pgindent will do with conditionally-included
indentation, but it's unlikely to be pleasing.

In this particular case, you could probably fix it by making the
other end of that be 

+   if (GetLocaleInfoEx(wctype,
+   LOCALE_IDEFAULTANSICODEPAGE | LOCALE_RETURN_NUMBER,
+   (LPWSTR) &cp, sizeof(cp) / sizeof(WCHAR)) > 0)
+   {
+   r = malloc(16); /* excess */
+   if (r != NULL)
+   sprintf(r, "CP%u", cp);
+   }
+   else
+#endif
+   {
+

and omitting the #if/#endif around the trailing }.

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] UNION ALL - Var attno

[sri harsha]

Its not an OpExpr . It is a VAR , got it from "reltargetlist" in base
relation ( RelOptInfo) . Can you shed some light on where the conversion
from 141 to "original" attribute number takes place ??


Regards,
Harsha

On Fri, Apr 29, 2016 at 10:03 AM, Tom Lane  wrote:

> sri harsha  writes:
> >Assume the following query ,
> > (SELECT a * 1 , b FROM TABLE_1) UNION ALL (SELECT a *1 , b FROM TABLE_2);
>
> > In this query , attribute number of the VARs are 141 and 2 respectively
> !!
>
> I doubt it.
>
> Maybe you're looking at something that's not a Var, possibly an OpExpr,
> but assuming it's a Var?  The fact that 141 is the pg_proc OID of int4mul
> lends considerable weight to this suspicion ...
>
> regards, tom lane
>

[HACKERS] [sqlsmith] Failed assertion in BecomeLockGroupLeader

[Andreas Seltenreich]

Hi,

tonight's sqlsmith run yielded another core dump:

TRAP: FailedAssertion("!(MyProc->lockGroupLeader == ((void *)0))", File: 
"proc.c", Line: 1787)

I couldn't identifiy a query for it though: debug_query_string is empty.
Additionally, the offending query was not reported in the error context
as it typically is for non-parallel executor crashes.

regards,
Andreas

GNU gdb (Debian 7.7.1+dfsg-5) 7.7.1
Core was generated by `postgres: bgworker: parallel worker for PID 4706 
'.
Program terminated with signal SIGABRT, Aborted.
#0  0x7ff1bda16067 in __GI_raise (sig=sig@entry=6)
at ../nptl/sysdeps/unix/sysv/linux/raise.c:56
56  ../nptl/sysdeps/unix/sysv/linux/raise.c: Datei oder Verzeichnis nicht 
gefunden.
#0  0x7ff1bda16067 in __GI_raise (sig=sig@entry=6) at 
../nptl/sysdeps/unix/sysv/linux/raise.c:56
#1  0x7ff1bda17448 in __GI_abort () at abort.c:89
#2  0x007eaa11 in ExceptionalCondition 
(conditionName=conditionName@entry=0x988318 "!(MyProc->lockGroupLeader == 
((void *)0))", errorType=errorType@entry=0x82a45d "FailedAssertion", 
fileName=fileName@entry=0x8760e5 "proc.c", lineNumber=lineNumber@entry=1787) at 
assert.c:54
#3  0x006e3e7b in BecomeLockGroupLeader () at proc.c:1787
#4  0x004e6a59 in LaunchParallelWorkers (pcxt=pcxt@entry=0x1db05c8) at 
parallel.c:437
#5  0x005ef2d7 in ExecGather (node=node@entry=0x1d9d0b8) at 
nodeGather.c:168
#6  0x005dd788 in ExecProcNode (node=node@entry=0x1d9d0b8) at 
execProcnode.c:515
#7  0x005d999f in ExecutePlan (dest=0x1d7d310, direction=, numberTuples=0, sendTuples=, operation=CMD_SELECT, 
use_parallel_mode=, planstate=0x1d9d0b8, estate=0x1d9c858) at 
execMain.c:1567
#8  standard_ExecutorRun (queryDesc=0x1db0080, direction=, 
count=0) at execMain.c:338
#9  0x005dcb3f in ParallelQueryMain (seg=, 
toc=0x7ff1be507000) at execParallel.c:716
#10 0x004e608b in ParallelWorkerMain (main_arg=) at 
parallel.c:1033
#11 0x00683a42 in StartBackgroundWorker () at bgworker.c:726
#12 0x0068eb82 in do_start_bgworker (rw=0x1d24ec0) at postmaster.c:5531
#13 maybe_start_bgworker () at postmaster.c:5706
#14 0x0046c993 in ServerLoop () at postmaster.c:1762
#15 0x006909fe in PostmasterMain (argc=argc@entry=3, 
argv=argv@entry=0x1cfa560) at postmaster.c:1298
#16 0x0046d5ed in main (argc=3, argv=0x1cfa560) at main.c:228
(gdb) bt full
#3  0x006e3e7b in BecomeLockGroupLeader () at proc.c:1787
leader_lwlock = 
#4  0x004e6a59 in LaunchParallelWorkers (pcxt=pcxt@entry=0x1db05c8) at 
parallel.c:437
oldcontext = 0x1d9ced0
worker = {
  bgw_name = 
"\220\a\333\001\000\000\000\000\370\340L\276\361\177\000\000\370\373\025\264\361\177\000\000P\a\333\001\000\000\000\000X\310\331\001\000\000\000\000\004\000\000\000\000\000\000\000\310\005\333\001\000\000\000\000\233\222J\000\000\000\000",
 
  bgw_flags = 1, 
  bgw_start_time = BgWorkerStart_PostmasterStart, 
  bgw_restart_time = 1, 
  bgw_main = 0x0, 
  bgw_library_name = 
"\000\000\000\000\000\000\000\000\001\000\000\000\000\000\000\000u\222J\000\000\000\000\000\350\336\331\001\000\000\000\000\360\260a\000\000\000\000\000\200\315]\000\000\000\000\000\300\330\367\217\375\177\000\000h\205\333\001\000\000\000",
 
  bgw_function_name = 
"`\346\327\001\000\000\000\000\004\000\000\000\000\000\000\000\360\260a\000\000\000\000\000\200\315]\000\000\000\000\000\300\330\367\217\375\177\000\000h\317\331\001\000\000\000\000\210\325\327\001\000\000\000\000\004\000\000\000\000\000\000",
 
  bgw_main_arg = 6402288, 
  bgw_extra = 
"X\310\331\001\000\000\000\000\000\000\000\000\000\000\000\000\060\233\333\001\000\000\000\000\001\000\000\000\000\000\000\000\004\000\000\000\000\000\000\000l\321]\000\000\000\000\000\000\000\000\000\000\000\000\000H-\334\001\000\000\000\000h\317\331\001\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\000\274\341M\276\361\177\000\000\310\005\333\001\000\000\000\000\004\000\000\000\000\000\000\000\310\005\333\001\000\000\000\000\000\000\000\000\000\000\000",
 
  bgw_notify_pid = 4
}
i = 
any_registrations_failed = 0 '\000'
#5  0x005ef2d7 in ExecGather (node=node@entry=0x1d9d0b8) at 
nodeGather.c:168
pcxt = 0x1db05c8
estate = 
gather = 0x1d7d440
fslot = 0x1d9ced0
i = 
resultSlot = 
isDone = ExprSingleResult
econtext = 
#6  0x005dd788 in ExecProcNode (node=node@entry=0x1d9d0b8) at 
execProcnode.c:515
result = 
__func__ = "ExecProcNode"
#7  0x005d999f in ExecutePlan (dest=0x1d7d310, direction=, numberTuples=0, sendTuples=, operation=CMD_SELECT, 
use_parallel_mode=, planstate=0x1d9d0b8, estate=0x1d9c858) at 
execMain.c:1567
slot = 
current_tuple_count = 0
#8  standard_ExecutorRun (queryDesc=0x1db0080, direction=, 
count=0)

Re: [HACKERS] UNION ALL - Var attno

[Amit Langote]

On Fri, Apr 29, 2016 at 2:42 PM, sri harsha  wrote:
>
> Its not an OpExpr . It is a VAR , got it from "reltargetlist" in base
> relation ( RelOptInfo) . Can you shed some light on where the conversion
> from 141 to "original" attribute number takes place ??

As Tom said, you must be looking at an OPEXPR's opfuncid value.
Because that's what I see as being 141.

Thanks,
Amit


-- 
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] 9.6 and fsync=off

[Abhijit Menon-Sen]

At 2016-04-28 13:44:23 -0700, and...@anarazel.de wrote:
>
> Abhijit had a patch implementing automatically running fsync whenever
> reenabled IIRC. Abhijit?

The patch I had written is attached, and it's not quite the same thing.
Here's how I originally described it in response to a question from
Robert:

«In 20150115133245.gg5...@awork2.anarazel.de, Andres explained his
rationale as follows:

«What I am thinking of is that, currently, if you start the
server for initial loading with fsync=off, and then restart it,
you're open to data loss. So when the current config file
setting is changed from off to on, we should fsync the data
directory. Even if there was no crash restart.»

That's what I tried to implement.»

I remember there was some subsequent discussion about it being better to
issue fsync during a checkpoint when we see that its value has changed,
but if I did any work on it (which I have a vague memory of), I can't
find it now. Sorry.

Do you want a patch along those lines now, or is it too late?

-- Abhijit
>From 1768680b672bcb037446230323cabcf9960f7f9a Mon Sep 17 00:00:00 2001
From: Abhijit Menon-Sen 
Date: Fri, 1 May 2015 17:59:51 +0530
Subject: Recursively fsync PGDATA on the next restart after fsync was disabled

Even if we didn't crash, we want to fsync PGDATA on startup if we know
the server ran with fsync=off at some point since the previous restart.
Otherwise, starting the server with fsync=off for initial data loading
and then restarting it opens you to data loss on a power failure after
the restart.
---
 src/backend/access/transam/xlog.c   | 9 +++--
 src/bin/pg_controldata/pg_controldata.c | 2 ++
 src/include/catalog/pg_control.h| 8 +++-
 3 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index 084174d..af12992 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -4823,6 +4823,7 @@ BootStrapXLOG(void)
 	ControlFile->checkPoint = checkPoint.redo;
 	ControlFile->checkPointCopy = checkPoint;
 	ControlFile->unloggedLSN = 1;
+	ControlFile->fsync_disabled = false;
 
 	/* Set important parameter values for use when replaying WAL */
 	ControlFile->MaxConnections = MaxConnections;
@@ -5893,10 +5894,12 @@ StartupXLOG(void)
 	 */
 
 	if (enableFsync &&
-		(ControlFile->state != DB_SHUTDOWNED &&
-		 ControlFile->state != DB_SHUTDOWNED_IN_RECOVERY))
+		(ControlFile->fsync_disabled ||
+		 (ControlFile->state != DB_SHUTDOWNED &&
+		  ControlFile->state != DB_SHUTDOWNED_IN_RECOVERY)))
 	{
 		fsync_pgdata(data_directory);
+		ControlFile->fsync_disabled = false;
 	}
 
 	if (ControlFile->state == DB_SHUTDOWNED)
@@ -8272,6 +8275,8 @@ CreateCheckPoint(int flags)
 	/* crash recovery should always recover to the end of WAL */
 	ControlFile->minRecoveryPoint = InvalidXLogRecPtr;
 	ControlFile->minRecoveryPointTLI = 0;
+	if (!enableFsync)
+		ControlFile->fsync_disabled = true;
 
 	/*
 	 * Persist unloggedLSN value. It's reset on crash recovery, so this goes
diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c
index d8cfe5e..e99014f 100644
--- a/src/bin/pg_controldata/pg_controldata.c
+++ b/src/bin/pg_controldata/pg_controldata.c
@@ -290,6 +290,8 @@ main(int argc, char *argv[])
 		   (uint32) ControlFile.backupEndPoint);
 	printf(_("End-of-backup record required:%s\n"),
 		   ControlFile.backupEndRequired ? _("yes") : _("no"));
+	printf(_("Fsync disabled at runtime:%s\n"),
+		   ControlFile.fsync_disabled ? _("yes") : _("no"));
 	printf(_("Current wal_level setting:%s\n"),
 		   wal_level_str(ControlFile.wal_level));
 	printf(_("Current wal_log_hints setting:%s\n"),
diff --git a/src/include/catalog/pg_control.h b/src/include/catalog/pg_control.h
index 2e4c381..a71d73e 100644
--- a/src/include/catalog/pg_control.h
+++ b/src/include/catalog/pg_control.h
@@ -21,7 +21,7 @@
 
 
 /* Version identifier for this pg_control format */
-#define PG_CONTROL_VERSION	942
+#define PG_CONTROL_VERSION	943
 
 /*
  * Body of CheckPoint XLOG records.  This is declared here because we keep
@@ -182,6 +182,12 @@ typedef struct ControlFileData
 	bool		track_commit_timestamp;
 
 	/*
+	 * Indicates whether fsync was ever disabled since the last restart.
+	 * Tested and set at checkpoints, reset at startup.
+	 */
+	bool		fsync_disabled;
+
+	/*
 	 * This data is used to check for hardware-architecture compatibility of
 	 * the database and the backend executable.  We need not check endianness
 	 * explicitly, since the pg_control version will surely look wrong to a
-- 
1.9.1


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