Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

[Jim Nasby]

On 9/1/15 8:42 PM, Michael Paquier wrote:

test_factory is a jungle to me. Perhaps you could just extract a
self-contained test case? It does not matter if the file is long as
long as the problem can be easily reproduced.


Sorry, more info on what's happening here.

The idea behind test_factory is to allow you to register a command that 
creates and returns test data (IE: INSERT INTO table VALUES( DEFAULT, 
'my test data' ) RETURNING *). That insert statement is stored in a 
table (_tf._test_factory). When this dynamic statement is executed, the 
results will be stored in a specially named table (plpgsql variable 
c_data_table_name).


When you call tf.get(), it first attempts to grab all the rows from 
c_data_table_name. The first time you do this in a database, that table 
won't exist. tg.get's exception block will create a temp table holding 
the results of the stored statement (IE: INSERT INTO table ...).


Something else important here is that in crash.sql there is a nested 
tf.get() call:


-- Invoice
, $$INSERT INTO invoice VALUES(
DEFAULT
, (tf.get( NULL::customer, 'insert' )).customer_id
, current_date
, current_date + 30
  ) RETURNING *$$

Note that calls tf.get for customer (which is a simple INSERT).

This is where stuff gets weird. If you run tf.get( NULL::customer, 
'insert' ) you get a regular plpgsql error. If you simply run tf.get() 
for invoices, *outside* of tap.results_eq(), you also only get a plpgsql 
error. To trigger the assert, you must use tf.get( NULL::invoice, 'base' 
) from within tap.results_eq(). That's the only way I've found to 
trigger this.


AFAICT, that call stack looks like this:

results_eq opens a cursor to run $$SELECT * FROM tf.get( NULL::invoice, 
'base' )$$


plpgsql does it's thing and eventually that statement begins execution
tf.get() does a few things then attempts to read from a non-existent 
table. tf.get's outer block catches that exception and runs dynamic SQL 
to create a temp table. That dynamic SQL contains (in part) this:  , 
(tf.get( NULL::customer, 'insert' )).customer_id


*That* tf.get also attempts to read from a non-existent table and 
*successfully* creates it's temp table. It then does

  PERFORM _tf.table_create( c_data_table_name );

which fails due to a bug in _tf.table_create().

Now we have a second exception bubbling back up to the exception handler 
of the second tf.get call, which goes up to the exception handler for 
the first tf.get call. That call was in the process of creating a temp 
table (invoice_003). The error continues up to the FETCH command in 
results_eq(). The assert happens somewhere after here, and it's because 
the refcount on that temp table (invoice_003) is unexpected. I'm tracing 
through this scenario by hand right now to try and figure out exactly 
when that assert blows up, but I know it's happening in 
results_eq(refcursor, refcursor, text).


BTW, I just updated the crash branch to ensure that test_factory 0.1.1 
is what gets installed.

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


--
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] Fwd: Core dump with nested CREATE TEMP TABLE

[Jim Nasby]

 90 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 91 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 92 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 93 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 94 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 95 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 96 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 112 is uninitialized --- fixing
WARNING:  relation "pg_depend" page 128 is uninitialized --- fixing
--
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


--
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] Counting lines correctly in psql help displays

[Jim Nasby]

On 9/5/15 3:50 PM, Tom Lane wrote:

Greg Stark <st...@mit.edu> writes:

But that said, here's a tricksy patch that triggers an assertion
failure if the expected_lines is wrong. I intended it to trigger in
the regression tests so it only checks if the pager is actually off.
It wouldn't be hard to make it always check though.


Hmm ... that would put a premium on the linecount always being exactly
right (for all callers, not just the help functions).  Not sure that
I want to buy into that --- it would require more complexity in the
callers than is there now, for sure.


But only in an assert-enabled build. Surely there's enough other 
performance hits with asserts enabled that this wouldn't matter?


As for paging, ISTM the only people that would care are those with 
enormous terminal sizes would care, and assuming their pager is less 
simply adding -F to $LESS would get them their old behavior. So I think 
it's safe to just force paging.

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


--
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] Summary of plans to avoid the annoyance of Freezing

[Jim Nasby]

On 9/6/15 7:25 AM, Andres Freund wrote:

On 2015-08-10 07:03:02 +0100, Simon Riggs wrote:

I was previously a proponent of (2) as a practical way forwards, but my
proposal here today is that we don't do anything further on 2) yet, and
seek to make progress on 5) instead.

If 5) fails to bring a workable solution by the Jan 2016 CF then we commit
2) instead.

If Heikki wishes to work on (5), that's good. Otherwise, I think its
something I can understand and deliver by 1 Jan, though likely for 1 Nov CF.


I highly doubt that we can get either variant into 9.6 if we only start
to seriously review them by then. Heikki's lsn ranges patch essentially
was a variant of 5) and it ended up being a rather complicated patch. I
don't think using an explicit epoch is going to be that much simpler.

So I think we need to decide now.

My vote is that we should try to get freeze maps into 9.6 - that seems
more realistic given that we have a patch right now. Yes, it might end
up being superflous churn, but it's rather localized. I think around
we've put off significant incremental improvements off with the promise
of more radical stuff too often.


I'm concerned with how to test this. Right now it's rather difficult to 
test things like epoch rollover, especially in a way that would expose 
race conditions and other corner cases. We obviously got burned by that 
on the MultiXact changes, and a lot of our best developers had to spend 
a huge amount of time fixing that. ISTM that a way to unit test things 
like CLOG/MXID truncation and visibility logic should be created before 
attempting a change like this. Would having this kind of test 
infrastructure have helped with the LSN patch development? More 
importantly, would it have reduced the odds of the MXID bugs, or made it 
easier to diagnose them?


In any case, thanks Simon for the summary. I really like the idea and 
will help with it if I can.

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


--
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] Fwd: Core dump with nested CREATE TEMP TABLE

[Jim Nasby]

On 9/2/15 2:56 PM, Jim Nasby wrote:

On 9/2/15 2:17 PM, Alvaro Herrera wrote:

Michael Paquier wrote:


I haven't written yet a test case but I think that we could reproduce
that simply by having a relation referenced in the exception block of
a first function, calling a second function that itself raises an
exception, causing the referencing error.


Hm, so function 2 is called in the exception block of function 1?

Are you going to produce the test case for this?  Jim?


I had attempted one but the issue is that it's more than just an
exception block thing. If it was that simple then we'd still get the
crash without involving pgTap. So I suspect you need to have a named
cursor in the mix as well.

Let me make another attempt at something simpler.


After some time messing around with this I've been able to remove pgTap 
from the equation using the attached crash.sql (relevant snippet below).


This only works if you pass a refcursor into the function. It doesn't 
work if you do


OPEN have FOR EXECUTE $$SELECT * FROM tf.get( NULL::invoice, 'base' )$$;

inside the function instead. This code also produces different results; 
it outputs the error message before crashing and the stack trace shows 
the assert is now failing while trying to abort the top-level 
transaction. So it looks like the scenario is:


BEGIN;
DECLARE (open?) cursor that calls function with exception handler that 
calls function with exception handler that calls something that fails


The double set of exceptions seems to be critical; if instead of calling 
tf.get(invoice) (which recursively does tf.get(customer)) I define the 
cursor to call tf.get(customer) directly there's no assert.


I can poke at this more tomorrow, but it'd be very helpful if someone 
could explain this failure mode, as I'm basically stumbling in the dark 
on a test case right now.


CREATE OR REPLACE FUNCTION a(
have refcursor
) RETURNS void LANGUAGE plpgsql AS $body$
DECLARE
have_rec record;
BEGIN
FETCH have INTO have_rec;
END
$body$;
DECLARE h CURSOR FOR SELECT * FROM tf.get( NULL::invoice, 'base' );
SELECT a(
  'h'::refcursor
);


(lldb) bt
* thread #1: tid = 0x722ed8, 0x7fff92a5a866 
libsystem_kernel.dylib`__pthread_kill + 10, queue = 'com.apple.main-thread', 
stop reason = signal SIGABRT
  * frame #0: 0x7fff92a5a866 libsystem_kernel.dylib`__pthread_kill + 10
frame #1: 0x7fff9001b35c libsystem_pthread.dylib`pthread_kill + 92
frame #2: 0x7fff8cf89b1a libsystem_c.dylib`abort + 125
frame #3: 0x00010cdb4039 
postgres`ExceptionalCondition(conditionName=0x00010cf59cfd, 
errorType=0x00010cec392d, fileName=0x00010cf59045, lineNumber=1972) + 
137 at assert.c:54
frame #4: 0x00010cd9b332 
postgres`RelationClearRelation(relation=0x00011658f110, rebuild='\0') + 162 
at relcache.c:1970
frame #5: 0x00010cd9cc0f 
postgres`AtEOSubXact_cleanup(relation=0x00011658f110, isCommit='\0', 
mySubid=15, parentSubid=1) + 79 at relcache.c:2665
frame #6: 0x00010cd9cb92 
postgres`AtEOSubXact_RelationCache(isCommit='\0', mySubid=15, parentSubid=1) + 
242 at relcache.c:2633
frame #7: 0x00010c9b6e88 postgres`AbortSubTransaction + 440 at 
xact.c:4373
frame #8: 0x00010c9b7208 postgres`AbortCurrentTransaction + 312 at 
xact.c:2947
frame #9: 0x00010cc249f3 postgres`PostgresMain(argc=1, 
argv=0x7fa3f4800378, dbname=0x7fa3f4800200, 
username=0x7fa3f30031f8) + 1875 at postgres.c:3902
frame #10: 0x00010cb9da48 postgres`PostmasterMain [inlined] BackendRun 
+ 8024 at postmaster.c:4155
frame #11: 0x00010cb9da22 postgres`PostmasterMain [inlined] 
BackendStartup at postmaster.c:3829
frame #12: 0x00010cb9da22 postgres`PostmasterMain [inlined] ServerLoop 
at postmaster.c:1597
frame #13: 0x00010cb9da22 postgres`PostmasterMain(argc=, 
argv=) + 7986 at postmaster.c:1244
frame #14: 0x00010cb218cd postgres`main(argc=, 
argv=) + 1325 at main.c:228
frame #15: 0x7fff8e9a35fd libdyld.dylib`start + 1

--
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
BEGIN;
\i test/helpers/tap_setup.sql

CREATE EXTENSION test_factory;
SET search_path=tap;
\i test/helpers/create.sql

SELECT tf.register(
  'customer'
  , array[
row(
  'insert'
  , $$INSERT INTO customer VALUES (DEFAULT, 'first', 'last' ) RETURNING *$$
)::tf.test_set
, row(
  'function'
  , $$SELECT * FROM customer__add( 'func first', 'func last' )$$
)::tf.test_set
  ]
);
SELECT tf.register(
  'invoice'
  , array[
  row(
'insert'
, $$INSERT INTO invoice VALUES(
DEFAULT
, (tf.get( NULL::customer, 'insert' )).customer_id
, current_date
, current_date + 30
  ) RETURNING *$$
  )::tf.test_set
  , row(
'function'
, $$INSERT INTO invoice VALUES

Re: [HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

[Jim Nasby]

On 9/2/15 2:17 PM, Alvaro Herrera wrote:

Michael Paquier wrote:


I haven't written yet a test case but I think that we could reproduce
that simply by having a relation referenced in the exception block of
a first function, calling a second function that itself raises an
exception, causing the referencing error.


Hm, so function 2 is called in the exception block of function 1?

Are you going to produce the test case for this?  Jim?


I had attempted one but the issue is that it's more than just an 
exception block thing. If it was that simple then we'd still get the 
crash without involving pgTap. So I suspect you need to have a named 
cursor in the mix as well.


Let me make another attempt at something simpler.
--
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


--
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] jsonb_concat: make sure we always return a non-scalar value

[Jim Nasby]

On 9/9/15 12:03 PM, Oskari Saarenmaa wrote:

andrew=# select '[1]'::jsonb || '{"a":"b"}';
 ?column?
-
  [1, {"a": "b"}]


It looks wrong, but I'm not sure what's right in that case.  I think
it'd make sense to just restrict concatenation to object || object,
array || array and array || scalar and document that.  I doubt many
people expect their objects to turn into arrays if they accidentally
concatenate an array into it.  Alternatively the behavior could depend
on the order of arguments for concatenation, array || anything -> array,
object || array -> error.


That definitely doesn't sound like a good default.

It might be useful to have a concat function that would concatinate 
anything into an array. But if we don't provide one by default users 
could always create their own with json__typeof() and json_build_array().

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


--
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] SQL function to report log message

[Jim Nasby]

On 9/9/15 5:27 PM, Robert Haas wrote:

Sure, it’s a clear fact that, we can implement this function with RAISE
>statements.

Given that, I suggest we just forget the whole thing.


Except that you can't use a variable to control the log level in a 
plpgsql RAISE, so then you end up with a CASE statement. That perhaps 
wouldn't be so bad, until you also want to optionally report detail, 
hint, and/or errcode. So trying to create a generic wrapper around RAISE 
is decidedly non-trivial. Another option is removing those restrictions 
from RAISE, but it seems a bit silly to take the hit of firing up a 
plpgsql function for this.


So I think there is value in a SQL equivalent to RAISE. I'm not thrilled 
by piling another hack onto the horribly inadequate errlevel 
infrastructure, but at least Dinesh's "MESSAGE" idea is essentially just 
side-stepping that hole instead of digging it deeper.

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


--
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] Attach comments to functions' parameters and return value

[Jim Nasby]

On 9/14/15 8:59 AM, Charles Clavadetscher wrote:

Hello

In PostgreSQL it is possible to attach comments to almost everything. This
made it possible for us to integrate the wiki that we use for our technical
documentation directly with the database using the MediaWiki [1] extensions
ExternalData [2] and MagicNoCache [3]. The result is a documentation on
tables and related objects (indexes, triggers, etc.) and views that always
shows the current state, i.e. any DDL change or any comment attached to an
object is shown in the wiki immediately (or on refresh if the change was
done after the reader landed on the page).


Neat! I hope you'll open source that. :)


In order to optimize the query, we wrote a small set of sql functions that
generate wiki markup for the objects queried. The idea behind is that this
is much quicker in PostgreSQL than on a web server hosting MediaWiki,
besides a better control of the privileges for the user retrieving data.


And that! :)


So far we can document in such a way tables and views. I started to create
something similar for functions until I noticed that there is no way to
attach comments on functions' parameters and return value. My first idea was
to add this information in the function description, but this is quite an
ugly solution.

My next workaround is to simulate the behaviour of a COMMENT ON FUNCTION
PARAMETER/RETURNVALUE command inserting comments on these directly in
pg_description. For that I used a convention similar to the one used for
table attributes and defined following pg_description.objsubid:

   -1 = return value
0 = comment on the function itself (this already exists)
1..n = comment on parameter at position n


At first glance, seems reasonable.


   - Add function to get the position of the parameter, e.g.
LookupFuncNamePos(function_name, param_name) or a function to create the
whole ObjectAddress e.g. .


Something similar might exist already. TBH, to start with, I would only 
support position number. You'll have to support that case anyway, and it 
should be simpler.



To long time PostgreSQL developers this may look straightforward. For the
moment I am not even sure if that is correct and if there are other places
that would need additions, apart from the obvious display in psql.


I suspect that changes to support this should be pretty localized. I 
suggest you look at other recent patches that have added COMMENT 
functionality to see what they did.


BTW, I'm also interested in this but I'm not sure when I'd have time to 
work on it.

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


--
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] Covering + unique indexes.

[Jim Nasby]

On 9/11/15 7:45 AM, Anastasia Lubennikova wrote:

This idea has obvious restriction. We can set unique only for first
index columns.
There is no clear way to maintain following index.
CREATE INDEX index ON table (c1, c2, c3) UNIQUE ON (c1, c3);

So I suggest following syntax:
CREATE [UNIQUE {ON FIRST {COLUMN | n_unique_column COLUMNS}} INDEX ON
table_name (column_name1, column_name2 ...);


I would use the first (simple) syntax and just throw an error if the 
user tries to skip a column on the UNIQUE clause.


Have you by chance looked to see what other databases have done for 
syntax? I'm guessing this isn't covered by ANSI but maybe there's 
already an industry consensus.

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


--
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] Do Layered Views/Relations Preserve Sort Order ?

[Jim Nasby]

On 9/9/15 7:55 PM, Charles Sheridan wrote:

The better question is how expensive is it to sort already sorted
data.  If its cheap, and it likely is, then placing explicit sorting
where you care is the best solution regardless of your level of
confidence that lower level sorting is being maintained.

...


David,  yes, I agree that sorting at the end is the highest-confidence
approach.  I don't (yet) have a large stack of views with an assumption
of a guaranteed underlying sort order, I'm just trying to get a better
sense of what Postgres behavior I can reasonably expect here.


BTW, I believe there is some code in the planner to remove useless 
ORDER-BYs.

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


--
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] WIP: Make timestamptz_out less slow.

[Jim Nasby]

On 9/13/15 2:43 AM, David Rowley wrote:

Are you worried about this because I've not focused on optimising float
timestamps as much as int64 timestamps?  Are there many people compiling
with float timestamps in the real world?


My $0.02: the default was changed some 5 years ago so FP time is 
probably pretty rare now. I don't think it's worth a bunch of extra work 
to speed them up.

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


--
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] Covering + unique indexes.

[Jim Nasby]

On 9/14/15 1:50 PM, Thomas Munro wrote:

CREATE [UNIQUE {ON FIRST {COLUMN | n_unique_column COLUMNS}}
INDEX ON
table_name (column_name1, column_name2 ...);


I would use the first (simple) syntax and just throw an error if the
user tries to skip a column on the UNIQUE clause.

Seems, second option looks as more natural extension of CREATE
UNIQUE INDEX


True, but it's awefully verbose. :( And...


It surprised me that you can INCLUDE extra columns on non-UNIQUE
indexes, since you could just add them as regular indexed columns for
the same effect.  It looks like when you do that in SQL Server, the
extra columns are only stored on btree leaf pages and so can't be used
for searching or ordering.  I don't know how useful that is or if we
would ever want it... but I just wanted to note that difference, and
that the proposed UNIQUE ON FIRST n COLUMNS syntax and catalog change
can't express that.


... we might want to support INCLUDE at some point. It enhances covering 
scans without bloating the heck out of the btree. (I'm not sure if it 
would help other index types...) So it seems like a bad idea to preclude 
that.


I don't see that UNIQUE ON FIRST precludes also supporting INCLUDE. 
Presumably we could do either


CREATE INDEX ... ON table (f1, f2, f3) UNIQUE(f1, f2) INCLUDE(f4);
or
CREATE UNIQUE ON FIRST 2 COLUMNS INDEX ... ON table (f1, f2, f3) 
INCLUDE(f4);


Personally, I find the first form easier to read.

Are we certain that no index type could ever support an index on (f1, 
f2, f3) UNIQUE(f1, f3)? Even if it doesn't make sense for btree, perhaps 
some other index could handle it.

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


--
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] Can extension build own SGML document?

[Jim Nasby]

On 9/15/15 8:43 AM, Tom Lane wrote:

Jim Nasby <jim.na...@bluetreble.com> writes:

I'm not sure SGML is the way to go anymore anyways. Asciidoc offers a
lot of what our SGML does in a much easier to support toolchain. It's
also natively supported by github, which makes it nice for others to
view the output (see [1] as an exmaple). If asciidoc isn't powerful
enough for what you need you can switch to asciidoctor which is even
more powerful[2].


AFAICT from a quick look at its documentation, asciidoc can produce
either html or docbook output; so as soon as you want something other
than html output (in particular, PDF), you're back to relying on the
exact same creaky docbook toolchain we use now.  Only with one extra
dependency in front of it.

Personally I never look at anything but the HTML rendering, but I doubt
that dropping support for all other output formats would fly :-(

I do agree that the SGML toolchain is getting pretty long in the tooth
and we need to be looking for something else.


I wasn't thinking of trying to replace the Postgres toolchain, but...

a2x (http://www.methods.co.nz/asciidoc/a2x.1.html) states that it can 
generate "PDF, EPUB, DVI, PS, LaTeX, XHTML (single page or chunked), man 
page, HTML Help or plain text formats using asciidoc(1) and other 
applications (see REQUISITES section). SOURCE_FILE can also be a DocBook 
file with an .xml extension."


What I expect would be a lot more effort is actually converting all the 
SGML to asciidoc. A quick google search doesn't turn up anything promising.


If the only bad part of our current toolchain is PDF then perhaps we 
should just rethink how that's being generated. Since a2x can take 
docbook input maybe that's the way to go. But my understanding was that 
we've modified docbook and that's where part of the pain is coming from? 
In that case a full-out migration to asciidoc might be better.

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


--
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] Multi-tenancy with RLS

[Jim Nasby]

On 9/14/15 7:38 PM, Haribabu Kommi wrote:

On Fri, Sep 11, 2015 at 7:50 AM, Joe Conway <m...@joeconway.com> wrote:

On 09/01/2015 11:25 PM, Haribabu Kommi wrote:

If any user is granted any permissions on that object then that user
can view it's meta data of that object from the catalog tables.
To check the permissions of the user on the object, instead of
checking each and every available option, I just added a new
privilege check option called "any". If user have any permissions on
the object, the corresponding permission check function returns
true. Patch attached for the same.

Any thoughts/comments?


Thanks for working on this! Overall I like the concept and know of use
cases where it is critical and should be supported. Some comments:


Thanks for the review, I will take care of the comments in the next patch.

I didn't find any better approach other than creating policies automatically
or providing permission to superuser on system catalog tables. If everyone
feels as this is the best approach, then i will create policies for all catalog
tables in the next version.


Instead of adding or removing the rules, couldn't they just stay in 
place and be governed by what the field in the database was set to? It 
would also be nice if we could grant full access to roles instead of 
requiring superuser to see everything. Perhaps instead of a boolean 
store a role name in pg_database; anyone granted that role can see the 
full catalogs.


Also, we've faced issues in the past with making catalog changes due to 
fear of breaking user scripts. Instead of doubling down on that with RLS 
on top of catalog tables, would it be better to move the tables to a 
different schema, make them accessible only to superusers and put views 
in pg_catalog?

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


--
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] Can extension build own SGML document?

[Jim Nasby]

On 9/14/15 6:06 PM, Michael Paquier wrote:

On Tue, Sep 15, 2015 at 6:01 AM, Alvaro Herrera wrote:

I think the only way upstream Postgres could offer this is as a way to
build a separate "book", i.e. not a chapter/section within the main
book.  I think it would require huge complications in doc/src/sgml's
Makefile.  Not sure it's worthwhile.


I'm not sure SGML is the way to go anymore anyways. Asciidoc offers a 
lot of what our SGML does in a much easier to support toolchain. It's 
also natively supported by github, which makes it nice for others to 
view the output (see [1] as an exmaple). If asciidoc isn't powerful 
enough for what you need you can switch to asciidoctor which is even 
more powerful[2].



I am not sure either, and as each project/developer have always
different requirements I am convinced that this is going to be
finished with enforced rules in Makefile rules for sure, so it is
really unclear what would be the potential benefit to have a
centralized facility. Take for example man pages,  those should not be
installed in share/doc/extension/ which is the default path, but in
$(DESTDIR)$(mandir)/man1...


I do wish there was better infrastructure for creating extensions, but I 
agree that the main project is not the way to handle that. There needs 
to be more than just a Makefile you can include too. In particular, the 
test framework is pretty ugly to deal with (unless you really like 
wading through regress.c diff output...) Bumping extension versions, 
creating distfiles and uploading to PGXN are also ripe for automation.


pgxn-utils makes an attempt at this by creating an extension template 
for you to build from, but that's ultimately problematic because there's 
no way to pull in improvements to the overall infrastructure (such as 
how to create manpages). I think something you can pull in via a git 
subtree might work better, at least for a makefile framework. I've been 
meaning to create that but haven't found the time yet.



[1] 
https://github.com/decibel/trunklet-format/blob/master/doc/trunklet-format.asc

[2] http://asciidoctor.org/docs/asciidoc-asciidoctor-diffs/
--
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


--
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] Rework the way multixact truncations work

[Jim Nasby]

On 9/27/15 2:25 PM, Andres Freund wrote:

On 2015-09-27 14:21:08 -0500, Jim Nasby wrote:

IMHO doing just a log of something this serious; it should at least be a
WARNING.


In postgres LOG, somewhat confusingly, is more severe than WARNING.


Ahh, right. Which in this case stinks, because WARNING is a lot more 
attention grabbing than LOG. :/



I think the concern about upgrading a replica before the master is valid; is
there some way we could over-ride a PANIC when that's exactly what someone
is trying to do? Check for a special file maybe?


I don't understand this concern - that's just the situation we have in
all released branches today.


There was discussion about making this a PANIC instead of a LOG, which I 
think is a good idea... but then there'd need to be some way to not 
PANIC if you were doing an upgrade.



+   boolsawTruncationInCkptCycle;
What happens if someone downgrades the master, back to a version that no
longer logs truncation? (I don't think assuming that the replica will need
to restart if that happens is a safe bet...)


It'll just to do legacy truncation again - without a restart on the
standby required.


Oh, I thought once that was set it would stay set. NM.


-   if (MultiXactIdPrecedes(oldestMXact, earliest))
+   /* If there's nothing to remove, we can bail out early. */
+   if (MultiXactIdPrecedes(oldestMulti, earliest))
{
-   DetermineSafeOldestOffset(oldestMXact);
+   LWLockRelease(MultiXactTruncationLock);
If/when this is backpatched, would it be safer to just leave this alone?


What do you mean? This can't just isolated be left alone?


I thought removing DetermineSafeOldestOffset was just an optimization, 
but I guess I was confused.

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


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

[HACKERS] Has anyone run Cachegrind against the code?

[Jim Nasby]

Has anyone ever run cachegrind [1] against Postgres? I see little about 
it on the mailing list so I'm guessing no...


[1] http://valgrind.org/docs/manual/cg-manual.html
--
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


--
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] Idea for improving buildfarm robustness

[Jim Nasby]

On 9/29/15 4:13 PM, Alvaro Herrera wrote:

Joe Conway wrote:

On 09/29/2015 01:48 PM, Alvaro Herrera wrote:



I remember it, but I'm not sure it would have helped you.  As I recall,
your trouble was that after a reboot the init script decided to initdb
the mount point -- postmaster wouldn't have been running at all ...


Right, which the init script non longer does as far as I'm aware, so
hopefully will never happen again to anyone.


Yeah.


But it was still a case where the postmaster started on one copy of
PGDATA (the newly init'd copy), and then the contents of the real PGDATA
was swapped in (when the filesystem was finally mounted), causing
corruption to the production data.


Ah, I didn't remember that part of it, but it makes sense.


Ouch. So it sounds like there's value to seeing if pg_control isn't what 
we expect it to be.


Instead of looking at the inode (portability problem), what if 
pg_control contained a random number that was created at initdb time? On 
startup postmaster would read that value and then if it ever changed 
after that you'd know something just went wrong.


Perhaps even stronger would be to write a new random value on startup; 
that way you'd know if an old copy accidentally got put in place.

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


--
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] No Issue Tracker - Say it Ain't So!

[Jim Nasby]

On 9/29/15 3:36 PM, Oleg Bartunov wrote:

 ...What we're not fine with is depending on a proprietary
system, no
 matter what type of license, as infrastructure...


Exactly. Which is why I was warning about latching onto features
only available in the closed enterprise version.

Like Tom, I more or less promised myself not to get terribly
involved in this discussion. Oh, well.


me too, but I have to mention one problem we should have in mind - it's
independency from political games (sanctions).  Think about developers
from Russia, who one day may be blocked by Github, for example.


No one is suggesting we depend on proprietary or closed anything.

Community GitLab is NOT a "free sample". There are literally hundreds[1] 
of people that have submitted code to it.


The only thing I did suggest is that the easiest way to kick the tires 
on it would probably be to just plug into their cloud service. If people 
like it then we'd run our own.


I wish people would at least consider this as an option because it 
integrates a ton of different features together. It has *the potential* 
to eliminate our need to keep maintaining CommitFest and buildfarm and 
could also replace mediawiki.


If people are hell-bent on every tool being separate then fine, but I 
get the distinct impression that everyone is discarding GitLab out of 
hand based on completely bogus information.

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


--
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] No Issue Tracker - Say it Ain't So!

[Jim Nasby]

On 9/29/15 12:46 PM, Tom Lane wrote:

Andres Freund <and...@anarazel.de> writes:

On 2015-09-29 13:40:28 -0400, Tom Lane wrote:

I think you missed my point: gitlab would then believe it's in charge of,
eg, granting write access to that repo.  We could perhaps whack it over
the head till it only does what we want and not ten other things, but
we'd be swimming upstream.



We today already have a github mirror, where exactly the same thing
exists, no?


Sure, there's a mirror out there somewhere.  It has nothing to do with
our core development processes.


There are APIs as well, so it could be driven that way. I suspect it's 
unnecessary though.


BTW, the docs are at http://doc.gitlab.com/ce/.
--
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


--
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] No Issue Tracker - Say it Ain't So!

[Jim Nasby]

On 9/30/15 4:31 PM, Josh Berkus wrote:

On 09/30/2015 12:02 AM, Jim Nasby wrote:

I wish people would at least consider this as an option because it
integrates a ton of different features together. It has *the potential*
to eliminate our need to keep maintaining CommitFest and buildfarm and
could also replace mediawiki.

If people are hell-bent on every tool being separate then fine, but I
get the distinct impression that everyone is discarding GitLab out of
hand based on completely bogus information.


Well, Gitlab was introduced into this dicussion in the context of being
an OSS version of Github Issues.  If it's more than that, you're going
to have to explain.


It started as an OSS version of *Github* (not just Github Issues). It 
appears to have all the major features of github (issues, code review, 
wiki) plus a CI framework.

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


--
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] Rework the way multixact truncations work

[Jim Nasby]

On 9/23/15 1:48 PM, Andres Freund wrote:

Honestly, I wonder whether this message
>ereport(LOG,
>(errmsg("performing legacy multixact 
truncation"),
> errdetail("Legacy truncations are sometimes 
performed when replaying WAL from an older primary."),
> errhint("Upgrade the primary, it is 
susceptible to data corruption.")));
>shouldn't rather be a PANIC.  (The main reason not to, I think, is that
>once you see this, there is no way to put the standby in a working state
>without recloning).

Huh? The behaviour in that case is still better than what we have in
9.3+ today (not delayed till the restartpoint). Don't see why that
should be a panic. That'd imo make it pretty much impossible to upgrade
a pair of primary/master where you normally upgrade the standby first?


IMHO doing just a log of something this serious; it should at least be a 
WARNING.


I think the concern about upgrading a replica before the master is 
valid; is there some way we could over-ride a PANIC when that's exactly 
what someone is trying to do? Check for a special file maybe?


+   boolsawTruncationInCkptCycle;
What happens if someone downgrades the master, back to a version that no 
longer logs truncation? (I don't think assuming that the replica will 
need to restart if that happens is a safe bet...)


-   if (MultiXactIdPrecedes(oldestMXact, earliest))
+   /* If there's nothing to remove, we can bail out early. */
+   if (MultiXactIdPrecedes(oldestMulti, earliest))
{
-   DetermineSafeOldestOffset(oldestMXact);
+   LWLockRelease(MultiXactTruncationLock);
If/when this is backpatched, would it be safer to just leave this alone?
--
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


--
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] On-demand running query plans using auto_explain and signals

[Jim Nasby]

On 9/18/15 5:05 AM, Shulgin, Oleksandr wrote:


It should not be true - the data sender create DSM and fills it.
Then set caller slot and send signal to caller. Caller can free DSM
any time, because data sender send newer touch it.


But the requester can timeout on waiting for reply and exit before it
sees the reply DSM.  Actually, I now don't even think a backend can free
the DSM it has not created.  First it will need to attach it,
effectively increasing the refcount, and upon detach it will only
decrease the refcount, but not actually release the segment...

So this has to be the responsibility of the reply sending backend in the
end: to create and release the DSM *at some point*.


What's wrong with just releasing it at the end of the statement? When 
the statement is done there's no point to reading it asynchronously anymore.

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


--
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] No Issue Tracker - Say it Ain't So!

[Jim Nasby]

On 9/28/15 5:34 PM, Tom Lane wrote:

Now, running gitlab on community-owned hardware would potentially be an
option, if we find gitlab attractive from a functionality standpoint.
The question I'd have about that is whether it has a real development
community, or is open-source in name only.  If github did go belly up,
would we find ourselves maintaining the gitlab code all by ourselves?
That might not be the end of the world, but it wouldn't be a good use
of community time either.


FWIW, Gitlab appears to be a completely separate company from GitHub. 
According to https://about.gitlab.com/about/, over 800 people have 
contributed to it. It actually started as an open source project.



Fundamentally, we're playing the long game here.  We do not want to make
a choice of tools that we're going to regret ten years from now.


Agreed.

In this case it's a question of whether we want (or think in the future 
we might want) the advanced features that Gitlab offers. Things like 
commenting directly on "patches" (really, pull requests), direct 
integration with the issue tracker, a CI framework, etc.


Perhaps there's not a strong desire for those features today, but 
tomorrow could be a very different case. I'm honestly rather shocked 
(plesantly) that the community is seriously considering a bug tracker, 
given the reaction that's happened every time in the past. I think it'd 
be a real shame if a few years from now the community might consider, 
say, pull requests instead of emailed patches... except that would mean 
we need Yet Another Tool. Another example is CI. Yes, we have the 
buildfarm, but that does nothing to prevent bitrot in patches.


Actually, now that I've poked around the site a bit, it might well be 
worth adopting Gitlab just to replace some of our current stand-alone 
tools with a single integrated solution, depending on how much time we 
spend maintaining all the separate stuff.

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


--
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] No Issue Tracker - Say it Ain't So!

[Jim Nasby]

On 9/28/15 6:06 PM, Tom Lane wrote:

Josh Berkus <j...@agliodbs.com> writes:

The infra team seems to be good with debbugs, and several committers
seem to like it, why not go with it?


It certainly seems like debbugs is the proposal to beat at this point.

In the end though, what matters is somebody doing the dogwork to make
it happen: connecting some tool up to our workflow and populating it
with an initial collection of items.  Until that happens, we're just
arguing in a vacuum with no way to see whether a proposal will really
work.


Note that since they also offer a hosted solution we should use that to 
play with instead of trying to install it at this point.


Integrating the issue tracker looks like it's just a call to this API: 
http://doc.gitlab.com/ce/api/issues.html#new-issue. I don't normally do 
web development myself so I'd rather not figuring out how to setup a 
copy of the website to hack on, but if no one else wants to try it I can 
take a stab at it.


Presumably mirroring our git repository would work the same as it does 
for mirroring to GitHub. My guess is that would be enough to get the 
basic git/issue tracker integration working.


Commitfest could be tied in as well. Presumably each commitfest would be 
a milestone (http://doc.gitlab.com/ce/api/milestones.html) and each 
submission an issue.

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


--
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] No Issue Tracker - Say it Ain't So!

[Jim Nasby]

On 9/28/15 11:43 AM, Andres Freund wrote:

On 2015-09-28 09:41:18 -0700, David Fetter wrote:

Since you're convinced that this is an unqualified win, please put
together a project plan for switching from our current system to
Github.


Err, no. That's a waste of all our time.

It has been stated pretty clearly in this thread by a number of senior
community people that we're not going to use a closed source system.


GitLab OTOH is released under a MIT license, so it is an option. I don't 
know how it compares to other suggested options, but if YUriy wants to 
propose it it's at least a viable option.


[1] https://gitlab.com/gitlab-org/gitlab-ce/blob/master/LICENSE
--
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


--
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] Rework the way multixact truncations work

[Jim Nasby]

On 9/28/15 8:49 PM, Robert Haas wrote:

If at some point we back-patch this further, then it potentially
becomes a live issue, but I would like to respectfully inquire what
exactly you think making it a PANIC would accomplish?  There are a lot
of scary things about this patch, but the logic for deciding whether
to perform a legacy truncation is solid as far as I know.


Maybe I'm confused, but I thought the whole purpose of this was to get 
rid of the risk associated with that calculation in favor of explicit 
truncation boundaries in the WAL log.


Even if that's not the case, ISTM that being big and in your face about 
a potential data corruption bug is a good thing, as long as the DBA has 
a way to "hit the snooze button".


Either way, I'm not going to make a fuss over it.

Just to make sure we're on the same page; Alvaro's original comment was:

Honestly, I wonder whether this message
ereport(LOG,
(errmsg("performing legacy multixact 
truncation"),
 errdetail("Legacy truncations are sometimes 
performed when replaying WAL from an older primary."),
 errhint("Upgrade the primary, it is 
susceptible to data corruption.")));
shouldn't rather be a PANIC.  (The main reason not to, I think, is that
once you see this, there is no way to put the standby in a working state
without recloning).

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


--
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] No Issue Tracker - Say it Ain't So!

[Jim Nasby]

On 9/28/15 9:03 PM, Stephen Frost wrote:

* Ryan Pedela (rped...@datalanche.com) wrote:

I haven't used Gitlab extensively, but it has a feature set similar to
Github and then some [1]. The OSS project does seem active [2], but it is
still relatively new.


I've set it up and used it for a relatively small environment and was
*not* impressed with it.  Perhaps it's come a long way in a year or two,
but I doubt it.


2 years ago is when they first released the enterprise edition, which 
according to [1] had "The most important new feature is that you can now 
add members to groups of projects."


So looking at it now I'd say it's come a long way in 2 years.

[1] 
https://about.gitlab.com/2013/08/22/introducing-gitlab-6-0-enterprise-edition/

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


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

[HACKERS] Less than ideal error reporting in pg_stat_statements

[Jim Nasby]

A client was getting some hard to diagnose out of memory errors. What 
made this especially confusing was that there was no context reported at 
all, other than the (enormous) statement that triggered the error.


At first I thought the lack of context indicated a palloc had failed 
during ereport() (since we apparently just toss the previous error when 
that happens), but it turns out there's some error reporting in 
pg_stat_statements that's less than ideal. Attached patch fixes, though 
I'm not sure if %lld is portable or not.


I'll also argue that this is a bug and should be backpatched, but I'm 
not hell-bent on that.


At the same time I looked for other messages that don't explicitly 
reference pg_stat_statements; the only others are in 
pg_stat_statements_internal() complaining about being called in an 
inappropriate function context. Presumably at that point there's a 
reasonable error context stack so I didn't bother with them.


This still seems a bit fragile to me though. Anyone working in here has 
to notice that most every errmsg mentions pg_stat_statements and decide 
there's a good reason for that. ISTM it'd be better to push a new 
ErrorContextCallback onto the stack any time we enter the module. If 
folks think that's a good idea I'll pursue it as a separate patch.

--
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
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c 
b/contrib/pg_stat_statements/pg_stat_statements.c
index 59b8a2e..06a0912 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -1893,14 +1893,24 @@ qtext_load_file(Size *buffer_size)
 
/* Allocate buffer; beware that off_t might be wider than size_t */
if (stat.st_size <= MaxAllocSize)
-   buf = (char *) malloc(stat.st_size);
-   else
-   buf = NULL;
-   if (buf == NULL)
{
+   buf = (char *) malloc(stat.st_size);
+
+   if (buf == NULL)
+   {
+   ereport(LOG,
+   (errcode(ERRCODE_OUT_OF_MEMORY),
+errmsg("out of memory attempting to 
pg_stat_statement file"),
+errdetail("file \"%s\": size %lld", 
PGSS_TEXT_FILE, stat.st_size)));
+   CloseTransientFile(fd);
+   return NULL;
+   }
+   } else {
ereport(LOG,
+   /* Is there a better code to use? IE: SQLSTATE 
53000, 53400 or 54000 */
(errcode(ERRCODE_OUT_OF_MEMORY),
-errmsg("out of memory")));
+errmsg("pg_stat_statement file is too large to 
process"),
+errdetail("file \"%s\": size %lld", 
PGSS_TEXT_FILE, stat.st_size)));
CloseTransientFile(fd);
return NULL;
}

-- 
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] Less than ideal error reporting in pg_stat_statements

[Jim Nasby]

On 9/22/15 5:58 PM, Peter Geoghegan wrote:

On Tue, Sep 22, 2015 at 3:16 PM, Jim Nasby <jim.na...@bluetreble.com> wrote:

At first I thought the lack of context indicated a palloc had failed during
ereport() (since we apparently just toss the previous error when that
happens), but it turns out there's some error reporting in
pg_stat_statements that's less than ideal. Attached patch fixes, though I'm
not sure if %lld is portable or not.


+ ereport(LOG,
+  (errcode(ERRCODE_OUT_OF_MEMORY),
+   errmsg("out of memory attempting to pg_stat_statement file"),
+   errdetail("file \"%s\": size %lld", PGSS_TEXT_FILE,
stat.st_size)));

Uh, what?


Oops. I'll fix that and address David's concern tomorrow.


I'm not opposed to this basic idea, but I think the message should be
reworded, and that the presence of two separate ereport() call sites
like the above is totally unnecessary. The existing MaxAllocSize check
is just defensive; no user-visible distinction needs to be made.


I disagree. If you're running this on a 200+GB machine with plenty of 
free memory and get that error you're going to be scratching your head. 
I can see an argument for using the OOM SQLSTATE, but treating an 
artificial limit the same as a system error seems pretty bogus.

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


--
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] VACUUM Progress Checker.

[Jim Nasby]

On 9/24/15 7:37 AM, Masahiko Sawada wrote:

* The progress of VACUUM FULL seems wrong.
When I run VACUUM FULL for a table, I got following progress.


It never occurred to me that this patch was attempting to measure the 
progress of a CLUSTER (aka VACUUM FULL). I'm not sure that's such a 
great idea, as the progress estimation presumably needs to be 
significantly different.


More to the point, you can't estimate a CLUSTER unless you can estimate 
the progress of an index build. That'd be a cool feature to have as 
well, but it seems like a bad idea to mix that in with this patch.


Keep in mind that running a VACUUM FULL is presumably a LOT less common 
than regular vacuums, so I don't think leaving it out for now is that 
big a deal.



* The vacuum by autovacuum is not displayed.
I tested about this by the executing the following queries in a row,
but the vacuum by autovacuum is not displayed,


IIRC this is the second problem related to autovacuum... is there some 
way to regression test that? Maybe disable autovac on a table, dirty it, 
then re-enable (all with an absurdly low autovacuum naptime)?

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


--
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.3.9 and pg_multixact corruption

[Jim Nasby]

On 9/20/15 9:23 AM, Christoph Berg wrote:

a short update here: the customer updated the compiler to a newer
version, is now compiling using -O2 instead of -O3, and the code
generated now looks sane, so this turned out to be a compiler issue.
(Though it's unclear if the upgrade fixed it, or the different -O
level.)


Do we officially not support anything > -O2? If so it'd be nice if 
configure threw at least a warning (if not an error that you had to 
explicitly over-ride).

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


--
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] Decimal64 and Decimal128

[Jim Nasby]

On 9/24/15 3:35 PM, Peter Geoghegan wrote:

I would worry about the implicit casts you've added. They might cause problems.


Given the cycle created between numeric->decimal and decimal->numeric, I 
can pretty much guarantee they will. In any case, I don't think implicit 
casting from numeric->decimal is a good idea since it can overflow. I'm 
not sure that the other direction is safe either... I can't remember 
offhand if casting correctly obeys typmod or not.


BTW, have you talked to Pavel about making these changes to his code? 
Seems a shame to needlessly fork it. :/

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


--
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] No Issue Tracker - Say it Ain't So!

[Jim Nasby]

On 9/23/15 3:12 PM, Thomas Kellerer wrote:

They also support Postgres as their backend (and you do find hints here and
there
that it is the recommended open source DBMS for them - but they don't
explicitly state it like that). We are using Jira at the company I work for
and
all Jira installations run on Postgres there.


I'll second Jira as well. It's the only issue tracker I've seen that you 
can actually use for multiple different things without it becoming a 
mess. IE: it could track Postgres bugs, infrastructure issues, and the 
TODO list if we wanted, allow issues to reference each other 
intelligently, yet still keep them as 3 separate bodies.


They're also based here in Austin so we've got community folks that can 
interface with them directly if that's ever needed.

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


--
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] No Issue Tracker - Say it Ain't So!

[Jim Nasby]

On 9/23/15 3:29 PM, Alvaro Herrera wrote:

Joshua D. Drake wrote:


Until that happens asking anyone to put resources into this idea is just not
worth it.


I wonder if you still have this conversation archived:

Date: Thu, 10 May 2007 11:30:55 -0400
From: Andrew Dunstan <and...@dunslane.net>
To: "Joshua D. Drake", Magnus Hagander, Alvaro Herrera, Robert Treat, Lukas 
Smith, Andrew Sullivan, David Fetter
Subject: tracker

There was some very good input there -- it would be a shame to have to
repeat that all over again.  Hey, it's only been 8 years ...


Ha! Good to know our scheduling is consistent at least!
--
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


--
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] Less than ideal error reporting in pg_stat_statements

[Jim Nasby]

On 9/22/15 6:27 PM, Jim Nasby wrote:

+ ereport(LOG,
+  (errcode(ERRCODE_OUT_OF_MEMORY),
+   errmsg("out of memory attempting to pg_stat_statement
file"),
+   errdetail("file \"%s\": size %lld", PGSS_TEXT_FILE,
stat.st_size)));

Uh, what?


Oops. I'll fix that and address David's concern tomorrow.


New patch attached. I stripped the size reporting out and simplified the 
conditionals a bit as well.

--
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
diff --git a/contrib/pg_stat_statements/pg_stat_statements.c 
b/contrib/pg_stat_statements/pg_stat_statements.c
index 59b8a2e..c9dcd89 100644
--- a/contrib/pg_stat_statements/pg_stat_statements.c
+++ b/contrib/pg_stat_statements/pg_stat_statements.c
@@ -1892,15 +1892,24 @@ qtext_load_file(Size *buffer_size)
}
 
/* Allocate buffer; beware that off_t might be wider than size_t */
-   if (stat.st_size <= MaxAllocSize)
-   buf = (char *) malloc(stat.st_size);
-   else
-   buf = NULL;
+   if (stat.st_size > MaxAllocSize)
+   {
+   ereport(LOG,
+   /* Is there a better code to use? IE: SQLSTATE 
53000, 53400 or 54000 */
+   (errcode(ERRCODE_OUT_OF_MEMORY),
+errmsg("pg_stat_statement file is too large to 
process"),
+errdetail("file \"%s\"", 
PGSS_TEXT_FILE.st_size)));
+   CloseTransientFile(fd);
+   return NULL;
+   }
+
+   buf = (char *) malloc(stat.st_size);
+
if (buf == NULL)
{
ereport(LOG,
(errcode(ERRCODE_OUT_OF_MEMORY),
-errmsg("out of memory")));
+errmsg("out of memory attempting to read 
pg_stat_statement file")));
CloseTransientFile(fd);
return NULL;
}

-- 
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] Less than ideal error reporting in pg_stat_statements

[Jim Nasby]

On 9/22/15 8:01 PM, Peter Geoghegan wrote:

I'm doubtful that this had anything to do with MaxAllocSize. You'd
certainly need a lot of bloat to be affected by that in any way. I
wonder how high pg_stat_statements.max was set to on this system, and
how long each query text was on average.


max was set to 1. I don't know about average query text size, but 
the command that was causing the error was a very large number of 
individual INSERT ... VALUES statements all in one command.


The machine had plenty of free memory and no ulimit, so I don't see how 
this could have been anything but MaxAllocSize, unless there's some 
other failure mode in malloc I don't know about.

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


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

[HACKERS] Core dump with nested CREATE TEMP TABLE

[Jim Nasby]

I don't have an independent reproduction yet (though make test in [1] 
should reproduce this). I haven't actually been able to reproduce by 
hand yet, but pgtap has something to do with this. This is affecting at 
least 9.4 and a fairly recent HEAD.



-- Bits from top of test/sql/base.sql
\i test/helpers/setup.sql

SET ROLE = DEFAULT;
CREATE ROLE test_role;
GRANT USAGE ON SCHEMA tap TO test_role;
GRANT test_role TO test_factory__owner;

CREATE SCHEMA test AUTHORIZATION test_role;
SET ROLE = test_role;
SET search_path = test, tap;
\i test/helpers/create.sql

SELECT tf.register(
  'customer'
  , array[
row(
  'insert'
  , $$INSERT INTO customer VALUES (DEFAULT, 'first', 'last' ) 
RETURNING *$$

)::tf.test_set
, row(
  'function'
  , $$SELECT * FROM customer__add( 'func first', 'func last' )$$
)::tf.test_set
  ]
);
SELECT tf.register(
  'invoice'
  , array[
  row(
'base'
, $$INSERT INTO invoice VALUES(
DEFAULT
, (tf.get( NULL::customer, 'insert' )).customer_id
, current_date
, current_date + 30
  ) RETURNING *$$
  )::tf.test_set
  ]
);
SELECT no_plan();
SELECT lives_ok($$SELECT * FROM tf.get( NULL::invoice, 'base' )$$);
not ok 1
# Failed test 1
# died: 42703: column c_data_table_name does not exist


-- Ok, got an error, but no crash. But now...

SELECT results_eq(
  $$SELECT * FROM tf.get( NULL::invoice, 'base' )$$
  , $$VALUES( 1, 1, current_date, current_date + 30 )$$
  , 'invoice factory output'
);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.

lives_ok() and results_eq() are both pgTap functions. Definitions at [2] 
and [3].


I've attached a full server log from running make test, but the most 
relevant bit is below:


DEBUG:  AbortSubTransaction
CONTEXT:  PL/pgSQL function results_eq(refcursor,refcursor,text) line 11 
during exception cleanup

PL/pgSQL function results_eq(text,text,text) line 9 at assignment
DEBUG:  name: unnamed; blockState:INPROGRESS; state: INPROGR, 
xid/subid/cid: 1980/1/979, nestlvl: 1, children: 1981 1982 1983 1984 
1985 1986 1987 1988 1989 1990 1991 1992 1993 1994 1995 1996 1997 1998 1999
CONTEXT:  PL/pgSQL function results_eq(refcursor,refcursor,text) line 11 
during exception cleanup

PL/pgSQL function results_eq(text,text,text) line 9 at assignment
DEBUG:  name: pg_psql_temporary_savepoint; blockState:  SUB INPROGRS; 
state: INPROGR, xid/subid/cid: 2000/34/979, nestlvl: 2, children:
CONTEXT:  PL/pgSQL function results_eq(refcursor,refcursor,text) line 11 
during exception cleanup

PL/pgSQL function results_eq(text,text,text) line 9 at assignment
DEBUG:  name: unnamed; blockState:  SUB INPROGRS; state: INPROGR, 
xid/subid/cid: 2001/35/979, nestlvl: 3, children:
CONTEXT:  PL/pgSQL function results_eq(refcursor,refcursor,text) line 11 
during exception cleanup

PL/pgSQL function results_eq(text,text,text) line 9 at assignment
TRAP: FailedAssertion(!(rebuild ? !((bool)((relation)-rd_refcnt == 0)) 
: ((bool)((relation)-rd_refcnt == 0))), File: relcache.c, Line: 2055)


[1] https://github.com/BlueTreble/test_factory/tree/crash
[2] https://github.com/theory/pgtap/blob/master/sql/pgtap.sql.in#L746
[3] https://github.com/theory/pgtap/blob/master/sql/pgtap.sql.in#L6541 
which is being called by 
https://github.com/theory/pgtap/blob/master/sql/pgtap.sql.in#L6591

--
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
DEBUG:  CommitTransactionCommand
DEBUG:  StartTransactionCommand
DEBUG:  ProcessUtility
DEBUG:  CommitTransactionCommand
DEBUG:  CommitSubTransaction
DEBUG:  name: unnamed; blockState:INPROGRESS; state: INPROGR, 
xid/subid/cid: 1980/1/968, nestlvl: 1, children: 1981 1982 1983 1984 1985 1986 
1987 1988 1989 1990 1991 1992 1993 1994 1995 1996 1997 1998 1999
DEBUG:  name: pg_psql_temporary_savepoint; blockState:   SUB RELEASE; state: 
INPROGR, xid/subid/cid: 0/32/968, nestlvl: 2, children: 
DEBUG:  StartTransactionCommand
DEBUG:  ProcessUtility
DEBUG:  CommitTransactionCommand
DEBUG:  StartSubTransaction
DEBUG:  name: unnamed; blockState:INPROGRESS; state: INPROGR, 
xid/subid/cid: 1980/1/968, nestlvl: 1, children: 1981 1982 1983 1984 1985 1986 
1987 1988 1989 1990 1991 1992 1993 1994 1995 1996 1997 1998 1999
DEBUG:  name: pg_psql_temporary_savepoint; blockState: SUB BEGIN; state: 
INPROGR, xid/subid/cid: 0/33/968, nestlvl: 2, children: 
DEBUG:  StartTransactionCommand
DEBUG:  ProcessUtility
DEBUG:  CommitTransactionCommand
DEBUG:  StartTransactionCommand
DEBUG:  ProcessUtility
DEBUG:  CommitTransactionCommand
DEBUG:  CommitSubTransaction
DEBUG:  name: unnamed; blockState:INPROGRESS; state: INPROGR, 
xid/subid/cid: 1980/1

Re: [HACKERS] proposal: multiple psql option -c

[Jim Nasby]

On 8/28/15 3:31 PM, David G. Johnston wrote:

--psqlrc​
​; read the standard rc files​
--no-psqlrc ; do not read the standard rc files

It belongs in a separate patch, though.

In this patch -g should disable the reading of the standard rc files.


Agreed; I didn't realize -c disabled psqlrc.


Yet another option could be added that allows the user to point to a
different set of rc files.  Its presence should not cause the
include/exclude behavior to change.  That way you can setup a psql
wrapper function or alias that uses a different ​rc file while still
having control over whether it is included or excluded.  The problem
here is exploding the logic in order to deal with both a system and a
user rc file.


If we had a \i variation that didn't fail if the file wasn't readable 
you could use that to pull a system psqlrc in from your custom one.

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


--
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] psql - better support pipe line

[Jim Nasby]

On 8/28/15 3:58 AM, Shulgin, Oleksandr wrote:

It occurs to me the most flexible thing that could be done here
would be providing a libpq function that spits out JSON connection
parameters and have psql turn that into a variable. It would be easy
to feed that to a SQL statement and do whatever you want with it at
that point, including format it to a connection URI.


Hm... but that would mean that suddenly psql would need JSON parsing
capabilities and URI escaping code would have to be moved there too?  So
every client that links to libpq and wants to use this feature going as
far as reconstructing an URI would need both of the capabilities.


Anything that's doing this presumably has connected to the database, 
which on any recent version means you have plenty of ability to process 
JSON at the SQL layer.



Why instead of JSON not spit conninfo format, with proper escaping?
That could be a separate library call, e.g. PGgetConnectionString() and
a separate backslash command: \conninfo


Do you mean as a URI? The downside to that it's it's more difficult to 
parse than JSON. Another option might be an array.


The other issue is there's no way to capture \conninfo inside of psql 
and do something with it. If instead this was exposed as a variable, you 
could handle it in SQL if you wanted to.


All that said, the patch already adds significant value and you could 
always parse the URI if you really needed to.

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


--
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] Function accepting array of complex type

[Jim Nasby]

On 8/25/15 6:28 PM, Tom Lane wrote:

Jim Nasby jim.na...@bluetreble.com writes:

This works:
CREATE TYPE c AS (r float, i float);
CREATE FUNCTION mag(c c) RETURNS float LANGUAGE sql AS $$
SELECT sqrt(c.r^2 + c.i^2)
$$;
SELECT mag( (2.2, 2.2) );
 mag
--
   3.11126983722081



But this doesn't:
CREATE FUNCTION magsum( c c[] ) RETURNS float LANGUAGE sql AS $$
SELECT sum(sqrt(c.r^2 + c.i^2)) FROM unnest(c) c
$$;
SELECT magsum( array[row(2.1, 2.1), row(2.2,2.2)] );
ERROR:  function magsum(record[]) does not exist at character 8


You need to cast it to some specific record type:

regression=# SELECT magsum( array[row(2.1, 2.1), row(2.2,2.2)]::c[] );


Right, I was wondering how hard it would be to improve that, but it's 
not clear to me where to look at in the code. Does the resolution happen 
as part of parsing, or is it further down the road?

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


--
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: multiple psql option -c

[Jim Nasby]

On 8/26/15 8:15 AM, Pavel Stehule wrote:

+  and then exit. This is useful in shell scripts. Start-up files
+  (filenamepsqlrc/filename and filename~/.psqlrc/filename) are
+  ignored with this option.


Sorry if this was discussed and I missed it, but I think this is a bad 
idea. There's already an option to control this. More important, there's 
no option to force the rc files to be used, so if -g disables them you'd 
be stuck with that.


I agree that the rc files are a danger when scripting, but if we want to 
do something about that then it needs to be consistent for ALL 
non-interactive use.

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


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

[HACKERS] Fwd: Core dump with nested CREATE TEMP TABLE

[Jim Nasby]

Looks like a 98k file won't get through the list...


 Forwarded Message 
Subject: Core dump with nested CREATE TEMP TABLE
Date: Thu, 27 Aug 2015 19:45:12 -0500
From: Jim Nasby jim.na...@bluetreble.com
To: Pg Hackers pgsql-hackers@postgresql.org

I don't have an independent reproduction yet (though make test in [1]
should reproduce this). I haven't actually been able to reproduce by
hand yet, but pgtap has something to do with this. This is affecting at
least 9.4 and a fairly recent HEAD.


-- Bits from top of test/sql/base.sql
\i test/helpers/setup.sql

SET ROLE = DEFAULT;
CREATE ROLE test_role;
GRANT USAGE ON SCHEMA tap TO test_role;
GRANT test_role TO test_factory__owner;

CREATE SCHEMA test AUTHORIZATION test_role;
SET ROLE = test_role;
SET search_path = test, tap;
\i test/helpers/create.sql

SELECT tf.register(
   'customer'
   , array[
 row(
   'insert'
   , $$INSERT INTO customer VALUES (DEFAULT, 'first', 'last' )
RETURNING *$$
 )::tf.test_set
 , row(
   'function'
   , $$SELECT * FROM customer__add( 'func first', 'func last' )$$
 )::tf.test_set
   ]
);
SELECT tf.register(
   'invoice'
   , array[
   row(
 'base'
 , $$INSERT INTO invoice VALUES(
 DEFAULT
 , (tf.get( NULL::customer, 'insert' )).customer_id
 , current_date
 , current_date + 30
   ) RETURNING *$$
   )::tf.test_set
   ]
);
SELECT no_plan();
SELECT lives_ok($$SELECT * FROM tf.get( NULL::invoice, 'base' )$$);
not ok 1
# Failed test 1
# died: 42703: column c_data_table_name does not exist


-- Ok, got an error, but no crash. But now...

SELECT results_eq(
   $$SELECT * FROM tf.get( NULL::invoice, 'base' )$$
   , $$VALUES( 1, 1, current_date, current_date + 30 )$$
   , 'invoice factory output'
);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.

lives_ok() and results_eq() are both pgTap functions. Definitions at [2]
and [3].

I've attached a full server log from running make test, but the most
relevant bit is below:

DEBUG:  AbortSubTransaction
CONTEXT:  PL/pgSQL function results_eq(refcursor,refcursor,text) line 11
during exception cleanup
PL/pgSQL function results_eq(text,text,text) line 9 at assignment
DEBUG:  name: unnamed; blockState:INPROGRESS; state: INPROGR,
xid/subid/cid: 1980/1/979, nestlvl: 1, children: 1981 1982 1983 1984
1985 1986 1987 1988 1989 1990 1991 1992 1993 1994 1995 1996 1997 1998 1999
CONTEXT:  PL/pgSQL function results_eq(refcursor,refcursor,text) line 11
during exception cleanup
PL/pgSQL function results_eq(text,text,text) line 9 at assignment
DEBUG:  name: pg_psql_temporary_savepoint; blockState:  SUB INPROGRS;
state: INPROGR, xid/subid/cid: 2000/34/979, nestlvl: 2, children:
CONTEXT:  PL/pgSQL function results_eq(refcursor,refcursor,text) line 11
during exception cleanup
PL/pgSQL function results_eq(text,text,text) line 9 at assignment
DEBUG:  name: unnamed; blockState:  SUB INPROGRS; state: INPROGR,
xid/subid/cid: 2001/35/979, nestlvl: 3, children:
CONTEXT:  PL/pgSQL function results_eq(refcursor,refcursor,text) line 11
during exception cleanup
PL/pgSQL function results_eq(text,text,text) line 9 at assignment
TRAP: FailedAssertion(!(rebuild ? !((bool)((relation)-rd_refcnt == 0))
: ((bool)((relation)-rd_refcnt == 0))), File: relcache.c, Line: 2055)

[1] https://github.com/BlueTreble/test_factory/tree/crash
[2] https://github.com/theory/pgtap/blob/master/sql/pgtap.sql.in#L746
[3] https://github.com/theory/pgtap/blob/master/sql/pgtap.sql.in#L6541
which is being called by
https://github.com/theory/pgtap/blob/master/sql/pgtap.sql.in#L6591
--
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





--
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] Equivalence Class Filters

[Jim Nasby]

On 12/6/15 10:38 AM, Tom Lane wrote:

I said "in most cases".  You can find example cases to support almost any
weird planner optimization no matter how expensive and single-purpose;
but that is the wrong way to think about it.  What you have to think about
is average cases, and in particular, not putting a drag on planning time
in cases where no benefit ensues.  We're not committing any patches that
give one uncommon case an 1100X speedup by penalizing every other query 10%,
or even 1%; especially not when there may be other ways to fix it.


This is a problem that seriously hurts Postgres in data warehousing 
applications. We can't keep ignoring optimizations that provide even as 
little as 10% execution improvements for 10x worse planner performance, 
because in a warehouse it's next to impossible for planning time to matter.


Obviously it'd be great if there was a fast, easy way to figure out 
whether a query would be expensive enough to go the whole 9 yards on 
planning it but at this point I suspect a simple GUC would be a big 
improvement.

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


--
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] Equivalence Class Filters

[Jim Nasby]

On 12/7/15 9:54 AM, Tom Lane wrote:

Jim Nasby<jim.na...@bluetreble.com>  writes:

>On 12/6/15 10:38 AM, Tom Lane wrote:

>>I said "in most cases".  You can find example cases to support almost any
>>weird planner optimization no matter how expensive and single-purpose;
>>but that is the wrong way to think about it.  What you have to think about
>>is average cases, and in particular, not putting a drag on planning time
>>in cases where no benefit ensues.  We're not committing any patches that
>>give one uncommon case an 1100X speedup by penalizing every other query 10%,
>>or even 1%; especially not when there may be other ways to fix it.

>This is a problem that seriously hurts Postgres in data warehousing
>applications.

Please provide some specific examples.  I remain skeptical that this
would make a useful difference all that often in the real world ...
and handwaving like that does nothing to change my opinion.  What do
the queries look like, and why would deducing an extra inequality
condition help them?


I was speaking more broadly than this particular case. There's a lot of 
planner improvements that get shot down because of the planning overhead 
they would add. That's great for cases when milliseconds count, but 
spending an extra 60 seconds (a planning eternity) to shave an hour off 
a warehouse/reporting query.


There needs to be some way to give the planner an idea of how much 
effort it should expend. GEQO and *_collapse_limit addresses this in the 
opposite direction (putting a cap on planner effort), but I think we 
need something that does the opposite "I know this query will take a 
long time, so expend extra effort on planning it."

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


--
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] Equivalence Class Filters

[Jim Nasby]

On 12/7/15 10:44 AM, Simon Riggs wrote:

There are many optimizations we might adopt, yet planning time is a
factor. It seems simple enough to ignore more complex optimizations if
we have already achieved a threshold cost (say 10). Such a test would
add nearly zero time for the common case. We can apply the optimizations
in some kind of ordering depending upon the cost, so we are careful to
balance the cost/benefit of trying certain optimizations.


Unfortunately I've seen a lot of millisecond queries that have 6 figure 
estimates, due to data being in cache. So I'm not sure how practical 
that would be.


Maybe a better starting point would be a planner timeout.

I definitely agree we need some method to limit planning time when 
necessary (ie: OLTP). Without that we'll never be able to start testing 
more complex optimizations.

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


--
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] psql: add \pset true/false

[Jim Nasby]

On 11/15/15 7:37 PM, Peter Eisentraut wrote:

On 11/15/15 3:20 PM, Jim Nasby wrote:

As to the argument about displaying a check or an X, why should that
capability only exist for boolean types? For example, why not allow psql
to convert a numeric value into a bar of varying sizes? I've frequently
emulated that with something like SELECT repeat( '*', blah * 30 /
max_of_blah ). I'm sure there's other examples people could think of.


Well, why not?  The question there is only how many marginal features
you want to stuff into psql, not whether it's the right place to stuff them.


I was more thinking it would be nice to be able to temporarily 
over-ride/wrap what an output function is doing. AFAIK that would allow 
this to work everywhere (row(), copy, etc). I don't know of any remotely 
practical way to do that, though.

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


--
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: function parse_ident

[Jim Nasby]

On 9/15/15 11:49 PM, Pavel Stehule wrote:

1. processing user input with little bit more comfort - the user doesn't
need to separate schema and table


This is especially useful if you're doing anything that needs to 
dynamically work with different objects. I'd say about 80% of the time 
I'm doing this ::regclass is good enough, but obviously the object has 
to exist then, and ::regclass doesn't separate schema from name.


There's a number of other handy convenience functions/views you can 
create to interface with the catalog, none of which are rocket science. 
But you're pretty screwed if what you want isn't in the catalog yet. (On 
a side note, something my TODO is to restart pg_newsysviews as an 
extension, and then add a bunch of convenience functions on top of 
that... things like relation_info(regclass) RETURNS (everything in 
pg_class, plus other useful bits like nspname), and 
relation_schema(regclass) RETURNS regnamespace).


FWIW, the other function I've wanted in the past that's difficult to 
implement externally is parsing the arguments of a function definition. 
::regprocedure kinda works for this, but it blows up on certain things 
(defaults being one, iirc). I've specifically wanted that capability for 
a function I wrote that made it easy to specify *everything* about a 
function in a single call, including it's permissions and a comment on 
the function. That may sound trivial, but it's a PITA to cut and paste 
the whole argument list into multiple REVOKE/GRANT/COMMENT on 
statements. Even worse, not all the options of CREATE FUNCTION are 
supported in those other commands, so often you can't even just cut and 
paste.

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


--
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] Double linking MemoryContext children

[Jim Nasby]

On 9/14/15 7:24 AM, Jan Wieck wrote:

On 09/12/2015 11:35 AM, Kevin Grittner wrote:


On the other hand, a grep indicates that there are two places that
MemoryContextData.nextchild is set (and we therefore probably need
to also set the new field), and Jan's proposed patch only changes
one of them.  If we do this, I think we need to change both places
that are affected, so ResourceOwnerCreate() in resowner.c would
need a line or two added.


ResourceOwnerCreate() sets ResourceOwnerData.nextchild, not
MemoryContextData.nextchild.


Anything ever happen with this? 
--
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


--
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] Attach comments to functions' parameters and return value

[Jim Nasby]

On 9/15/15 12:35 AM, Charles Clavadetscher wrote:

COMMENT ON FUNCTION function_name ( [ [ argmode ] [ argname ] argtype [,
...] ] ) PARAMETER param_position IS 'text';

COMMENT ON FUNCTION function_name ( [ [ argmode ] [ argname ] argtype [,
...] ] ) RETURN VALUE IS 'text';

An alternative to "RETURN VALUE" could be "RETURNS", which would make
the statement shorter, but I think this may be confusing.


I like RETURN VALUE better myself.


The parameter list of the function is only required to identify the
function also in cases it exists with the same name in different
flavours. This sticks to the general syntax of the command and should be
easy to understand.


Right. You should be able to use the current COMMENT ON code. Note 
however that the last time I looked it does NOT support the full syntax 
that CREATE FUNCTION does. It'd be nice to fix that, but that's mostly a 
separate matter.


Though, it would probably be nice if all of this stuff (along with the 
regprocedure input function) could be factored into a single piece of 
code...

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


--
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] Equivalence Class Filters

[Jim Nasby]

On 12/7/15 7:26 PM, David Rowley wrote:

I was talking to Thomas Munro yesterday about this, and he mentioned
that it would be quite nice to have some stats on how much time is spent
in the planner, vs executor. He came up with the idea of adding a column
to pg_stat_statements to record the planning time.


I think that would be useful. Maybe something in pg_stat_database too.


If we could get real statistics on real world cases and we discovered
that, for example on average that the total CPU time of planning was
only 1% of execution time, then it would certainly make adding 2%
overhead onto the planner a bit less of a worry, as this would just be
%2 of 1% (0.02%). Such information, if fed back into the community might
be able to drive us in the right direction when it comes to deciding
what needs to be done to resolve this constant issue with accepting
planner improvement patches.


Might be nice, but I think it's also pretty unnecessary.

I've dealt with dozens of queries that took minutes to hours to run. Yet 
I can't recall ever having an EXPLAIN on one of these take more than a 
few seconds. I tend to do more OLTP stuff so maybe others have 
experienced long-running EXPLAIN, in which case it'd be great to know that.


Actually, I have seen long EXPLAIN times, but only as part of trying to 
aggressively increase *_collapse_limit. IIRC I was able to increase one 
of those to 14 and one to 18 before plan time became unpredictably bad 
(it wasn't always bad though, just sometimes).



I believe that with parallel query on the horizon for 9.6 that we're now
aiming to support bigger OLAP type database than ever before. So if we
ignore patches like this one then it appears that we have some
conflicting goals in the community as it seems that we're willing to add
the brawn, but we're not willing to add the brain. If this is the case
then it's a shame, as I think we can have both. So I very much agree on
the fact that we must find a way to maintain support and high
performance of small OLTP databases too.


+1
--
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


--
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] Equivalence Class Filters

[Jim Nasby]

On 12/8/15 3:52 AM, Jeremy Harris wrote:

On 07/12/15 16:44, Simon Riggs wrote:

There are many optimizations we might adopt, yet planning time is a factor.
It seems simple enough to ignore more complex optimizations if we have
already achieved a threshold cost (say 10). Such a test would add nearly
zero time for the common case. We can apply the optimizations in some kind
of ordering depending upon the cost, so we are careful to balance the
cost/benefit of trying certain optimizations.


Given parallelism, why not continue planning after initiating a
a cancellable execution, giving a better plan to be used if the
excecution runs for long enough?


Because that would take significantly more work than what Simon is 
proposing.


That said, I think the ability to restart with a different plan is 
something we might need, for cases when we discover the plan estimates 
were way off. If that ever gets built it might be useful for what you 
propose as well.

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


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

Re: Fwd: [HACKERS] Another little thing about psql wrapped expanded output

[Jim Nasby]

On 12/8/15 1:36 PM, Robert Haas wrote:

Your point is also valid, so I don't mean to detract from that.  But
the status quo is definitely annoying.


+1, and I even use -S.
--
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


--
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] Size of Path nodes

[Jim Nasby]

On 12/4/15 11:50 AM, Tom Lane wrote:

which means Robert has already blown the planner's space consumption out
by close to a factor of 2, and I should stop worrying.  Or else he should
start worrying.  Do we really need this field?  Did anyone pay any
attention to what happened to planner space consumption and performance
when the parallel-scan patch went in?  Or am I just too conditioned by
working with last-century hardware, and I should stop caring about how
large these nodes are?


I suspect Cachegrind[1] would answer a lot of these questions (though 
I've never actually used it). I can't get postgres to run under valgrind 
on my laptop, but maybe someone that's been successful at valgrind can 
try cachegrind (It's just another mode of valgrind).


[1] http://valgrind.org/docs/manual/cg-manual.html
--
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


--
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] Size of Path nodes

[Jim Nasby]

On 12/4/15 5:14 PM, Peter Geoghegan wrote:

On Fri, Dec 4, 2015 at 2:44 PM, Jim Nasby<jim.na...@bluetreble.com>  wrote:

>I suspect Cachegrind[1] would answer a lot of these questions (though I've
>never actually used it). I can't get postgres to run under valgrind on my
>laptop, but maybe someone that's been successful at valgrind can try
>cachegrind (It's just another mode of valgrind).

I've used Cachegrind, and think it's pretty good. You still need a
test case that exercises what you're interested in, though.
Distributed costs are really hard to quantify. Sometimes that's
because they don't exist, and sometimes it's because they can only be
quantified as part of a value judgement.


If we had a good way to run cachegrind (and maybe if it was run 
automatically somewhere) then at least we'd know what effect a patch had 
on things. (For those not familiar, there is a tool for diffing too 
cachegrind runs). Knowing is half the battle and all that.


Another interesting possibility would be a standardized perf test. [1] 
makes an argument for that.


Maybe a useful way to set stuff like this up would be to create support 
for things to run in travis-ci. Time-based tools presumably would be 
useless, but something doing analysis like cachegrind would probably be 
OK (though I think they do cap test runs to an hour or something).


[1] https://news.ycombinator.com/item?id=8426302
--
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


--
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 IS (NOT) DISTINCT to subquery_Op

[Jim Nasby]

On 12/10/15 7:03 PM, Tom Lane wrote:

Jim Nasby <jim.na...@bluetreble.com> writes:

Is there any reason we couldn't/shouldn't support IS DISTINCT in
subquery_Op? (Or really, just add support to ANY()/ALL()/(SELECT ...)?)


It's not an operator (in the sense of something with a pg_operator OID),
which means this would be quite a bit less than trivial as far as internal
representation/implementation goes.  I'm not sure if there would be
grammar issues, either.


make_distinct_op() simply calls make_op() and then changes the tag of 
the result node to T_DistinctExpr. So I was hoping something similar 
could be done for ANY/ALL?

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


--
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] Disabling an index temporarily

[Jim Nasby]

On 12/16/15 12:15 AM, Jeff Janes wrote:

But also, while loading 1.5 million records into a table with 250
million records is horribly, rebuilding all the indexes on a 251.5
million record table from scratch is even more horrible.  I don't know
if suspending maintenance (either globally or just for one session)
and then doing a bulk fix-up would be less horrible, but would be
willing to give it a test run.


I would think that's something completely different though, no? If 
you're doing that wouldn't you want other inserting/updating backends to 
still maintain the index, and only do something special in the backend 
that's doing the bulk load? Otherwise the bulk load would have to wait 
for all running backends to finish to ensure that no one was using the 
index. That's ugly enough for CIC; I can't fathom it working in any 
normal batch processing.


(Doing a single bulk insert to the index at the end of an INSERT should 
be safe though because none of those tuples are visible yet, though I'd 
have to make sure your backend didn't try to use the index for anything 
while the command was running... like as part of a trigger.)

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


--
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: custom compression methods

[Jim Nasby]

On 12/16/15 7:03 AM, Tomas Vondra wrote:


While versioning or increasing the 1GB limit are interesting, they have
nothing to do with this particular patch. (BTW I don't see how the
versioning would work at varlena level - that's something that needs to
be handled at data type level).


Right, but that's often going to be very hard to do and still support 
pg_upgrade. It's not like most types have spare bits laying around. 
Granted, this still means non-varlena types are screwed.



I think the only question we need to ask here is whether we want to
allow mixed compression for a column. If no, we're OK with the current
header. This is what the patch does, as it requires a rewrite after
changing the compression method.


I think that is related to the other items though: none of those other 
items (except maybe the 1G limit) seem to warrant dorking with varlena, 
but if there were 3 separate features that could make use of support for 
8 byte varlena then perhaps it's time to invest in that effort. 
Especially since IIRC we're currently out of bits, so if we wanted to 
change this we'd need to do it at least a release in advance so there 
was a version that would expand 4 byte varlena to 8 byte as needed.



And we're not painting ourselves in the corner - if we decide to
increase the varlena header size in the future, this patch does not make
it any more complicated.


True.
--
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


--
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] Remove array_nulls?

[Jim Nasby]

On 12/16/15 6:01 PM, Robert Haas wrote:

On Tue, Dec 15, 2015 at 1:26 AM, Michael Paquier
<michael.paqu...@gmail.com> wrote:

On Tue, Dec 15, 2015 at 2:57 AM, Jim Nasby <jim.na...@bluetreble.com> wrote:

On 12/11/15 2:57 PM, Tom Lane wrote:

Jim Nasby <jim.na...@bluetreble.com> writes:
Perhaps, but I'd like to have a less ad-hoc process about it.  What's
our policy for dropping backwards-compatibility GUCs?  Are there any
others that should be removed now as well?



Perhaps it should be tied to bumping the major version number, which I'm
guessing would happen next whenever we get parallel query execution. If we
do that, a reasonable policy might be that a compatability GUC lives across
no more than 1 major version bump (ie, we wouldn't remove something in 9.0
that was added in 8.4).


Another possibility may be to link that with the 5-year maintenance
window of community: a compatibility GUC is dropped in the following
major release if the oldest stable version maintained is the one that
introduced it. Just an idea.


Yeah, there's something to be said for that, although to be honest in
most cases I'd prefer to wait longer.   I wonder about perhaps
planning to drop things after two lifecycles.  That is, when the
release where the incompatibility was added goes out of support, we
don't do anything, but when the release that was current when it went
out of support is itself out of support, then we drop the GUC.  For
example, 8.2 went EOL in December 2011, at which point the newest
release was 9.1.  So when 9.1 is out of support, then we could drop
it; that's due to happen this September.  So 9.6 (or 10.0, if we call
it that) could drop it.


IIUC, that means supporting backwards compat. GUCs for 10 years, which 
seems a bit excessive. Granted, that's about the worse-case scenario for 
what I proposed (ie, we'd still be supporting 8.0 stuff right now).


The reason I thought of tying it to "major major" release is just 
because those generate even more notice and attract more users than 
normal, so it'd be nice to clean house before doing one. Perhaps I'm 
just introducing complexity that there's no need for.


If we don't want to tie "major major" number, then I think we should 
just go with the normal support cycle.

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


--
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] Fwd: [GENERAL] pgxs/config/missing is... missing

[Jim Nasby]

On 12/11/15 6:25 PM, Jim Nasby wrote:

On 12/10/15 7:09 PM, Tom Lane wrote:

Jim Nasby <jim.na...@bluetreble.com> writes:

AFAICT the problem is that missing wasn't included in install or
uninstall in config/Makefile. Attached patch fixes that, and results in
missing being properly installed in lib/pgxs/config.


I thought we'd more or less rejected that approach in the previous
thread.


David Wheeler and I worked on a way to work around this in the pgTap
extension, but AFAICT there's a bug here. The FreeBSD packages seems to
be built without having PERL on the system, so if you try and use it
with PGXS to set PERL, you end up with

PERL = /bin/sh
/usr/local/lib/postgresql/pgxs/src/makefiles/../../config/missing perl

which is coming out of the PGXS makefile. And that would work fine, if
we were actually installing config/missing.

If instead of installing config/missing we want to just drop that file
completely we can do that, but then we should remove it from sorce and
from the makefiles.


Grr, right after sending this I found the thread you were talking about.

I'm not really sure our missing is better than just letting the error 
bubble up. If folks think that's better then lets just rip missing out 
entirely.


If we do decide to keep missing, we should probably clarify it's 
messages to indicate that the relevant file was missing when *configure 
was run*.

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


--
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] Fwd: [GENERAL] pgxs/config/missing is... missing

[Jim Nasby]

On 12/10/15 7:09 PM, Tom Lane wrote:

Jim Nasby <jim.na...@bluetreble.com> writes:

AFAICT the problem is that missing wasn't included in install or
uninstall in config/Makefile. Attached patch fixes that, and results in
missing being properly installed in lib/pgxs/config.


I thought we'd more or less rejected that approach in the previous thread.


David Wheeler and I worked on a way to work around this in the pgTap 
extension, but AFAICT there's a bug here. The FreeBSD packages seems to 
be built without having PERL on the system, so if you try and use it 
with PGXS to set PERL, you end up with


PERL = /bin/sh
/usr/local/lib/postgresql/pgxs/src/makefiles/../../config/missing perl

which is coming out of the PGXS makefile. And that would work fine, if 
we were actually installing config/missing.


If instead of installing config/missing we want to just drop that file 
completely we can do that, but then we should remove it from sorce and 
from the makefiles.

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


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

[HACKERS] Remove array_nulls?

[Jim Nasby]

A quick doc search indicates this config was created in 9.0, though the 
docs state it's for a change that happened in 8.2[1]. Both versions are 
now supported, and 8.2 is obviously ancient.


Is it time to remove this GUC?

[1] http://www.postgresql.org/docs/9.0/static/runtime-config-compatible.html
--
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


--
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] Logical replication and multimaster

[Jim Nasby]

On 12/13/15 7:37 AM, David Fetter wrote:

As I understand it, pushing these into a library has been proposed but
not rejected.  That it hasn't happened yet is mostly about the lack of
tuits (the round ones) to rewrite the functionality as libraries and
refactor pg_dump/pg_restore to use only calls to same.  As usual, it's
less about writing the code and more about the enormous amount of
testing any such a refactor would entail.


My understanding as well. IIRC Jon Erdman brought this question up a 
couple years ago and the response was "It'd probably be accepted, it's 
just that no one has done the work."



I believe that refactoring much of pg_dump's functionality for the
current version of the server into SQL-accessible functions and making
pg_dump use only those functions is achievable with available
resources.

Such a refactor need not be all-or-nothing.  For example, the
dependency resolution stuff is a first step that appears to be worth
doing by itself even if the effort then pauses, possibly for some
time.


If someone wanted to spend time on this, I suspect it'd be worth looking 
at how bad some of the backward compatibility issues would be if done in 
the server. Maybe they wouldn't be that bad. I suspect the audience for 
this code would be much larger if it was in the server as opposed to a C 
library.

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


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

[HACKERS] Fwd: [GENERAL] pgxs/config/missing is... missing

[Jim Nasby]

Full story below, but in short:


I see that there is a config/missing script in source, and that 
Makefile.global.in references it:


> src/Makefile.global.in:missing= $(SHELL) 
$(top_srcdir)/config/missing


AFAICT the problem is that missing wasn't included in install or 
uninstall in config/Makefile. Attached patch fixes that, and results in 
missing being properly installed in lib/pgxs/config.



 Forwarded Message 
Subject: [GENERAL] pgxs/config/missing is... missing
Date: Wed, 28 Oct 2015 12:54:54 -0500
From: Jim Nasby <jim.na...@bluetreble.com>
To: pgsql-general <pgsql-gene...@postgresql.org>
CC: David E. Wheeler <da...@justatheory.com>

I'm trying to install pgTAP on a FreeBSD machine and running into an odd
problem:


sed -e 's,MODULE_PATHNAME,$libdir/pgtap,g' -e 's,__OS__,freebsd,g' -e 
's,__VERSION__,0.95,g' sql/pgtap-core.sql > sql/pgtap-core.tmp
/bin/sh /usr/local/lib/postgresql/pgxs/src/makefiles/../../config/missing perl 
compat/gencore 0 sql/pgtap-core.tmp > sql/pgtap-core.sql
cannot open /usr/local/lib/postgresql/pgxs/src/makefiles/../../config/missing: 
No such file or directory
Makefile:124: recipe for target 'sql/pgtap-core.sql' failed


That particular recipe is


sql/pgtap-core.sql: sql/pgtap.sql.in
cp $< $@
sed -e 's,sql/pgtap,sql/pgtap-core,g' compat/install-8.4.patch | patch 
-p0
sed -e 's,sql/pgtap,sql/pgtap-core,g' compat/install-8.3.patch | patch 
-p0
sed -e 's,MODULE_PATHNAME,$$libdir/pgtap,g' -e 's,__OS__,$(OSNAME),g' -e 
's,__VERSION__,$(NUMVERSION),g' sql/pgtap-core.sql > sql/pgtap-core.tmp
$(PERL) compat/gencore 0 sql/pgtap-core.tmp > sql/pgtap-core.sql
rm sql/pgtap-core.tmp


and it's the $(PERL) that's failing. If I add this recipe

print-%  : ; @echo $* = $($*)

and do

gmake print-PERL

I indeed get

PERL = /bin/sh
/usr/local/lib/postgresql/pgxs/src/makefiles/../../config/missing perl

even after explicitly exporting PERL=/usr/local/bin/perl

I see that there is a config/missing script in source, and that
Makefile.global.in references it:


src/Makefile.global.in:missing= $(SHELL) 
$(top_srcdir)/config/missing


Any ideas why it's not being installed, or why the PGXS Makefile is
ignoring/over-riding $PERL?
--
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


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


diff --git a/config/Makefile b/config/Makefile
index da12838..67e7998 100644
--- a/config/Makefile
+++ b/config/Makefile
@@ -7,9 +7,11 @@ include $(top_builddir)/src/Makefile.global
 
 install: all installdirs
$(INSTALL_SCRIPT) $(srcdir)/install-sh 
'$(DESTDIR)$(pgxsdir)/config/install-sh'
+   $(INSTALL_SCRIPT) $(srcdir)/missing 
'$(DESTDIR)$(pgxsdir)/config/missing'
 
 installdirs:
$(MKDIR_P) '$(DESTDIR)$(pgxsdir)/config'
 
 uninstall:
rm -f '$(DESTDIR)$(pgxsdir)/config/install-sh'
+   rm -f '$(DESTDIR)$(pgxsdir)/config/missing'

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

[HACKERS] Add IS (NOT) DISTINCT to subquery_Op

[Jim Nasby]

Is there any reason we couldn't/shouldn't support IS DISTINCT in 
subquery_Op? (Or really, just add support to ANY()/ALL()/(SELECT ...)?)

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


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

[HACKERS] array_remove(anyarray, anyarray)

[Jim Nasby]

Recently I had need of removing occurrences of a number of values from 
an array. Obviously I could have nested array_remove() call or wrapped 
the whole thing in a SELECT unnest(), but that seems rather silly and 
inefficient.


Any one have objections to changing array_replace_internal() to make 
search and repalace arrays instead of Datums?

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


--
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] Remove array_nulls?

[Jim Nasby]

On 12/11/15 2:57 PM, Tom Lane wrote:

Jim Nasby <jim.na...@bluetreble.com> writes:

A quick doc search indicates this config was created in 9.0, though the
docs state it's for a change that happened in 8.2[1].


Don't know what you're looking at, but the GUC is definitely there (and
documented) in 8.2.


Is it time to remove this GUC?


Perhaps, but I'd like to have a less ad-hoc process about it.  What's
our policy for dropping backwards-compatibility GUCs?  Are there any
others that should be removed now as well?


Perhaps it should be tied to bumping the major version number, which I'm 
guessing would happen next whenever we get parallel query execution. If 
we do that, a reasonable policy might be that a compatability GUC lives 
across no more than 1 major version bump (ie, we wouldn't remove 
something in 9.0 that was added in 8.4).

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


--
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: custom compression methods

[Jim Nasby]

On 12/14/15 12:50 AM, Craig Ringer wrote:

The issue with per-Datum is that TOAST claims two bits of a varlena
header, which already limits us to 1 GiB varlena values, something
people are starting to find to be a problem. There's no wiggle room to
steal more bits. If you want pluggable compression you need a way to
store knowledge of how a given datum is compressed with the datum or
have a fast, efficient way to check.


... unless we allowed for 8 byte varlena headers. Compression changes 
themselves certainly don't warrant that, but if people are already 
unhappy with 1GB TOAST then maybe that's enough.


The other thing this might buy us are a few bits that could be used to 
support Datum versioning for other purposes, such as when the binary 
format of something changes. I would think that at some point we'll need 
that for pg_upgrade.

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


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

[HACKERS] Clarify vacuum verbose message

[Jim Nasby]

VACUUM VERBOSE spits out two different messages for the heap, one of 
which is rather confusing:


INFO:  "trades": removed 625664 row versions in 20967 pages
...
INFO:  "trades": found 3282 removable, 56891627 nonremovable row 
versions in 1986034 out of 1986034 pages


After discussion with RhodiumToad I think I now understand how this can 
happen:


20:00 < RhodiumToad> the LP_DEAD slot is where the index entries for the 
deleted row point to, so that has to stay
20:01 < RhodiumToad> so for example, if you delete a lot of rows, then 
try and do a lot of updates (which will hint the

 pages as needing pruning),
20:01 < RhodiumToad> then do more updates or a seqscan (to let prune 
look at the pages),
20:02 < RhodiumToad> then do a vacuum, the vacuum will see a lot of 
LP_DEAD slots to remove index entries for, but not

 actual tuples

This example is from a table that was VACUUM FULL'd this weekend and had 
a nightly batch process run last night. That process INSERTs a bunch of 
rows and then does a bunch of UPDATEs on different subsets of those 
rows. I don't believe there would have been a large amount of deletes; 
I'll check with them tomorrow.


IMHO we need to change the messages so they are explicit about line 
pointers vs actual tuples. Trying to obfuscate that just leads to 
confusion. heap_page_prune needs to report only non-rootlp tuples that 
were pruned. (None of the other callers care about the return value.)

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


--
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.5RC1 wraps *today*

[Jim Nasby]

On 12/15/15 7:31 AM, Tom Lane wrote:

Accordingly, the release team has decided to wrap 9.5RC1 today, Tuesday,
for announcement Friday the 18th.  I'll start making the tarballs around
5PM (2200 UTC).  Sorry for the short notice, but there's no better way.


Since the 18th is my birthday, I think this is actually a good luck sign. ;P
--
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


--
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] Disabling an index temporarily

[Jim Nasby]

On 12/13/15 9:27 PM, Tom Lane wrote:

Corey Huinker<corey.huin...@gmail.com>  writes:

>So, I'd propose we following syntax:
>ALTER INDEX foo SET DISABLED
>-- does the SET indisvalid = false shown earlier.

This is exactly*not*  what Tatsuo-san was after, though; he was asking
for a session-local disable, which I would think would be by far the more
common use-case.  It's hard for me to see much of a reason to disable an
index globally while still paying all the cost to maintain it.  Seems to
me the typical work flow would be more like "disable index in a test
session, try all your queries and see how well they work, if you conclude
you don't need the index then drop it".


Both have value.

Sometimes the only realistic way to test this is to disable the index 
server-wide and see if anything blows up. Actually, in my experience, 
that's far more common than having some set of queries you can test 
against and call it good.


FWIW, I also don't see the use case for disabling maintenance on an 
index. Just drop it and if you know you'll want to recreate it squirrel 
away pg_get_indexdef() before you do.

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


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

[HACKERS] 9.5 BLOCKER: regrole and regnamespace and quotes

[Jim Nasby]

I didn't see this discussed on the thread...

regrole and regnamespace don't run their output through quote_ident(). 
That's contrary to all the other reg* operators.


Worse, they also don't *allow* quoted input. Not only is that different 
from reg*, it's the *opposite*:


select 'with spaces'::regclass;
ERROR:  invalid name syntax
LINE 1: select 'with spaces'::regclass;
select '"with spaces"'::regclass;
   regclass
---
 "with spaces"
(1 row)

I think this needs to be fixed before 9.5 releases. :(
--
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


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

[HACKERS] Very confusing installcheck behavior with PGXS

[Jim Nasby]

The rule that gets executed if you do `make installcheck` with something 
using PGXS is


pgxs.mk:$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)

where $(pg_regress_installcheck) is set in Makefile.global.in to


pg_regress_installcheck = $(top_builddir)/src/test/regress/pg_regress 
--inputdir=$(srcdir) --bindir='$(bindir)' $(pg_regress_locale_flags) 
$(EXTRA_REGRESS_OPTS)


The problem here is that in a PGXS make, srcdir is set to '.'[1], and 
--inputdir is specified a second time in REGRESS_OPTS. Normally that 
works OK (for some reason ignoring what's in ./sql), but if you happen 
to have a file in your test/sql directory that matches a file in ./sql, 
pg_regress runs the first file and not the second.


Presumably that's exactly what you'd want in most of the tree, but it's 
definitely not what you want in an extension.


Is the best way to fix this to add a pg_regress_installcheck_pgxs 
variable in Makefile.global.in and modify pgxs.mk accordingly?


[1]:

decibel@decina:[16:18]~/git/trunklet (master=)$make 
print-pg_regress_installcheck print-REGRESS_OPTS print-REGRESS
pg_regress_installcheck = 
/Users/decibel/pgsql/9.4/i/lib/pgxs/src/makefiles/../../src/test/regress/pg_regress
 --inputdir=./ --psqldir=/Users/decibel/pgsql/9.4/i/bin
REGRESS_OPTS = --inputdir=test --load-language=plpgsql 
--dbname=contrib_regression
REGRESS = all build

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


--
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] count_nulls(VARIADIC "any")

[Jim Nasby]

On 1/3/16 2:37 PM, Pavel Stehule wrote:

+   /* num_nulls(VARIADIC NULL) is defined as NULL */
+   if (PG_ARGISNULL(0))
+   return false;


Could you add to the comment explaining why that's the desired behavior?


+   /*
+* Non-null argument had better be an array.  We assume that 
any call
+* context that could let get_fn_expr_variadic return true will 
have
+* checked that a VARIADIC-labeled parameter actually is an 
array.  So
+* it should be okay to just Assert that it's an array rather 
than
+* doing a full-fledged error check.
+*/
+   
Assert(OidIsValid(get_base_element_type(get_fn_expr_argtype(fcinfo->flinfo, 
0;


Erm... is that really the way to verify that what you have is an array? 
ISTM there should be a macro for that somewhere...



+   /* OK, safe to fetch the array value */
+   arr = PG_GETARG_ARRAYTYPE_P(0);
+
+   ndims = ARR_NDIM(arr);
+   dims = ARR_DIMS(arr);
+   nitems = ArrayGetNItems(ndims, dims);
+
+   bitmap = ARR_NULLBITMAP(arr);
+   if (bitmap)
+   {
+   bitmask = 1;
+
+   for (i = 0; i < nitems; i++)
+   {
+   if ((*bitmap & bitmask) == 0)
+   count++;
+
+   bitmask <<= 1;
+   if (bitmask == 0x100)
+   {
+   bitmap++;
+   bitmask = 1;


For brevity and example sake it'd probably be better to just use the 
normal iterator, unless there's a serious speed difference?


In the unit test, I'd personally prefer just building a table with the 
test cases and the expected NULL/NOT NULL results, at least for all the 
calls that would fit that paradigm. That should significantly reduce the 
size of the test. Not a huge deal though...


Also, I don't think anything is testing multiples of whatever value... 
how 'bout change the generate_series CASE statement to >40 instead of <>40?

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


--
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.5 BLOCKER: regrole and regnamespace and quotes

[Jim Nasby]

On 1/3/16 9:23 PM, Tom Lane wrote:

Jim Nasby <jim.na...@bluetreble.com> writes:

regrole and regnamespace don't run their output through quote_ident().
That's contrary to all the other reg* operators.
Worse, they also don't *allow* quoted input. Not only is that different
from reg*, it's the *opposite*:


BTW, there's a concrete reason why this is broken, which is that although
regrole and regnamespace didn't bother with copying quoting/dequoting from
the other types, they *did* copy the special case logic about allowing
and emitting numeric OIDs.  This means that an output like "1234" from
regroleout is formally inconsistent: there is no way to tell if it's an
numeric OID or a role name that happens to be all digits.  (With proper
quoting logic, you could tell because an all-digits role name would have
gotten quoted.)  Conversely, if you create an all-digits role name, there
is no way to get regrolein to interpret it as such, whereas a dequoting
rule would have disambiguated.

I'm inclined to leave to_regrole and to_regnamespace alone, though, since
they have no numeric-OID path, and they will provide an "out" for anyone
who wants to handle nonquoted names.  (Though at least in HEAD we ought to
fix them to take type text as input.  Using cstring for ordinary functions
is just sloppy.)


None of the other to_reg* casts do that though, so this would be 
inconsistent.


Another potential problem for regnamespace is that it doesn't allow an 
entry for the catalog. I'm not sure what the spec says about that, but 
every other function allows dbname.schema.blah (dbname == catalog).


I started working on a fix, but it's currently blowing up in bootstrap 
and I haven't been able to figure out why yet:


running bootstrap script ... FATAL:  improper qualified name (too many 
dotted names): oid_ops

--
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
diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c
index 8b105fe..a555a1f 100644
--- a/src/backend/catalog/namespace.c
+++ b/src/backend/catalog/namespace.c
@@ -2613,6 +2613,9 @@ TSConfigIsVisible(Oid cfgid)
  * extract the schema name and object name.
  *
  * *nspname_p is set to NULL if there is no explicit schema name.
+ * 
+ * If *objname_p is not set then treat names as a schema name, possibly with a
+ * catalog name.
  */
 void
 DeconstructQualifiedName(List *names,
@@ -2623,19 +2626,21 @@ DeconstructQualifiedName(List *names,
char   *schemaname = NULL;
char   *objname = NULL;
 
-   switch (list_length(names))
+   switch (list_length(names) + objname_p ? 0 : 1)
{
case 1:
objname = strVal(linitial(names));
break;
case 2:
schemaname = strVal(linitial(names));
-   objname = strVal(lsecond(names));
+   if (objname_p)
+   objname = strVal(lsecond(names));
break;
case 3:
catalogname = strVal(linitial(names));
schemaname = strVal(lsecond(names));
-   objname = strVal(lthird(names));
+   if (objname_p)
+   objname = strVal(lthird(names));
 
/*
 * We check the catalog name and then ignore it.
@@ -2655,7 +2660,8 @@ DeconstructQualifiedName(List *names,
}
 
*nspname_p = schemaname;
-   *objname_p = objname;
+   if (objname_p)
+   *objname_p = objname;
 }
 
 /*
diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c
index 59e5dc8..8c862cf 100644
--- a/src/backend/utils/adt/regproc.c
+++ b/src/backend/utils/adt/regproc.c
@@ -1569,6 +1569,7 @@ Datum
 regrolein(PG_FUNCTION_ARGS)
 {
char   *role_name_or_oid = PG_GETARG_CSTRING(0);
+   List   *names;
Oid result;
 
/* '-' ? */
@@ -1586,7 +1587,15 @@ regrolein(PG_FUNCTION_ARGS)
}
 
/* Normal case: see if the name matches any pg_authid entry. */
-   result = get_role_oid(role_name_or_oid, false);
+   names = stringToQualifiedNameList(role_name_or_oid);
+
+   if (list_length(names) > 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("improper qualified name (too many dotted 
names): %s",
+  NameListToString(names;
+
+   result = get_role_oid(strVal(linitial(names)), false);
 
PG_RETURN_OID(result);
 }
@@ -1668,7 +1677,9 @@ Datum
 regnamespacein(PG_FUNCTION_ARGS)
 {
char   *nsp_name_or_oid = PG_GETARG_CSTRING(0);
+   char   *nsp_name;

Re: [HACKERS] 9.5 BLOCKER: regrole and regnamespace and quotes

[Jim Nasby]

On 1/3/16 10:37 PM, Tom Lane wrote:

It's not a release stopper, but I plan to fix it in HEAD whenever I have
an idle hour.


Since I'm sure there's much better things you can be working on I was 
going to just do this myself. Then it occurred to me that this should be 
a great item for a new hacker to handle, so I'm going to figure out what 
we're doing with those things now-a-days and put it there.


If no one picks it up I'll get it into the last commitfest.
--
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


--
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] Accessing non catalog table in backend

[Jim Nasby]

On 1/4/16 12:53 PM, Atri Sharma wrote:
Please don't top-post.

On 5 Jan 2016 12:20 am, "Jim Nasby" <jim.na...@bluetreble.com
<mailto:jim.na...@bluetreble.com>> wrote:

On 1/4/16 12:07 PM, Atri Sharma wrote:

Hi All,

I wanted to check if it is possible to query a non catalog table
in backend.

I understand that cql interface is only for catalog querying
hence it is
not usable for this purpose per se.


AFAIK it's possible to do it with low level routines, but I think
unless you have a really good reason to go that route you're much
better off just using SPI. If it's good enough for plpgsql... :)


> I was wary to use SPI inside the executor for node evaluation functions.
> Does it seem safe?

Oh, inside the executor? Yeah, I doubt attempting SPI there would end 
well...


What exactly are you trying to do?
--
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


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

[HACKERS] Beginner hacker item: Fix to_reg*() input type

[Jim Nasby]

All the to_reg* functions in src/backend/utils/adt/regproc.c currently 
accept a cstring. Per [1], they should accept text. This should be a 
fairly simple change to make:


- Modify the functions in regproc.c. Take a look at how other text input 
functions work to see what needs to happen here (you'll want to use 
text_to_cstring() as part of that.)


- Modify the appropriate entries in src/include/catalog/pg_proc.h

[1] http://www.postgresql.org/message-id/29618.1451882...@sss.pgh.pa.us. 
Note that I mistakenly referred to reg*in functions in that email; it's 
the to_reg* functions that need to change.

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


--
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.5 BLOCKER: regrole and regnamespace and quotes

[Jim Nasby]

On 1/3/16 10:20 PM, Jim Nasby wrote:

What I went with. Now to figure out why this is happening...


Nevermind, see my stupidity now. Should have full patch soon.
--
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


--
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.5 BLOCKER: regrole and regnamespace and quotes

[Jim Nasby]

On 1/3/16 11:05 PM, Michael Paquier wrote:

I'll take a look, but Michael, if you have time to review, an extra set
>of eyeballs wouldn't hurt.  There is no margin for error right now.

I'm just on it:)  Will update in a couple of minutes, I noticed some
stuff in Jim's patch.


BTW, in case you miss it... I was inconsistent with the list 
length_names checks... one is


if (list_length(names) > 1)

the other is

if (list_length(names) != 1)

(stringToQualifiedNameList() can't actually return a 0 length list and 
IIRC there was another place doing a > check.)

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


--
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.5 BLOCKER: regrole and regnamespace and quotes

[Jim Nasby]

On 1/3/16 10:46 PM, Jim Nasby wrote:

Added. I'm gonna call this good for now. Note this is just against HEAD
since I don't have 9.5 setup yet. Presumably the patch should still
apply...


BTW, in case it's helpful... 
https://github.com/decibel/postgres/tree/regquote

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


--
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.5 BLOCKER: regrole and regnamespace and quotes

[Jim Nasby]

On 1/3/16 11:24 PM, Michael Paquier wrote:

Sorry, didn't realize you were on it.

No worries. I know it's already late where you are.


And late where 9.5 is... ;)


I would use != 1 instead here, even if the function is strict.


Yeah, I effectively pulled the pattern from DeconstructQualifiedName, 
but != 1 is better.



You forgot to update the regression test output. And actually on

Doh.


second thoughts the tests you added do not actually bring much value
because what is tested is just the behavior of
stringToQualifiedNameList, and the other reg* are not tested that as


Seemed good to test what the original bug was, but sure.


well. However I think that we had better test the failures of
regnamespace and regrole when more than 1 element is parsed as this is
a new particular code path. Attached is an updated patch which passes
check-world in my environment.


Patch looks good to me. FWIW, RhodiumToad and macdice looked at my patch 
as well and didn't see any problems you didn't mention.

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


--
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.5 BLOCKER: regrole and regnamespace and quotes

[Jim Nasby]

On 1/3/16 10:26 PM, Michael Paquier wrote:

Thanks, this is more or less what I... just did..


Sorry, didn't realize you were on it.


+result = get_namespace_oid(nsp_name, false);
This is incorrect, you should use strVal(linitial(names)) instead.


Yup. Dur.


+if (list_length(names) > 1)
+ereport(ERROR,
+(errcode(ERRCODE_SYNTAX_ERROR),
+errmsg("improper qualified name (too many dotted names): %s",
+   NameListToString(names;
I would just mark that as "Invalid syntax".


Just noticed this... I just copied the same syntax used elsewhere... 
whoever commits feel free to editorialize...



A couple of tests in regproc.sql would be a good addition as well.


Added. I'm gonna call this good for now. Note this is just against HEAD 
since I don't have 9.5 setup yet. Presumably the patch should still apply...

--
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
diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c
index 59e5dc8..529d692 100644
--- a/src/backend/utils/adt/regproc.c
+++ b/src/backend/utils/adt/regproc.c
@@ -1569,6 +1569,7 @@ Datum
 regrolein(PG_FUNCTION_ARGS)
 {
char   *role_name_or_oid = PG_GETARG_CSTRING(0);
+   List   *names;
Oid result;
 
/* '-' ? */
@@ -1586,7 +1587,15 @@ regrolein(PG_FUNCTION_ARGS)
}
 
/* Normal case: see if the name matches any pg_authid entry. */
-   result = get_role_oid(role_name_or_oid, false);
+   names = stringToQualifiedNameList(role_name_or_oid);
+
+   if (list_length(names) > 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("improper qualified name (too many dotted 
names): %s",
+  NameListToString(names;
+
+   result = get_role_oid(strVal(linitial(names)), false);
 
PG_RETURN_OID(result);
 }
@@ -1600,9 +1609,18 @@ Datum
 to_regrole(PG_FUNCTION_ARGS)
 {
char   *role_name = PG_GETARG_CSTRING(0);
+   List   *names;
Oid result;
 
-   result = get_role_oid(role_name, true);
+   names = stringToQualifiedNameList(role_name);
+
+   if (list_length(names) > 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("improper qualified name (too many dotted 
names): %s",
+  NameListToString(names;
+
+   result = get_role_oid(strVal(linitial(names)), true);
 
if (OidIsValid(result))
PG_RETURN_OID(result);
@@ -1668,6 +1686,7 @@ Datum
 regnamespacein(PG_FUNCTION_ARGS)
 {
char   *nsp_name_or_oid = PG_GETARG_CSTRING(0);
+   List   *names;
Oid result = InvalidOid;
 
/* '-' ? */
@@ -1685,7 +1704,15 @@ regnamespacein(PG_FUNCTION_ARGS)
}
 
/* Normal case: see if the name matches any pg_namespace entry. */
-   result = get_namespace_oid(nsp_name_or_oid, false);
+   names = stringToQualifiedNameList(nsp_name_or_oid);
+
+   if (list_length(names) != 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("improper qualified name (too many dotted 
names): %s",
+  NameListToString(names;
+
+   result = get_namespace_oid(strVal(linitial(names)), false);
 
PG_RETURN_OID(result);
 }
@@ -1699,9 +1726,18 @@ Datum
 to_regnamespace(PG_FUNCTION_ARGS)
 {
char   *nsp_name = PG_GETARG_CSTRING(0);
+   List   *names;
Oid result;
 
-   result = get_namespace_oid(nsp_name, true);
+   names = stringToQualifiedNameList(nsp_name);
+
+   if (list_length(names) != 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("improper qualified name (too many dotted 
names): %s",
+  NameListToString(names;
+
+   result = get_namespace_oid(strVal(linitial(names)), true);
 
if (OidIsValid(result))
PG_RETURN_OID(result);
diff --git a/src/test/regress/sql/regproc.sql b/src/test/regress/sql/regproc.sql
index 8edaf15..cb427dc 100644
--- a/src/test/regress/sql/regproc.sql
+++ b/src/test/regress/sql/regproc.sql
@@ -14,7 +14,9 @@ SELECT regprocedure('abs(numeric)');
 SELECT regclass('pg_class');
 SELECT regtype('int4');
 SELECT regrole('regtestrole');
+SELECT regrole('"regtestrole"');
 SELECT regnamespace('pg_catalog');
+SELECT regnamespace('"pg_catalog"');
 
 SELECT to_regoper('||/');
 SELECT to_regoperator('+(int4,int4)')

Re: [HACKERS] 9.5 BLOCKER: regrole and regnamespace and quotes

[Jim Nasby]

On 1/3/16 9:43 PM, Tom Lane wrote:

Jim Nasby <jim.na...@bluetreble.com> writes:

On 1/3/16 9:23 PM, Tom Lane wrote:
Another potential problem for regnamespace is that it doesn't allow an
entry for the catalog. I'm not sure what the spec says about that, but
every other function allows dbname.schema.blah (dbname == catalog).


Meh, these types conform to no spec, so we can make them do what we
like.  I see about zero reason to allow a catalog name, it would mostly
just confuse people.


Ok. Not like CREATE SCHEMA allows that anyway.


I started working on a fix, but it's currently blowing up in bootstrap
and I haven't been able to figure out why yet:
running bootstrap script ... FATAL:  improper qualified name (too many
dotted names): oid_ops


Recommendation: don't muck with DeconstructQualifiedName.  That is called
in too many places and we are *not* touching any such API twenty-four
hours before release wrap.


Yeah, looks like that's what was blowing up.


There's no real advantage to that anyway, compared with just doing
stringToQualifiedNameList and then complaining if its list_length isn't 1.


What I went with. Now to figure out why this is happening...

  SELECT regnamespace('pg_catalog');
! ERROR:  schema "Y" does not exist
! LINE 1: SELECT regnamespace('pg_catalog');
!
--
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
diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c
index 59e5dc8..339bfe7 100644
--- a/src/backend/utils/adt/regproc.c
+++ b/src/backend/utils/adt/regproc.c
@@ -1569,6 +1569,7 @@ Datum
 regrolein(PG_FUNCTION_ARGS)
 {
char   *role_name_or_oid = PG_GETARG_CSTRING(0);
+   List   *names;
Oid result;
 
/* '-' ? */
@@ -1586,7 +1587,15 @@ regrolein(PG_FUNCTION_ARGS)
}
 
/* Normal case: see if the name matches any pg_authid entry. */
-   result = get_role_oid(role_name_or_oid, false);
+   names = stringToQualifiedNameList(role_name_or_oid);
+
+   if (list_length(names) > 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("improper qualified name (too many dotted 
names): %s",
+  NameListToString(names;
+
+   result = get_role_oid(strVal(linitial(names)), false);
 
PG_RETURN_OID(result);
 }
@@ -1668,7 +1677,9 @@ Datum
 regnamespacein(PG_FUNCTION_ARGS)
 {
char   *nsp_name_or_oid = PG_GETARG_CSTRING(0);
+   char   *nsp_name;
Oid result = InvalidOid;
+   List   *names;
 
/* '-' ? */
if (strcmp(nsp_name_or_oid, "-") == 0)
@@ -1685,7 +1696,15 @@ regnamespacein(PG_FUNCTION_ARGS)
}
 
/* Normal case: see if the name matches any pg_namespace entry. */
-   result = get_namespace_oid(nsp_name_or_oid, false);
+   names = stringToQualifiedNameList(nsp_name_or_oid);
+
+   if (list_length(names) != 1)
+   ereport(ERROR,
+   (errcode(ERRCODE_SYNTAX_ERROR),
+   errmsg("improper qualified name (too many dotted 
names): %s",
+  NameListToString(names;
+
+   result = get_namespace_oid(nsp_name, false);
 
PG_RETURN_OID(result);
 }

-- 
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.5 BLOCKER: regrole and regnamespace and quotes

[Jim Nasby]

On 1/3/16 9:23 PM, Tom Lane wrote:

(Though at least in HEAD we ought to
fix them to take type text as input.  Using cstring for ordinary functions
is just sloppy.)


BTW, *all* the reg*in() functions do that...
--
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


--
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] count_nulls(VARIADIC "any")

[Jim Nasby]

On 1/3/16 10:23 PM, Pavel Stehule wrote:

Hi

2016-01-03 22:49 GMT+01:00 Jim Nasby <jim.na...@bluetreble.com
<mailto:jim.na...@bluetreble.com>>:

On 1/3/16 2:37 PM, Pavel Stehule wrote:

+   /* num_nulls(VARIADIC NULL) is defined as NULL */
+   if (PG_ARGISNULL(0))
+   return false;


Could you add to the comment explaining why that's the desired behavior?


This case should be different than num_nulls(VARIADIC ARRAY[..]) - this
situation is really equivalent of missing data and NULL is correct
answer. It should not be too clean in num_nulls, but when it is cleaner
for num_notnulls. And more, it is consistent with other variadic
functions in Postgres: see concat_internal and text_format.


Makes sense, now that you explain it. Which is why I'm thinking it'd be 
good to add that explanation to the comment... ;)



  
Assert(OidIsValid(get_base_element_type(get_fn_expr_argtype(fcinfo->flinfo, 
0;


Erm... is that really the way to verify that what you have is an
array? ISTM there should be a macro for that somewhere...


really, it is. It is used more time. Although I am not against some
macro, I don't think so it is necessary. The macro should not be too
shorter than this text.


Well, if there's other stuff doing that... would be nice to refactor 
that though.



For brevity and example sake it'd probably be better to just use the
normal iterator, unless there's a serious speed difference?


The iterator does some memory allocations and some access to type cache.
Almost all work of iterator is useless for this case. This code is
developed by Marko, but I agree with this design. Using the iterator is
big gun for this case. I didn't any performance checks, but it should be
measurable  for any varlena arrays.


Makes sense then.
--
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


--
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] Improved error reporting in format()

[Jim Nasby]

On 1/2/16 5:57 PM, Jim Nasby wrote:

Attached patch clarifies that %-related error messages with hints as
well as (IMHO) improving the clarity of the message:


Sorry, forgot to update regression tests. New patch attached.
--
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
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index 8683188..9583ed0 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -4647,7 +4647,8 @@ text_reverse(PG_FUNCTION_ARGS)
if (++(ptr) >= (end_ptr)) \
ereport(ERROR, \

(errcode(ERRCODE_INVALID_PARAMETER_VALUE), \
-errmsg("unterminated format 
specifier"))); \
+errmsg("unterminated format() type 
specifier"), \
+errhint("For a single \"%%\" use 
\"\"."))); \
} while (0)
 
 /*
@@ -4779,8 +4780,9 @@ text_format(PG_FUNCTION_ARGS)
if (strchr("sIL", *cp) == NULL)
ereport(ERROR,

(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-errmsg("unrecognized conversion type 
specifier \"%c\"",
-   *cp)));
+errmsg("unrecognized format() type 
specifier \"%c\"",
+   *cp),
+errhint("For a single \"%%\" use 
\"\".")));
 
/* If indirect width was specified, get its value */
if (widthpos >= 0)
@@ -4791,7 +4793,7 @@ text_format(PG_FUNCTION_ARGS)
if (arg >= nargs)
ereport(ERROR,

(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-errmsg("too few arguments for 
format")));
+errmsg("too few arguments for 
format()")));
 
/* Get the value and type of the selected argument */
if (!funcvariadic)
@@ -4899,8 +4901,9 @@ text_format(PG_FUNCTION_ARGS)
/* should not get here, because of previous 
check */
ereport(ERROR,

(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("unrecognized conversion type 
specifier \"%c\"",
-*cp)));
+ errmsg("unrecognized format() type 
specifier \"%c\"",
+*cp),
+ errhint("For a single \"%%\" use 
\"\".")));
break;
}
}
diff --git a/src/test/regress/expected/text.out 
b/src/test/regress/expected/text.out
index 1587046..a74b9ce 100644
--- a/src/test/regress/expected/text.out
+++ b/src/test/regress/expected/text.out
@@ -211,7 +211,8 @@ ERROR:  too few arguments for format
 select format('Hello %s');
 ERROR:  too few arguments for format
 select format('Hello %x', 20);
-ERROR:  unrecognized conversion type specifier "x"
+ERROR:  unrecognized format() type specifier "x"
+HINT:  For a single "%" use "%%".
 -- check literal and sql identifiers
 select format('INSERT INTO %I VALUES(%L,%L)', 'mytab', 10, 'Hello');
  format 
@@ -263,9 +264,11 @@ ERROR:  format specifies argument 0, but arguments are 
numbered from 1
 select format('%*0$s', 'Hello');
 ERROR:  format specifies argument 0, but arguments are numbered from 1
 select format('%1$', 1);
-ERROR:  unterminated format specifier
+ERROR:  unterminated format() type specifier
+HINT:  For a single "%" use "%%".
 select format('%1$1', 1);
-ERROR:  unterminated format specifier
+ERROR:  unterminated format() type specifier
+HINT:  For a single "%" use "%%".
 -- check mix of positional and ordered placeholders
 select format('Hello %s %1$s %s', 'World', 'Hello again');
 format 

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

[HACKERS] Improved error reporting in format()

[Jim Nasby]

The current error message for an invalid format conversion type is 
extremely confusing except in the simplest of uses:


select format( '% moo');
ERROR:  unrecognized conversion type specifier " "

Obviously in that example you can figure out what's going on, but 
frequently format() is used in a complex context where it's not at all 
obvious that format is the problem. Even worse, "conversion" makes it 
sound like a localization issue.


Attached patch clarifies that %-related error messages with hints as 
well as (IMHO) improving the clarity of the message:


select format( '% moo');
ERROR:  unrecognized format() type specifier " "
HINT:  For a single "%" use "%%"

I also made the use of "format()" consistent in all the other error 
messages.

--
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
diff --git a/src/backend/utils/adt/varlena.c b/src/backend/utils/adt/varlena.c
index a89f586..a4552ad 100644
--- a/src/backend/utils/adt/varlena.c
+++ b/src/backend/utils/adt/varlena.c
@@ -4647,7 +4647,8 @@ text_reverse(PG_FUNCTION_ARGS)
if (++(ptr) >= (end_ptr)) \
ereport(ERROR, \

(errcode(ERRCODE_INVALID_PARAMETER_VALUE), \
-errmsg("unterminated format 
specifier"))); \
+errmsg("unterminated format() type 
specifier"), \
+errhint("For a single \"%%\" use 
\"\"."))); \
} while (0)
 
 /*
@@ -4779,8 +4780,9 @@ text_format(PG_FUNCTION_ARGS)
if (strchr("sIL", *cp) == NULL)
ereport(ERROR,

(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-errmsg("unrecognized conversion type 
specifier \"%c\"",
-   *cp)));
+errmsg("unrecognized format() type 
specifier \"%c\"",
+   *cp),
+errhint("For a single \"%%\" use 
\"\".")));
 
/* If indirect width was specified, get its value */
if (widthpos >= 0)
@@ -4791,7 +4793,7 @@ text_format(PG_FUNCTION_ARGS)
if (arg >= nargs)
ereport(ERROR,

(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-errmsg("too few arguments for 
format")));
+errmsg("too few arguments for 
format()")));
 
/* Get the value and type of the selected argument */
if (!funcvariadic)
@@ -4899,8 +4901,9 @@ text_format(PG_FUNCTION_ARGS)
/* should not get here, because of previous 
check */
ereport(ERROR,

(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("unrecognized conversion type 
specifier \"%c\"",
-*cp)));
+ errmsg("unrecognized format() type 
specifier \"%c\"",
+*cp),
+ errhint("For a single \"%%\" use 
\"\".")));
break;
}
}

-- 
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 schema-qualified relnames in constraint error messages.

[Jim Nasby]

On 1/5/16 9:16 PM, Tom Lane wrote:

Jim Nasby <jim.na...@bluetreble.com> writes:

FWIW, I suspect very few people know about the verbosity setting (I
didn't until a few months ago...) Maybe psql should hint about it the
first time an error is reported in a session.


Actually, what'd be really handy IMO is something to regurgitate the
most recent error in verbose mode, without making a permanent session
state change.  Something like

regression=# insert into bar values(1);
ERROR:  insert or update on table "bar" violates foreign key constraint 
"bar_f1_fkey"
DETAIL:  Key (f1)=(1) is not present in table "foo".
regression=# \saywhat
ERROR:  23503: insert or update on table "bar" violates foreign key constraint 
"bar_f1_fkey"
DETAIL:  Key (f1)=(1) is not present in table "foo".
SCHEMA NAME:  public
TABLE NAME:  bar
CONSTRAINT NAME:  bar_f1_fkey
LOCATION:  ri_ReportViolation, ri_triggers.c:3326
regression=#

Not sure how hard that would be to do within psql's current structure.


At first glance, it looks like it just means changing AcceptResult() to 
use PQresultErrorField in addition to PQresultErrorMessage, and stuffing 
the results somewhere. And of course adding \saywhat to the psql parser, 
but maybe someone more versed in psql code can verify that.


If it is that simple, looks like another good beginner hacker task. :)
--
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


--
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 schema-qualified relnames in constraint error messages.

[Jim Nasby]

On 1/5/16 8:41 PM, Tom Lane wrote:

Jim Nasby<jim.na...@bluetreble.com>  writes:

>does psql do anything with those fields? ISTM the biggest use for this
>info is someone sitting at psql or pgAdmin.

Sure, if you turn up the error verbosity.


FWIW, I suspect very few people know about the verbosity setting (I 
didn't until a few months ago...) Maybe psql should hint about it the 
first time an error is reported in a session.

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


--
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 schema-qualified relnames in constraint error messages.

[Jim Nasby]

On 1/5/16 7:21 PM, Tom Lane wrote:

What seems more likely to be useful and to not break anything is to push
forward on adding PG_DIAG_SCHEMA_NAME and similar auxiliary fields to more
error messages (see errtable() and siblings).  That would allow
applications to extract this information automatically and reliably.
Only after that work is complete and there's been a reasonable amount of
time for clients to start relying on it can we really start thinking about
whacking the message texts around.


+1, but...

does psql do anything with those fields? ISTM the biggest use for this 
info is someone sitting at psql or pgAdmin.


Maybe schema info could be presented in HINT or DETAIL messages as well?
--
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


--
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] No Issue Tracker - Say it Ain't So!

[Jim Nasby]

On 1/6/16 9:22 PM, Michael Paquier wrote:

On Thu, Jan 7, 2016 at 9:28 AM, Tom Lane <t...@sss.pgh.pa.us> wrote:

Jim Nasby <jim.na...@bluetreble.com> writes:

Somewhat related to that, I don't believe there's any reason why commit
fest managers need to be committers; it seems like the job is really
just about reading through email activity to see where things are at.


They don't need to be.


More thoughts on that. I would even think the contrary, someone who
has just monitored for 1/2 years -hackers and knowing how things are
handled would be able to do this job as well, the only issue being to
juggle with many threads at the same time, to be sure that each patch
status is correct, and to not lose motivation during the CF. The last
one is the hardest by far.


Sorry, I inadvertently used 'committer' when I should have said 'coder'. 
There's nothing code related about CFM, and I think it's a dis-service 
to the community to have coders serving that role.

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


--
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] No Issue Tracker - Say it Ain't So!

[Jim Nasby]

On 1/6/16 6:18 PM, Greg Stark wrote:

On Wed, Jan 6, 2016 at 11:42 PM, Jim Nasby <jim.na...@bluetreble.com> wrote:

Right. Personally, I feel the TODO has pretty much outlived it's usefulness.
An issue tracker would make maintaining items like this a lot more
reasonable, but it certainly wouldn't be free.


Eh, a bug tracker that tracks actual bugs would be useful, I don't
think anyone would argue with that. A vague "issue" tracker that just
collects ideas people have had that seemed like a good idea at some
time in history would suffer exactly the same problem the TODO has.


I think one of the biggest hurdles people face is getting the community 
to agree that something is a desired feature. So if there was a list of 
things that the community had agreed would be good I think that itself 
would be useful. Even better if items had a rough outline of the work 
necessary.


Obviously that won't do too much for really big features. But if our 
experienced hackers focused less on coding and more on design and 
creating smaller tasks that people could work on, more people could 
potentially be put to work.


ISTM that the design work needs to be done and documented no matter 
what, so there shouldn't be much overhead there. The overhead would be 
in maintaining the tracker and making sure folks were actively getting 
stuff done. That can be done by a non-coder. That means it shouldn't 
really cost the community much in terms of current resources, as long as 
we attract new people to take on these new tasks.

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


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

[HACKERS] BEGINNER HACKERS: array_remove(anyarray, anyarray)

[Jim Nasby]

On 12/10/15 6:25 PM, Jim Nasby wrote:

Recently I had need of removing occurrences of a number of values from
an array. Obviously I could have nested array_remove() call or wrapped
the whole thing in a SELECT unnest(), but that seems rather silly and
inefficient.

Any one have objections to changing array_replace_internal() to make
search and replace arrays instead of Datums?


Seeing none... let's see if someone wants to tackle this.

The goal is to modify array_replace_internal() (in 
src/backend/utils/adt/arrayfuncs.c) so that two of it's arguments 
(search and replace) can be arrays. I believe the best way to do that 
would be to add bool search_isarray and bool replace_isarray arguments, 
and have array_replace_internal() act accordingly if those are true.


The expected behavior when search_isarray is true would be to search for 
a group of elements that completely match the search array. When 
replace_isarray is true, then any replacements into *array would be the 
elements from replace, growing or shrinking the array as necessary.


The user visible array_remove and array_replace functions will need to 
check the data type of their inputs using get_fn_expr_argtype(). You can 
find examples of that in arrayfuncs.c.


A complete patch will also need regression tests 
(src/test/regress/sql/arrays.sql) and tweaks to the documentation for 
those functions (doc/src/sgml/func.sgml).

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


--
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] Very confusing installcheck behavior with PGXS

[Jim Nasby]

On 1/7/16 8:47 AM, Tom Lane wrote:

Jim Nasby <jim.na...@bluetreble.com> writes:

However, if I do this:
mv test/sql/acl_type.sql test/sql/acl.sql
mv test/expected/acl_type.out test/expected/acl.out



And change acl_type to acl in that pg_regress command:
/Users/decibel/pgsql/HEAD/i/lib/pgxs/src/makefiles/../../src/test/regress/pg_regress
--bindir='/Users/decibel/pgsql/HEAD/i/bin' --inputdir=test
--load-language=plpgsql --dbname=contrib_regression acl build compat rights



Instead of executing test/sql/acl.sql, it executes ./sql/acl.sql.


That's pretty hard to believe.  There's nothing in pg_regress that looks
in places other than the given --inputdir.


Actually, I think it does... from pg_regress_main.c:

/*
 * Look for files in the output dir first, consistent with a vpath 
search.
 * This is mainly to create more reasonable error messages if the file 
is
 * not found.  It also allows local test overrides when running 
pg_regress
 * outside of the source tree.
 */
snprintf(infile, sizeof(infile), "%s/sql/%s.sql",
 outputdir, testname);
if (!file_exists(infile))
snprintf(infile, sizeof(infile), "%s/sql/%s.sql",
 inputdir, testname);

If I add --outputdir=test, then everything works fine.

Obviously I can just deal with this in my Makefile, but is this really 
the behavior we want? It certainly seems to violate POLA...



I wonder whether you have a test/input/acl.sql and/or test/output/acl.out
that's confusing matters.


Nope...

decibel@decina:[08:35]~/git/pg_acl (master *%=)$ll test
total 16
drwxr-x---   6 decibel  staff  204 Jan  2 17:31 ./
drwxr-x---  17 decibel  staff  578 Jan  7 08:35 ../
-rw-r-   1 decibel  staff   65 Jan  2 17:31 deps.sql
drwxr-x---   6 decibel  staff  204 Jan  7 08:32 expected/
lrwxr-x---   1 decibel  staff   25 Dec 26 13:43 pgxntool@ -> 
../pgxntool/test/pgxntool

drwxr-x---   6 decibel  staff  204 Jan  7 08:32 sql/
decibel@decina:[08:48]~/git/pg_acl (master *%=)$


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


--
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] Very confusing installcheck behavior with PGXS

[Jim Nasby]

On 1/6/16 11:54 AM, Tom Lane wrote:

Robert Haas <robertmh...@gmail.com> writes:

On Sun, Jan 3, 2016 at 5:22 PM, Jim Nasby <jim.na...@bluetreble.com> wrote:

The rule that gets executed if you do `make installcheck` with something
using PGXS is

pgxs.mk:$(pg_regress_installcheck) $(REGRESS_OPTS) $(REGRESS)

where $(pg_regress_installcheck) is set in Makefile.global.in to


pg_regress_installcheck = $(top_builddir)/src/test/regress/pg_regress
--inputdir=$(srcdir) --bindir='$(bindir)' $(pg_regress_locale_flags)
$(EXTRA_REGRESS_OPTS)


The problem here is that in a PGXS make, srcdir is set to '.'[1], and
--inputdir is specified a second time in REGRESS_OPTS. Normally that works
OK (for some reason ignoring what's in ./sql), but if you happen to have a
file in your test/sql directory that matches a file in ./sql, pg_regress
runs the first file and not the second.



Stupid question time: why in the world would you have that?


It doesn't seem that odd to have a test file that matches an extension 
file...



AFAICS, the pieces we supply (Makefile.global and pgxs.mk) would only
specify --inputdir once.  If there's a second specification being added
to REGRESS_OPTS by your own Makefile, that would override the default,
which seems like entirely sensible behavior.  Maybe I'm misunderstanding
something, but it sounds like you're saying "if I write --inputdir=test
in REGRESS_OPTS, it runs the tests in test/sql not those in ./sql".
Why would you not think that's expected?


Actually, it's more bizarre than I thought...

This is what my makefile[1] produces, which currently works:
/Users/decibel/pgsql/HEAD/i/lib/pgxs/src/makefiles/../../src/test/regress/pg_regress 
--inputdir=./ --bindir='/Users/decibel/pgsql/HEAD/i/bin' 
--inputdir=test --load-language=plpgsql --dbname=contrib_regression 
acl_type build compat rights


Removing the first --inputdir also works:
/Users/decibel/pgsql/HEAD/i/lib/pgxs/src/makefiles/../../src/test/regress/pg_regress 
 --bindir='/Users/decibel/pgsql/HEAD/i/bin'--inputdir=test 
--load-language=plpgsql --dbname=contrib_regression acl_type build 
compat rights


However, if I do this:
mv test/sql/acl_type.sql test/sql/acl.sql
mv test/expected/acl_type.out test/expected/acl.out

And change acl_type to acl in that pg_regress command:
/Users/decibel/pgsql/HEAD/i/lib/pgxs/src/makefiles/../../src/test/regress/pg_regress 
--bindir='/Users/decibel/pgsql/HEAD/i/bin' --inputdir=test 
--load-language=plpgsql --dbname=contrib_regression acl build compat rights


Instead of executing test/sql/acl.sql, it executes ./sql/acl.sql.

I'd assumed this was due to the extra --inputdir option, but clearly 
something else is going on here.


Time to gvim pg_regress.c...

[1] https://github.com/decibel/pg_acl
--
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


--
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] Very confusing installcheck behavior with PGXS

[Jim Nasby]

On 1/7/16 9:12 AM, Tom Lane wrote:

Jim Nasby <jim.na...@bluetreble.com> writes:

On 1/7/16 8:47 AM, Tom Lane wrote:

That's pretty hard to believe.  There's nothing in pg_regress that looks
in places other than the given --inputdir.



Actually, I think it does... from pg_regress_main.c:



/*
 * Look for files in the output dir first, consistent with a vpath 
search.
 * This is mainly to create more reasonable error messages if the file 
is
 * not found.  It also allows local test overrides when running 
pg_regress
 * outside of the source tree.
 */
snprintf(infile, sizeof(infile), "%s/sql/%s.sql",
 outputdir, testname);
if (!file_exists(infile))
snprintf(infile, sizeof(infile), "%s/sql/%s.sql",
 inputdir, testname);


Oh, I'd just been looking in pg_regress.c.

The comment's argument about "more reasonable error messages" seems pretty
dubious to me.  The real reason for this behavior seems to have been to
simplify VPATH builds --- see commit feae7856, which added this code,
and was able to get rid of quite a lot of makefile cruft.

I think though that doing it exactly like this is a bit klugy.  A better
implementation of VPATH would've been to introduce an optional "secondary
input directory" into which we look if we fail to find something in the
main input directory.  Then we'd run it with main input directory in the
build tree and secondary input directory pointing to the source tree.


Seems reasonable. Sounds like a good beginner project to boot. :)


(I'm also wondering how convert_sourcefiles() works at all in a vpath
build, considering that I don't see it doing anything like this ...)


It's only looking at outputdir, which I suspect is never ambiguous.
--
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


--
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] No Issue Tracker - Say it Ain't So!

[Jim Nasby]

On 1/6/16 2:18 PM, Robert Haas wrote:

On Wed, Jan 6, 2016 at 10:43 AM, Catalin Iacob <iacobcata...@gmail.com> wrote:

I also think a list of small things suitable for new contributors
would help attracting them. Not all would stick and go on to larger
items but hopefully at least some would.


I agree with this.  Curating such a list is a fair amount of work that
somebody's got to do, though.  The TODO list is full of an awful lot
of things that don't matter and missing an awful lot of things that
do.


Right. Personally, I feel the TODO has pretty much outlived it's 
usefulness. An issue tracker would make maintaining items like this a 
lot more reasonable, but it certainly wouldn't be free.


Something else to consider though: I sent one email and the task was 
done in 24 hours. For things that can be well defined and are fairly 
mechanical, I suspect an email to hackers with a big [NEW HACKER] banner 
would do wonders.


Related to that is JD's offer to donate staff time to infrastructure 
work. There's probably a fair amount of things that could be "farmed 
out" that way. Some folks in the community proper would still need to 
keep tabs on things, but they wouldn't have to do the gruntwork. If, 
say, the Ops teams at 2nd Quadrant, CMD, and EDB wanted to work together 
on improving infrastructure, that's pretty much community at that point, 
and not a dependence on a single external entity.

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


--
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] Very confusing installcheck behavior with PGXS

[Jim Nasby]

On 1/7/16 9:56 AM, Tom Lane wrote:

Jim Nasby <jim.na...@bluetreble.com> writes:

On 1/7/16 9:12 AM, Tom Lane wrote:

(I'm also wondering how convert_sourcefiles() works at all in a vpath
build, considering that I don't see it doing anything like this ...)



It's only looking at outputdir, which I suspect is never ambiguous.


Eh, no, look again.  What it's actually doing is reading $inputdir/input/
and converting into $outputdir/sql/, and reading $inputdir/output/ and
converting into $outputdir/expected/.  I guess that works, but again it's
kind of at variance with the normal expectation of VPATH behavior.  What
you'd expect is that $builddir/input files would override $srcdir/input
files; but as is, $builddir/input and $builddir/output are never examined
at all.


Yeah, I just discovered the whole input/ and output/ bit. That really 
complicates things, because ISTM it's very common to directly specify 
both sql/ and expected/ files directly, and you'd certainly THINK that 
those files are input, and not output.


Which begs the question... how the hell do sql/ and expected/ ever work 
in a vpath build? AFAICT things are never copied from 
$inputdir/(sql|expected) to $outputdir...


Maybe it's just me, but this whole convention seems like a giant POLA 
violation. If pg_regress was only used in Postgres source maybe that 
wouldn't matter since presumably there's always an example to copy from 
(and presumably tests use either input/ and output/ OR sql/ and 
expected/, but never both). But pg_regress is used by contrib and now 
extensions, so it's got a much wider audience than just -hackers. :/


input and output are used in only 3 places in the whole tree[1], so 
maybe the best thing to do is just rip support for that out of 
pg_regress and handle it some other way.


Also worth noting: the only reason I'm using pg_regress is it's the 
easiest way to get a test cluster. If not for that, I'd just use 
pg_prove since I'm already using pgTap.


[1] find . \( -name input -o -name output \) -type d
./contrib/dblink/input
./contrib/dblink/output
./contrib/file_fdw/input
./contrib/file_fdw/output
./src/test/regress/input
./src/test/regress/output
--
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


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