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

[Rod Taylor]

On Thu, Feb 2, 2017 at 11:40 AM, Alvaro Herrera 
wrote:

> Pavel Stehule wrote:
>
> > Identification of unjoined tables should be very useful - but it is far
> to
> > original proposal - so it can be solved separately.
> >
> > This patch is simple - and usually we prefer more simple patches than one
> > bigger.
> >
> > Better to enhance this feature step by step.
>
> Agreed -- IMO this is a reasonable first step, except that I would
> rename the proposed extension so that it doesn't focus solely on the
> first step.  I'd pick a name that suggests that various kinds of checks
> are applied to queries, so "require_where" would be only one of various
> options that can be enabled.
>

A general SQL-Critic would be a very welcome extension.

Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Fabien COELHO]

Hello Corey,


The check I was suggesting on whether Ctrl+C has been pressed

on an empty line seems harder to implement, because get_interactive()
just calls readline() or fgets(), which block to return when a whole
line is ready. AFAICS psql can't know what was the edit-in-progress
when these functions are interrupted by a signal instead of
returning normally.
But I don't think this check is essential, it could be left to another
patch.


Glad I wasn't missing something obvious.
I suppose we could base the behavior on whether there's at least one full
line already buffered.
However, I agree that it can be left to another patch.


Hmmm. ISTM that control-c must at least reset the stack, otherwise their 
is not easy way to get out. What can be left to another patch is doing a 
control-C for contents and then another one for the stack when there is no 
content.


Comments about v6:


- error messages are now a bit more terse, following suggestions


Ok.


- help text is more terse and Conditionals section was moved below Input
Output


Ok.


- leverage IFSTATE_NONE a bit to fold some not-in-a-branch cases into
existing switch statements, giving flatter, slightly cleaner code and that
addresses expected cases before exceptional ones


Code looks ok.


- comments highlight which messages are printed in both interactive and
script mode.


Yep.


- put Fabien's tap test in place verbatim


Hmmm. That was really just a POC... I would suggest some more tests, eg:

  # elif error
  "\\if false\n\\elif error\n\\endif\n"

  # ignore commands on error (stdout must be empty)
  "\\if error\n\\echo NO\n\\else\n\\echo NO\n\\endif\n"

Also there is an empty line before the closing } of the while loop.


- No mention of Ctrl-C or PROMPT. Those can be addressed in separate
patches.


I think that Ctrl-C resetting the stack must be addressed in this patch. 
Trying to be more intelligent/incremental on Ctrl-C can wait for another 
time.



There's probably some more consensus building to do over the interactive
messages and comments,


Barking is now quite more verbose (?), but at least it is clear about the 
status and what is expected. I noticed that it is now always on, whether 
an error occured or not, which is ok with me.


and if interactive-ish tests are possible with TAP, we should add those 
too.


Good point. It seems that it is decided based on "source == stdin" plus 
checking whether both stdin/stdout are on terminal. Allowing to work 
around the later requires some more infrastructure to force "notty" (yuk, 
a negative variable tested negatively...) to false whatever, which does 
not seem to exist. So this is for another time.


--
Fabien.


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

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

[Michael Paquier]

On Sun, Feb 5, 2017 at 10:08 AM, Rod Taylor  wrote:
> A general SQL-Critic would be a very welcome extension.

Please no hyphen for extension names!
-- 
Michael


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

[HACKERS] Ignore tablespace ACLs when ignoring schema ACLs

[Noah Misch]

DefineIndex() has a check_rights argument that determines whether to perform a
namespace ACL check.  When ALTER TABLE ALTER TYPE rebuilds an index, it sets
that flag.  The theory goes that use of DROP INDEX and CREATE INDEX is a mere
implementation detail of ALTER TABLE ALTER TYPE; the operation is logically like
an alteration of the existing index.  I think the same treatment should extend
to the tablespace ACL check, as attached.
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index ed6136c..7fec099 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -291,8 +291,8 @@ CheckIndexCompatible(Oid oldId,
  * 'indexRelationId': normally InvalidOid, but during bootstrap can be
  * nonzero to specify a preselected OID for the index.
  * 'is_alter_table': this is due to an ALTER rather than a CREATE operation.
- * 'check_rights': check for CREATE rights in the namespace.  (This should
- * be true except when ALTER is deleting/recreating an index.)
+ * 'check_rights': check for CREATE rights in namespace and tablespace.  (This
+ * should be true except when ALTER is deleting/recreating an 
index.)
  * 'skip_build': make the catalog entries but leave the index file empty;
  * it will be filled later.
  * 'quiet': suppress the NOTICE chatter ordinarily provided for constraints.
@@ -433,8 +433,9 @@ DefineIndex(Oid relationId,
/* note InvalidOid is OK in this case */
}
 
-   /* Check permissions except when using database's default */
-   if (OidIsValid(tablespaceId) && tablespaceId != MyDatabaseTableSpace)
+   /* Check tablespace permissions */
+   if (check_rights &&
+   OidIsValid(tablespaceId) && tablespaceId != 
MyDatabaseTableSpace)
{
AclResult   aclresult;
 
diff --git a/src/test/regress/input/tablespace.source 
b/src/test/regress/input/tablespace.source
index 743c4b9..03a62bd 100644
--- a/src/test/regress/input/tablespace.source
+++ b/src/test/regress/input/tablespace.source
@@ -109,11 +109,18 @@ DROP TABLESPACE regress_tblspace;
 
 CREATE ROLE regress_tablespace_user1 login;
 CREATE ROLE regress_tablespace_user2 login;
+GRANT USAGE ON SCHEMA testschema TO regress_tablespace_user2;
 
 ALTER TABLESPACE regress_tblspace OWNER TO regress_tablespace_user1;
 
+CREATE TABLE testschema.tablespace_acl (c int);
+-- new owner lacks permission to create this index from scratch
+CREATE INDEX k ON testschema.tablespace_acl (c) TABLESPACE regress_tblspace;
+ALTER TABLE testschema.tablespace_acl OWNER TO regress_tablespace_user2;
+
 SET SESSION ROLE regress_tablespace_user2;
 CREATE TABLE tablespace_table (i int) TABLESPACE regress_tblspace; -- fail
+ALTER TABLE testschema.tablespace_acl ALTER c TYPE bigint;
 RESET ROLE;
 
 ALTER TABLESPACE regress_tblspace RENAME TO regress_tblspace_renamed;
diff --git a/src/test/regress/output/tablespace.source 
b/src/test/regress/output/tablespace.source
index 31f2ac0..aaedf5f 100644
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -221,10 +221,16 @@ DROP TABLESPACE regress_tblspace;
 ERROR:  tablespace "regress_tblspace" is not empty
 CREATE ROLE regress_tablespace_user1 login;
 CREATE ROLE regress_tablespace_user2 login;
+GRANT USAGE ON SCHEMA testschema TO regress_tablespace_user2;
 ALTER TABLESPACE regress_tblspace OWNER TO regress_tablespace_user1;
+CREATE TABLE testschema.tablespace_acl (c int);
+-- new owner lacks permission to create this index from scratch
+CREATE INDEX k ON testschema.tablespace_acl (c) TABLESPACE regress_tblspace;
+ALTER TABLE testschema.tablespace_acl OWNER TO regress_tablespace_user2;
 SET SESSION ROLE regress_tablespace_user2;
 CREATE TABLE tablespace_table (i int) TABLESPACE regress_tblspace; -- fail
 ERROR:  permission denied for tablespace regress_tblspace
+ALTER TABLE testschema.tablespace_acl ALTER c TYPE bigint;
 RESET ROLE;
 ALTER TABLESPACE regress_tblspace RENAME TO regress_tblspace_renamed;
 ALTER TABLE ALL IN TABLESPACE regress_tblspace_renamed SET TABLESPACE 
pg_default;
@@ -235,10 +241,11 @@ NOTICE:  no matching relations in tablespace 
"regress_tblspace_renamed" found
 -- Should succeed
 DROP TABLESPACE regress_tblspace_renamed;
 DROP SCHEMA testschema CASCADE;
-NOTICE:  drop cascades to 4 other objects
+NOTICE:  drop cascades to 5 other objects
 DETAIL:  drop cascades to table testschema.foo
 drop cascades to table testschema.asselect
 drop cascades to table testschema.asexecute
 drop cascades to table testschema.atable
+drop cascades to table testschema.tablespace_acl
 DROP ROLE regress_tablespace_user1;
 DROP ROLE regress_tablespace_user2;

-- 
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] Review: GIN non-intrusive vacuum of posting tree

[Jeff Davis]

On Sun, Jan 22, 2017 at 10:32 PM, Jeff Davis  wrote:
> On Sat, Jan 21, 2017 at 4:25 AM, Andrew Borodin  wrote:
> One idea I had that might be simpler is to use a two-stage page
> delete. The first stage would remove the link from the parent and mark
> the page deleted, but leave the right link intact and prevent
> recycling. The second stage would follow the chain of right links
> along each level, removing the right links to deleted pages and
> freeing the page to be recycled.

Do you think this approach is viable as a simplification?

Regards,
Jeff Davis


-- 
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] Index corruption with CREATE INDEX CONCURRENTLY

[Pavan Deolasee]

On Sat, Feb 4, 2017 at 11:54 PM, Tom Lane  wrote:

>
> Based on Pavan's comments, I think trying to force this into next week's
> releases would be extremely unwise.  If the bug went undetected this long,
> it can wait for a fix for another three months.


Yes, I think bug existed ever since and went unnoticed. One reason could be
that the race happens only when the new index turns HOT updates into
non-HOT updates. Another reason could be that we don't have checks in place
to catch these kinds of corruption. Having said that, since we have
discovered the bug, at least many 2ndQuadrant customers have expressed
worry and want to know if the fix will be available in 9.6.2 and other
minor releases.  Since the bug can lead to data corruption, the worry is
justified. Until we fix the bug, there will be a constant worry about using
CIC.

If we can have some kind of band-aid fix to plug in the hole, that might be
enough as well. I tested my first patch (which will need some polishing)
and that works well AFAIK. I was worried about prepared queries and all,
but that seems ok too. RelationGetIndexList() always get called within
ExecInitModifyTable. The fix seems quite unlikely to cause any other side
effects.

Another possible band-aid is to throw another relcache invalidation in CIC.
Like adding a dummy index_set_state_flags() within yet another transaction.
Seems ugly, but should fix the problem for now and not cause any impact on
relcache mechanism whatsoever.

That seems better than
> risking new breakage when it's barely 48 hours to the release wrap
> deadline.  We do not have time to recover from any mistakes.
>

I'm not sure what the project policies are, but can we consider delaying
the release by a week for issues like these? Or do you think it will be
hard to come up with a proper fix for the issue and it will need some
serious refactoring?

Thanks,
Pavan

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

[HACKERS] Variable name typo in launcher.c

[Masahiko Sawada]

Hi,

I think "laucher" should be "launcher". Attached patch fixes it.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


fix_typo_launcher_c.patch
Description: Binary data

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

Re: [HACKERS] Index corruption with CREATE INDEX CONCURRENTLY

[Pavan Deolasee]

On Sat, Feb 4, 2017 at 12:10 PM, Amit Kapila 
wrote:

>
>
> If we do above, then I think primary key attrs won't be returned
> because for those we are using relation copy rather than an original
> working copy of attrs. See code below:
>
> switch (attrKind)
> {
> ..
> case INDEX_ATTR_BITMAP_PRIMARY_KEY:
> return bms_copy(relation->rd_pkattr);
>
>
I don't think that would a problem since if relation_rd_indexattr is NULL,
primary key attrs will be recomputed and returned.


>
> Apart from above, I think after the proposed patch, it will be quite
> possible that in heap_update(), hot_attrs, key_attrs and id_attrs are
> computed using different index lists (consider relcache flush happens
> after computation of hot_attrs or key_attrs) which was previously not
> possible.  I think in the later code we assume that all the three
> should be computed using the same indexlist.  For ex. see the below
> code:
>

This seems like a real problem to me. I don't know what the consequences
are, but definitely having various attribute lists to have different view
of the set of indexes doesn't seem right.

The problem we are trying to fix is very clear by now. Relcache flush
clears rd_indexvalid and rd_indexattr together, but because of the race,
rd_indexattr is recomputed with the old information and gets cached again,
while rd_indexvalid (and rd_indexlist) remains unset. That leads to the
index corruption in CIC and may cause other issues too that we're not aware
right now. We want cached attribute lists to become invalid whenever index
list is invalidated and that's not happening right now.

So we have tried three approaches so far.

1. Simply reset rd_indexattr whenever we detect rd_indexvalid has been
reset. This was the first patch. It's a very simple patch and has worked
for me with CACHE_CLOBBER_ALWAYS and even fixes the CIC bug. The only
downside it seems that we're invalidating cached attribute lists outside
the normal relcache invalidation path. It also works on the premise that
RelationGetIndexList() will be called every time
before RelationGetIndexAttrBitmap() is called, otherwise we could still end
up using stale cached copies. I wonder if cached plans can be a problem
here.

2. In the second patch, we tried to recompute attribute lists if a relcache
flush happens in between and index list is invalidated. We've seen problems
with that, especially it getting into an infinite loop with
CACHE_CLOBBER_ALWAYS. Not clear why it happens, but apparently relcache
flushes keep happening.

3. We tried the third approach where we don't cache attribute lists if know
that index set has changed and hope that the next caller gets the correct
info. But Amit rightly identified problems with that approach too.

So we either need to find a 4th approach or stay with the first patch
unless we see a problem there too (cached plans, anything else). Or may be
fix CREATE INDEX CONCURRENTLY so that at least the first phase which add
the index entry to the catalog happens with a strong lock. This will lead
to momentary blocking of write queries, but protect us from the race. I'm
assuming this is the only code that can cause the problem, and I haven't
checked other potential hazards.

Ideas/suggestions?

Thanks,
Pavan

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

Re: [HACKERS] Vacuum: allow usage of more than 1GB of work mem

[Masahiko Sawada]

On Wed, Feb 1, 2017 at 11:55 PM, Claudio Freire  wrote:
> On Wed, Feb 1, 2017 at 6:13 PM, Masahiko Sawada  wrote:
>> On Wed, Feb 1, 2017 at 10:02 PM, Claudio Freire  
>> wrote:
>>> On Wed, Feb 1, 2017 at 5:47 PM, Masahiko Sawada  
>>> wrote:
 Thank you for updating the patch.

 Whole patch looks good to me except for the following one comment.
 This is the final comment from me.

 /*
  *  lazy_tid_reaped() -- is a particular tid deletable?
  *
  *  This has the right signature to be an IndexBulkDeleteCallback.
  *
  *  Assumes dead_tuples array is in sorted order.
  */
 static bool
 lazy_tid_reaped(ItemPointer itemptr, void *state)
 {
 LVRelStats *vacrelstats = (LVRelStats *) state;

 You might want to update the comment of lazy_tid_reaped() as well.
>>>
>>> I don't see the mismatch with reality there (if you consider
>>> "dead_tples array" in the proper context, that is, the multiarray).
>>>
>>> What in particular do you find out of sync there?
>>
>> The current lazy_tid_reaped just find a tid from a tid array using
>> bsearch but in your patch lazy_tid_reaped handles multiple tid arrays
>> and processing method become complicated. So I thought it's better to
>> add the description of this function.
>
> Alright, updated with some more remarks that seemed relevant

Thank you for updating the patch.

The patch looks good to me. There is no review comment from me.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center


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

Re: [HACKERS] pg_sequences bug ?

[Michael Paquier]

On Sat, Feb 4, 2017 at 2:50 PM, Shinoda, Noriyoshi
 wrote:
>  I tried a committed pg_sequences for PostgreSQL 10dev 
> (https://commitfest.postgresql.org/12/771/).
> I found that when multiple users create SEQUENCE, I cannot see the 
> pg_sequences catalog. I think that should work just like pg_tables.
>
> $ psql -U user1
> postgres=> CREATE SEQUENCE seq1 ;
> CREATE SEQUENCE
>
> $ psql -U user2
> postgres=> CREATE SEQUENCE seq2 ;
> CREATE SEQUENCE
> postgres=> SELECT * FROM pg_sequences ;
> ERROR:  permission denied for sequence seq1
>
> Apparently it seems that the pg_sequence_last_value function included in the 
> pg_sequences view definition cannot be executed.
> Is this behavior supposed?

That seems user-unfriendly to me.

We could perhaps just use has_sequence_privilege() and return NULL if
the caller of pg_sequences does not have select and usage access to a
given sequence? Please see the patch attached.
-- 
Michael
diff --git a/src/backend/catalog/system_views.sql b/src/backend/catalog/system_views.sql
index 28be27a07e..907e0fb630 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -175,7 +175,11 @@ CREATE OR REPLACE VIEW pg_sequences AS
 S.seqincrement AS increment_by,
 S.seqcycle AS cycle,
 S.seqcache AS cache_size,
-pg_sequence_last_value(C.oid) AS last_value
+CASE
+WHEN has_sequence_privilege(C.oid, 'SELECT,USAGE'::text)
+THEN pg_sequence_last_value(C.oid)
+ELSE NULL
+END AS last_value
 FROM pg_sequence S JOIN pg_class C ON (C.oid = S.seqrelid)
  LEFT JOIN pg_namespace N ON (N.oid = C.relnamespace)
 WHERE NOT pg_is_other_temp_schema(N.oid)
diff --git a/src/test/regress/expected/rules.out b/src/test/regress/expected/rules.out
index de5ae00970..d7a165eb42 100644
--- a/src/test/regress/expected/rules.out
+++ b/src/test/regress/expected/rules.out
@@ -1647,7 +1647,10 @@ pg_sequences| SELECT n.nspname AS schemaname,
 s.seqincrement AS increment_by,
 s.seqcycle AS cycle,
 s.seqcache AS cache_size,
-pg_sequence_last_value((c.oid)::regclass) AS last_value
+CASE
+WHEN has_sequence_privilege(c.oid, 'SELECT,USAGE'::text) THEN pg_sequence_last_value((c.oid)::regclass)
+ELSE NULL::bigint
+END AS last_value
FROM ((pg_sequence s
  JOIN pg_class c ON ((c.oid = s.seqrelid)))
  LEFT JOIN pg_namespace n ON ((n.oid = c.relnamespace)))

-- 
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] Constraint exclusion failed to prune partition in case of partition expression involves function call

[amul sul]

I see, thanks Amit.

Regards,
Amul

Sent from a mobile device. Please excuse brevity and tpyos.

[HACKERS] pg_restore foreign keys NOT VALID, or [assume] VALID; VALIDATE CONSTRAINT CONCURRENTLY

[Tomasz Ostrowski]

Hi.

A lot of time during pg_restore of a large database is spent on 
validating all the foreign keys. In contrast to importing data and 
creating indexes this operation does not parallelize well. So large 
percentage of parallel restore time ends up using single worker to 
validate foreign keys for the largest table.


If we'd have a option to restore the table without validating foreign 
keys and leaving them in NOT VALID state, the downtime needed for us to 
restore would decrease significantly.


If we'd also have an option to avoid blocking updates on the table 
during (potentially long) validating, for example:

  ALTER TABLE distributors VALIDATE CONSTRAINT CONCURRENTLY distfk;
Then we could postpone it and do it during normal operation of the 
database, out of precious disaster recovery time.


Alternatively maybe it should be allowed to do for example:
  ALTER TABLE distributor ADD CONSTRAINT distfk
FOREIGN KEY (address) REFERENCES addresses (address) VALID;
It would mean that the database should assume that this constraint is 
valid. Should be possible to turn it on using some pg_restore option (or 
pg_dump option when dumping to text format), though maybe only when 
restoring whole database, not single table. Though there's a possibility 
that a partially failed restore could leave database in inconsistent 
state. So I'd rather prefer the above option (NOT VALID + VALIDATE 
CONCURRENTLY).


Any comments on this? Does it look like a good idea? It shouldn't be 
hard to implement.


--
Tomasz "Tometzky" Ostrowski


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

Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Daniel Verite]

Corey Huinker wrote:

[about Ctrl-C]

> That does seem to be the consensus desired behavior. I'm just not sure
> where to handle that. The var "cancel_pressed" shows up in a lot of places.
> Advice?

Probably you don't need to care about cancel_pressed, and
the /if stack could be unwound at the point the SIGINT
handler longjumps to, in mainloop.c:

/* got here with longjmp */

/* reset parsing state */
psql_scan_finish(scan_state);
psql_scan_reset(scan_state);
resetPQExpBuffer(query_buf);
resetPQExpBuffer(history_buf);
count_eof = 0;
slashCmdStatus = PSQL_CMD_UNKNOWN;
prompt_status = PROMPT_READY;
pset.stmt_lineno = 1;
cancel_pressed = false;

The check I was suggesting on whether Ctrl+C has been pressed
on an empty line seems harder to implement, because get_interactive()
just calls readline() or fgets(), which block to return when a whole
line is ready. AFAICS psql can't know what was the edit-in-progress
when these functions are interrupted by a signal instead of
returning normally.
But I don't think this check is essential, it could be left to another patch.


Best regards,
-- 
Daniel Vérité
PostgreSQL-powered mailer: http://www.manitou-mail.org
Twitter: @DanielVerite


-- 
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] Provide list of subscriptions and publications in psql's completion

[Michael Paquier]

On Fri, Feb 3, 2017 at 3:56 AM, Peter Eisentraut
 wrote:
> On 2/2/17 12:48 AM, Michael Paquier wrote:
>> +#define Query_for_list_of_subscriptions \
>> +" SELECT pg_catalog.quote_ident(subname) "\
>> +"   FROM pg_catalog.pg_subscription "\
>> +"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"
>
> This query should also be qualified by current database.

Indeed. Here is an updated patch.
-- 
Michael
diff --git a/src/bin/psql/tab-complete.c b/src/bin/psql/tab-complete.c
index d6fffcf42f..0835a71bab 100644
--- a/src/bin/psql/tab-complete.c
+++ b/src/bin/psql/tab-complete.c
@@ -830,6 +830,18 @@ static const SchemaQuery Query_for_list_of_matviews = {
 "   FROM pg_catalog.pg_am "\
 "  WHERE substring(pg_catalog.quote_ident(amname),1,%d)='%s'"
 
+#define Query_for_list_of_subscriptions \
+" SELECT pg_catalog.quote_ident(c1.subname) "\
+"   FROM pg_catalog.pg_subscription c1, pg_catalog.pg_database c2"\
+"  WHERE substring(pg_catalog.quote_ident(c1.subname),1,%d)='%s'"\
+"AND c2.datname = pg_catalog.current_database()"\
+"AND c1.subdbid = c2.oid"
+
+#define Query_for_list_of_publications \
+" SELECT pg_catalog.quote_ident(pubname) "\
+"   FROM pg_catalog.pg_publication "\
+"  WHERE substring(pg_catalog.quote_ident(pubname),1,%d)='%s'"
+
 /* the silly-looking length condition is just to eat up the current word */
 #define Query_for_list_of_arguments \
 "SELECT pg_catalog.oidvectortypes(proargtypes)||')' "\
@@ -960,13 +972,13 @@ static const pgsql_thing_t words_after_create[] = {
 	{"OWNED", NULL, NULL, THING_NO_CREATE},		/* for DROP OWNED BY ... */
 	{"PARSER", Query_for_list_of_ts_parsers, NULL, THING_NO_SHOW},
 	{"POLICY", NULL, NULL},
-	{"PUBLICATION", NULL, NULL},
+	{"PUBLICATION", Query_for_list_of_publications},
 	{"ROLE", Query_for_list_of_roles},
 	{"RULE", "SELECT pg_catalog.quote_ident(rulename) FROM pg_catalog.pg_rules WHERE substring(pg_catalog.quote_ident(rulename),1,%d)='%s'"},
 	{"SCHEMA", Query_for_list_of_schemas},
 	{"SEQUENCE", NULL, _for_list_of_sequences},
 	{"SERVER", Query_for_list_of_servers},
-	{"SUBSCRIPTION", NULL, NULL},
+	{"SUBSCRIPTION", Query_for_list_of_subscriptions},
 	{"TABLE", NULL, _for_list_of_tables},
 	{"TABLESPACE", Query_for_list_of_tablespaces},
 	{"TEMP", NULL, NULL, THING_NO_DROP},		/* for CREATE TEMP TABLE ... */

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

[HACKERS] 3D Z-curve spatial index

[Boris Muratshin]

Hi hackers,

The low-level implementation of 3D Z-curve index
(https://github.com/bmuratshin/zcurve/tree/master)
is getting close to GiST R-Tree performance at
significantly lesser number of pages read from disk.

See attached figures,
times2 - average request execution time VS average points number in result
reads2 - average shared reads number VS average points number in result

Feel free to connect with me if you have any questions.

-- 
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] pageinspect: Hash index support

[Ashutosh Sharma]

> As far as I can tell, the hash_bitmap_info() function is doing
> something completely ridiculous.  One would expect that the purpose of
> this function was to tell you about the status of pages in the bitmap.
> The documentation claims that this is what the function does: it
> claims that this function "shows the status of a bit in the bitmap
> page for a particular overflow page".  So you would think that what
> the function would do is:
>
> 1. Work out which bitmap page contains the bit for the page number in 
> question.
> 2. Read that bitmap page.
> 3. Indicate the status of that bit within that page.
>
> However, that's not what the function actually does.  Instead, it does this:
>
> 1. Go examine the overflow page and see whether hasho_prevblkno.  If
> so, claim that the bit is set in the bitmap; if not, claim that it
> isn't.
> 2. Work out which bitmap page contains the bit for the page number in 
> question.
> 3. But don't look at it.  Instead, tell the user which bitmap page and
> bit you would have looked at, but instead of returning the status of
> that bit, return the value you computed in step 1.
>
> I do not think this can be the right approach.

Yes, It is not a right approach. As I mentioned in [1], the overflow
page being freed is completely filled with zero values which means it
is not in a readable state. So, we won't be able to examine a free
overflow page. Considering these facts, I would take following
approach,

1) Check if an overflow page is a new page. If so, read a bitmap page
to confirm if a bit corresponding to this overflow page is clear or
not. For this, I would first add Assert statement to ensure that the
bit is clear and if it is, then set the statusbit as false indicating
that the page is free.

2) In case if an overflow page is not new, first verify if it is
really an overflow page and if so, check if the bit corresponding to
it in the bitmap page is SET. If so, set the statusbit as true; If
not, we would see an assertion failure happening.

If you are okay with this approach, please let me know I will share
you an updated patch. Thanks.

[1]- 
https://www.postgresql.org/message-id/CAE9k0PkiwT0qD3fdruU8bgAjTpzJpnqcT0XNWnnKxxFbogbL9A%40mail.gmail.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] pgbench more operators & functions

[Fabien COELHO]

Hello,


For my 2c, at least, while I'm definitely interested in this, it's not
nearly high enough on my plate with everything else going on to get any
attention in the next few weeks, at least.

I do think that, perhaps, this patch may deserve a bit of a break, to
allow people to come back to it with a fresh perspective, so perhaps
moving it to the next commitfest would be a good idea, in a Needs Review
state.


So, let's try again for the next CF...

Here is a v9 which includes some more cleanup, hopefully in the expected 
direction which is to make pgbench expressions behave as SQL expressions, 
and I hope taking into account all other feedback as well.



CONTEXT

Pgbench has been given an expression parser (878fdcb8) which allows to use 
full expressions instead of doing one-at-a-time operations. This parser 
has been extended with functions (7e137f84) & double type (86c43f4e). The 
first batch of functions was essentially a poc about how to add new 
functions with various requirements. Pgbench default "tpcb-like" test 
takes advantage of these additions to reduce the number of lines it needs.



MOTIVATION

This patch aims at providing actually useful functions for benchmarking. 
The functions and operators provided here are usual basic operations. They 
are not chosen randomly, but are simply taken from existing benchmarks:


In TPC-B 2.0.0 section 5.3.5 and TPC-C 5.11 section 2.5.1.2, the selection 
of accounts uses a test (if ...), logical conditions (AND, OR) and 
comparisons (<, =, >=, >).


In TPC-C 5.11 section 2.1.6, a bitwise or (|) is used to skew a 
distribution based on two uniform distributions.


In TPC-C 5.11 section 5.2.5.4, a log function is used to determine "think 
time", which can be truncated (i.e. "least" function, already in pgbench).



CONTENTS

The attached patch provides a consistent set of functions and operators 
based on the above examples, with operator precedence taken from postgres 
SQL parser:


- "boolean" type support is added, because it has been requested that 
pgbench should be as close as SQL expressions as possible. This induced 
some renaming as some functions & struct fields where named "num" because 
they where expecting an int or a double, but a boolean is not really a 
numeral.


- SQL comparisons (= <> < > <= >=) plus pg SQL "!=", which result in a 
boolean.


- SQL logical operators (and or not) on booleans.

- SQL bitwise operators taken from pg: | & # << >> ~.

- mod SQL function as a synonymous for %.

- ln and exp SQL functions.

- SQL CASE/END conditional structure.

The patch also includes documentation and additional tap tests.
A test script is also provided.

This version is strict about typing, mimicking postgres behavior. For 
instance, using an int as a boolean results in a error. It is easy to make 
it more tolerant to types, which was the previous behavior before it was 
suggested to follow SQL behavior.


Together with another submitted patch about retrieving query results, the 
added capabilities allow to implement strictly conforming TPC-B 
transactions.


--
Fabien.diff --git a/doc/src/sgml/ref/pgbench.sgml b/doc/src/sgml/ref/pgbench.sgml
index 1eee8dc..36a63fc 100644
--- a/doc/src/sgml/ref/pgbench.sgml
+++ b/doc/src/sgml/ref/pgbench.sgml
@@ -827,12 +827,13 @@ pgbench  options  dbname
   from expression.
   The expression may contain integer constants such as 5432,
   double constants such as 3.14159,
+  boolean constants TRUE and FALSE,
   references to variables :variablename,
-  unary operators (+, -) and binary operators
-  (+, -, *, /,
-  %) with their usual precedence and associativity,
-  function calls, and
-  parentheses.
+  operators
+  with their usual SQL precedence and associativity,
+  function calls,
+  SQL CASE generic conditional expressions
+  and parentheses.
  
 
  
@@ -917,6 +918,165 @@ pgbench  options  dbname
   
  
 
+ 
+  Built-In Operators
+
+  
+   The arithmetic, bitwise, comparison and logical operators listed in
+are built into pgbench
+   and may be used in expressions appearing in
+   \set.
+  
+
+  
+   pgbench Operators by increasing precedence
+   
+
+ 
+  Operator
+  Description
+  Example
+  Result
+ 
+
+
+ 
+  OR
+  logical or
+  5 or 0
+  1
+ 
+ 
+  AND
+  logical and
+  3 and 0
+  0
+ 
+ 
+  NOT
+  logical not
+  not false
+  1
+ 
+ 
+  =
+  is equal
+  5 = 4
+  0
+ 
+ 
+  
+  is not equal
+  5  4
+  1
+ 
+ 
+  !=
+  is not equal
+  5 != 5
+  0
+ 
+ 
+  
+  lower than
+  5  4
+  0
+ 
+ 
+  =
+  lower or equal
+  5 = 4
+  0
+ 
+ 
+  
+  greater than
+  5  4
+  1
+ 
+ 
+  =
+  greater or equal
+  5 = 4
+  1
+ 
+ 
+  |
+  integer bitwise OR
+   

Re: [HACKERS] pageinspect: Hash index support

[Ashutosh Sharma]

On Sat, Jan 28, 2017 at 10:25 PM, Ashutosh Sharma  wrote:
> Hi,
>
> Please find my reply inline.
>
>> In hash_bimap_info(), we go to the trouble of creating a raw page but
>> never do anything with it.  I guess the idea here is just to error out
>> if the supplied page number is not an overflow page, but there are no
>> comments explaining that.  Anyway, I'm not sure it's a very good idea,
>> because it means that if you try to write a query to interrogate the
>> status of all the bitmap pages, it's going to read all of the overflow
>> pages to which they point, which makes the operation a LOT more
>> expensive.  I think it would be better to leave this part out; if the
>> user wants to know which pages are actually overflow pages, they can
>> use hash_page_type() to figure it out.
>
> Yes, the intention was to ensure that user only passes overflow page
> as an input to this function. I think if we wan't to avoid creating a
> raw page then we may need to find some other way to verify if it is an
> overflow page or not, may be we can make use of hash_check_page().
>
> Let's make it the job of this
>> function just to check the available/free status of the page in the
>> bitmap, without reading the bitmap itself.
>>
>
> okay, In that case I think we can check the previous block number that
> the overflow page is pointing to in order to identify if it is free or
> in-use. AFAIU, if an overflow page is free it's prev block number will
> be Invalid. This way, we may not have to read bitmap page. Now the
> question here is, we also have bucket pages where previous block
> number is always Invalid but before checking this we ensure that we
> are only dealing with an overflow page.Please let me know if you feel
> we do have some better option than this to identify the status of
> overflow page without reading bitmap.
>

I think this was a very poor finding by me. If an overflow page is
freed, it is completely filled with zero values rather than marking
it's prev and next block number as invalid. Therefore, we won't be
able to read a free overflow page as it is a new page and hence, we
can't decide if an overflow page is free or not without reading the
corresponding bitmap page.

>> In doing that, I think it should either return (bitmapblkno,
>> bitmapbit, bit) or just bit.  Returning bitmapblkno but not bitmapbit
>> seems strange.  Also, I think it should return bit as a bool, not
>> int4.
>
> It would be good to return bitmap bit number as well along with the
> bitmap block number. Also, returning bit as bool would look good. I
> will do that.
>
> I am also working on other review comments and will share the updated
> patch asap. Thanks.


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

Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Corey Huinker]

On Sat, Feb 4, 2017 at 11:53 AM, Corey Huinker 
wrote:

> The check I was suggesting on whether Ctrl+C has been pressed
>> on an empty line seems harder to implement, because get_interactive()
>> just calls readline() or fgets(), which block to return when a whole
>> line is ready. AFAICS psql can't know what was the edit-in-progress
>> when these functions are interrupted by a signal instead of
>> returning normally.
>> But I don't think this check is essential, it could be left to another
>> patch.
>>
>
> Glad I wasn't missing something obvious.
> I suppose we could base the behavior on whether there's at least one full
> line already buffered.
> However, I agree that it can be left to another patch.
>

v6 patch. highlights:
- error messages are now a bit more terse, following suggestions
- help text is more terse and Conditionals section was moved below Input
Output
- leverage IFSTATE_NONE a bit to fold some not-in-a-branch cases into
existing switch statements, giving flatter, slightly cleaner code and that
addresses expected cases before exceptional ones
- comments highlight which messages are printed in both interactive and
script mode.
- put Fabien's tap test in place verbatim
- No mention of Ctrl-C or PROMPT. Those can be addressed in separate
patches.

There's probably some more consensus building to do over the interactive
messages and comments, and if interactive-ish tests are possible with TAP,
we should add those too.
diff --git a/doc/src/sgml/ref/psql-ref.sgml b/doc/src/sgml/ref/psql-ref.sgml
index ae58708..c0ba4c4 100644
--- a/doc/src/sgml/ref/psql-ref.sgml
+++ b/doc/src/sgml/ref/psql-ref.sgml
@@ -2035,6 +2035,67 @@ hello 10
 
   
 
+  
+\if expr
+\elif expr
+\else
+\endif
+
+
+This group of commands implements nestable conditional blocks, like
+this:
+
+
+SELECT
+EXISTS(SELECT 1 FROM customer WHERE customer_id = 123) as is_customer,
+EXISTS(SELECT 1 FROM employee WHERE employee_id = 456) as is_employee
+\gset
+\if :is_customer
+SELECT * FROM customer WHERE customer_id = 123;
+\elif :is_employee
+\echo 'is not a customer but is an employee'
+SELECT * FROM employee WHERE employee_id = 456;
+\else
+\if yes
+\echo 'not a customer or employee'
+\else
+\echo 'this should never print'
+\endif
+\endif
+
+
+Conditional blocks must begin with a \if and end
+with an \endif, and the pairs must be found in
+the same source file. If an EOF is reached on the main file or an
+\include-ed file before all local
+\if-\endif are matched, then
+psql will raise an error.
+
+
+A conditional block can have any number of 
+\elif clauses, which may optionally be followed by a
+single \else clause.
+
+
+The \if and \elif commands
+read the rest of the line and evaluate it as a boolean expression.
+Currently, expressions are limited to a single unquoted string
+which are evaluated like other on/off options, so the valid values
+are any unambiguous case insensitive matches for one of:
+true, false, 
1,
+0, on, off,
+yes, no.  So 
+t, T, and tR
+will all match true.
+
+
+Lines within false branches are not evaluated in any way: queries are
+not sent to the server, non-conditional commands are not evaluated but
+bluntly ignored, nested if-expressions in such branches are also not
+evaluated but are tallied to check for proper nesting.
+
+
+  
 
   
 \ir or \include_relative 
filename
diff --git a/src/bin/psql/Makefile b/src/bin/psql/Makefile
index c53733f..7a418c6 100644
--- a/src/bin/psql/Makefile
+++ b/src/bin/psql/Makefile
@@ -61,8 +61,16 @@ uninstall:
 
 clean distclean:
rm -f psql$(X) $(OBJS) lex.backup
+   rm -rf tmp_check
 
 # files removed here are supposed to be in the distribution tarball,
 # so do not clean them in the clean/distclean rules
 maintainer-clean: distclean
rm -f sql_help.h sql_help.c psqlscanslash.c
+
+
+check:
+   $(prove_check)
+
+installcheck:
+   $(prove_installcheck)
diff --git a/src/bin/psql/command.c b/src/bin/psql/command.c
index f17f610..9058897 100644
--- a/src/bin/psql/command.c
+++ b/src/bin/psql/command.c
@@ -49,6 +49,7 @@
 #include "psqlscanslash.h"
 #include "settings.h"
 #include "variables.h"
+#include "fe_utils/psqlscan_int.h"
 
 /*
  * Editable database object types.
@@ -132,7 +133,7 @@ HandleSlashCmds(PsqlScanState scan_state,
status = PSQL_CMD_ERROR;
}
 
-   if (status != PSQL_CMD_ERROR)
+   if (status != PSQL_CMD_ERROR && pset.active_branch)
{
/* eat any remaining arguments after a valid command */
/* note we suppress evaluation of backticks here */
@@ 

[HACKERS] Ancient row-ordering instability in regression tests

[Tom Lane]

I noticed that skink failed today with a row-ordering difference:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=skink=2017-02-04%2009%3A40%3A01

Looking at the regression test operations that change table onek2,
I think the blame has to fall on this sequence in the "misc" test:

DELETE FROM onek2;

COPY onek2 FROM '@abs_builddir@/results/onek.data';

Presumably, what happened on "skink" is that a background autovacuum
started cleaning up the DELETE's detritus before the COPY finished,
allowing rows inserted by COPY to go into the table out-of-order.

We could replace the DELETE with a TRUNCATE, but that would change
the test conditions.  Basically always up to this point, onek2 has
had a bunch of dead rows that might get vacuumed away at some point
in the tests, and I'm a bit loath to discard that.

A slightly better option is to wrap this sequence in BEGIN/COMMIT
so that the DELETE's not committed done until after the COPY.

Or we could leave it alone, but I dislike regression tests that sometimes
fail, even if "sometimes" is "only once in years".

Thoughts?

regards, tom lane


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

Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Corey Huinker]

>
> I noticed that the "barking" is conditional to "success". ISTM that it
> should always "bark" in interactive mode, whether success or not.
>

"success" in those cases means "the expression was a valid boolean", and
non-success cases (should) result in an error being printed regardless of
interactive mode. If you see otherwise, let me know.


>
> While testing it and seeing the code, I agree that it is too
> verbose/redundant. At least remove "active/inactive, ".


Have done so, new patch pending "how-do-I-know-when-input-is-empty" in Ctrl
C.


> - Help text. New block in help text called "Conditionals"
>>
>
> Maybe it could be moved to "Input/Output" renamed as "Input/Output
> Control", or maybe the "Conditionals" section could be moved next to it, it
> seems more logical than after large objects.
>

I put it near the bottom, figuring someone would have a better idea of
where to put it. You did.



> I think that the descriptions are too long. The interactive user can be
> trusted to know what "if/elif/else/endif" mean, or to refer to the full
> documentation otherwise. The point is just to provide a syntax and function
> reminder, not a substitute for the doc. Thus I would suggest shorter
> one-line messages like:
>
>  \if begin a new conditional block
>  \elif   else if in the current conditional block
>  \else else in current conditional block
>  \endifend current conditional block
>

+1



>
>>
> Hmmm. Perl is perl. Attached an attempt at improving that, which is
> probably debatable, but at least it is easy to add further tests without
> massive copy-pasting.


+1 that's a good start.


>
>
> - regression tests now have comments to explain purpose
>>
>
> Ok.
>
> Small details about the code:
>
>   +   if (!pset.active_branch && !is_branching_command(cmd) )
>
> Not sure why there is a space before the last closing parenthesis.


+1

Re: \if, \elseif, \else, \endif (was Re: [HACKERS] PSQL commands: \quit_if, \quit_unless)

[Corey Huinker]

>
> The check I was suggesting on whether Ctrl+C has been pressed
> on an empty line seems harder to implement, because get_interactive()
> just calls readline() or fgets(), which block to return when a whole
> line is ready. AFAICS psql can't know what was the edit-in-progress
> when these functions are interrupted by a signal instead of
> returning normally.
> But I don't think this check is essential, it could be left to another
> patch.
>

Glad I wasn't missing something obvious.
I suppose we could base the behavior on whether there's at least one full
line already buffered.
However, I agree that it can be left to another patch.

[HACKERS] Draft release notes for next week's releases are up for review

[Tom Lane]

First-draft release notes are available at
https://git.postgresql.org/gitweb/?p=postgresql.git;a=commitdiff;h=9863017b87f3592ff663d03fc663a4f1f8fdb8b2
They should appear in a more readable form at
https://www.postgresql.org/docs/devel/static/release-9-6-2.html
after guaibasaurus' next buildfarm run, due a couple hours from now.

As usual, I've filled in the frontmost branch's section with all the
material, even though a couple of items don't apply because they only
went into further-back branches.  I'll sort that out when I split
things up.  Right now the question is whether individual items are
correctly/adequately documented.

regards, tom lane


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

Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

[Fujii Masao]

On Fri, Feb 3, 2017 at 3:55 AM, Peter Eisentraut
 wrote:
> On 2/2/17 12:48 PM, Fujii Masao wrote:
>> +#define Query_for_list_of_subscriptions \
>> +" SELECT pg_catalog.quote_ident(subname) "\
>> +"   FROM pg_catalog.pg_subscription "\
>> +"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"
>>
>> Since non-superuser is not allowed to access to pg_subscription,
>> pg_stat_subscription should be accessed here, instead. Thought?
>
> Arguably, you could leave it like that, assuming it fails cleanly for
> nonsuperusers.  Nonsuperusers are not going to be able to run any
> commands on subscriptions anyway, so there is little use for it.

No. You can get rid of superuser privilege from the owner of the subscription
and that nonsuperuser owner can run some commands on the subscriptin.
It's a bit artificial, but you can. I'm not sure if we should add the check to
prevent the owner from becoming nonsuperuser. But if the owner always must
have a superuser privilege per the specification of logical replication,
I think that such check would be necessary.

Also I prefer to tab-complete the subscriptions even for nonsuperusers.
There are some objects that only superuser or owner can manage, but their
names are currently tab-completed even when current user is "normal" one.
So I'm afraid that handling only subscriptions differently might be more
confusing.

Regards,

-- 
Fujii Masao


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

Re: [HACKERS] Logical Replication and Character encoding

[Petr Jelinek]

On 01/02/17 04:05, Kyotaro HORIGUCHI wrote:
> Hello,
> 
> At Tue, 31 Jan 2017 12:46:18 +, "Shinoda, Noriyoshi" 
>  wrote in 
> 
>>  I tried a committed Logical Replication environment. I found
>>  that replication between databases of different encodings did
>>  not convert encodings in character type columns. Is this
>>  behavior correct?
> 
> The output plugin for subscription is pgoutput and it currently
> doesn't consider encoding but would easiliy be added if desired
> encoding is informed.
> 
> The easiest (but somewhat seems fragile) way I can guess is,
> 
> - Subscriber connects with client_encoding specification and the
>   output plugin pgoutput decide whether it accepts the encoding
>   or not. If the subscriber doesn't, pgoutput send data without
>   conversion.
> 

Hmm I wonder if we should just make the subscriber send the
client_encoding always (based on server encoding of the subscriber).
That should solve the issue in combination with your patch no?

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


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

Re: [HACKERS] Index corruption with CREATE INDEX CONCURRENTLY

[Tom Lane]

Alvaro Herrera  writes:
> I intend to commit this soon to all branches, to ensure it gets into the
> next set of minors.

Based on Pavan's comments, I think trying to force this into next week's
releases would be extremely unwise.  If the bug went undetected this long,
it can wait for a fix for another three months.  That seems better than
risking new breakage when it's barely 48 hours to the release wrap
deadline.  We do not have time to recover from any mistakes.

regards, tom lane


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

Re: [HACKERS] PinBuffer() no longer makes use of strategy

[Alexander Korotkov]

On Sat, Feb 4, 2017 at 4:33 AM, Andres Freund  wrote:

> On 2017-02-03 19:13:45 -0600, Jim Nasby wrote:
> > No, I noticed it while reading code. Removing that does mean that if any
> > non-default strategy (in any backend) hits that buffer again then the
> buffer
> > will almost certainly migrate into the main buffer pool the next time
> one of
> > the rings hits that buffer
>
> Well, as long as the buffer is used from the ring, BufferAlloc() -
> BufferAlloc() will reset the usagecount when rechristening the
> buffer. So unless anything happens inbetween the buffer being remapped
> last and remapped next, it'll be reused. Right?
>
> The only case where I can see the old logic mattering positively is for
> synchronized seqscans.  For pretty much else that logic seems worse,
> because it essentially prevents any buffers ever staying in s_b when
> only ringbuffer accesses are performed.
>
> I'm tempted to put the old logic back, but more because this likely was
> unintentional, not because I think it's clearly better.
>

+1
Yes, it was unintentional change.  So we should put old logic back unless
we have proof that this change make it better.
Patch is attached.  I tried to make some comments, but probably they are
not enough.

--
Alexander Korotkov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company


put-buffer-usagecount–logic–back.patch
Description: Binary data

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

Re: [HACKERS] Provide list of subscriptions and publications in psql's completion

[Fujii Masao]

On Sat, Feb 4, 2017 at 9:01 PM, Michael Paquier
 wrote:
> On Fri, Feb 3, 2017 at 3:56 AM, Peter Eisentraut
>  wrote:
>> On 2/2/17 12:48 AM, Michael Paquier wrote:
>>> +#define Query_for_list_of_subscriptions \
>>> +" SELECT pg_catalog.quote_ident(subname) "\
>>> +"   FROM pg_catalog.pg_subscription "\
>>> +"  WHERE substring(pg_catalog.quote_ident(subname),1,%d)='%s'"
>>
>> This query should also be qualified by current database.
>
> Indeed. Here is an updated patch.

With this patch, when normal users type TAB after SUBSCRIPTION,
they got "permission denied" error. I don't think that's acceptable.

In "CREATE SUBSCRIPTION ... PUBLICATION" and "ALTER SUBSCRIPTION ... SET
PUBLICATION" cases, the publication defined in the publisher side should be
specified. But, with this patch, the tab-completion for PUBLICATION shows
the publications defined in local server (ie, subscriber side in those cases).
This might be confusing.

Regards,

-- 
Fujii Masao


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

Re: [HACKERS] Cannot shutdown subscriber after DROP SUBSCRIPTION

[Petr Jelinek]

On 03/02/17 19:38, Fujii Masao wrote:
> On Sat, Feb 4, 2017 at 12:49 AM, Fujii Masao  wrote:
>> On Fri, Feb 3, 2017 at 10:56 AM, Kyotaro HORIGUCHI
>>  wrote:
>>> At Fri, 3 Feb 2017 01:02:47 +0900, Fujii Masao  
>>> wrote in 
>>>