Re: [HACKERS] Queuing all tables for analyze after recovery
On 10/19/2017 11:26 PM, Tom Lane wrote: > Vik Fearing <vik.fear...@2ndquadrant.com> writes: >> On 10/19/2017 10:54 PM, Tom Lane wrote: >>> Uh ... recommended by whom? pg_statistic has exactly the same reliability >>> guarantees as the rest of the system catalogs. > >> For data statistics, sure. One thing I'm unhappy about is that >> pg_stat_all_tables is blank. > > Well, that's because we throw away the stats collector's stats after a > crash -- or, in the failover case, because the promoted slave has its own > counters and not the master's. ANALYZE isn't going to help that at all. Sure it will. ANALYZE estimates n_dead_tup, which often accurate enough for autovacuum to figure out what to do. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Queuing all tables for analyze after recovery
On 10/19/2017 10:54 PM, Tom Lane wrote: > Tomasz Ostrowski <tometzky...@ato.waw.pl> writes: >> Some (maybe all) row statistics are lost after the database has >> recovered after a failover. So it's recommended to ANALYZE all databases >> in a cluster after recovery. > > Uh ... recommended by whom? pg_statistic has exactly the same reliability > guarantees as the rest of the system catalogs. > > I don't deny that there might be cases where this is worth doing, but > it does not seem so likely that it should be part of one's standard > checklist. Much less something that we should expend a great deal > of effort to automate. For data statistics, sure. One thing I'm unhappy about is that pg_stat_all_tables is blank. An idea I've been throwing around in my head is to have autovacuum work on tables that have vacuum_count and autovacuum_count both zero (and likewise for analyze). This will cause a flurry of activity after failover or crash, but the alternative is autovacuum not knowing anything about the state of the tables and allowing massive bloat to potentially occur. For example, if you have a 1 billion row table, and crash when there are 199,999,999 dead tuples, you currently get to wait for another 200 million to die before anything gets cleaned up. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Determine state of cluster (HA)
On 10/17/2017 08:40 PM, Joshua D. Drake wrote: > On 10/16/2017 07:31 PM, Craig Ringer wrote: >> On 17 October 2017 at 01:02, Joshua D. Drake <j...@commandprompt.com> >> wrote: >>> On 10/15/2017 07:39 PM, Craig Ringer wrote: > >>>> - Get info about master. We should finish merging recovery.conf into >>>> postgresql.conf. >>> >>> Definitely. >> >> There's a patch from Abhijit Menon-Sen you could adopt for PostgreSQL >> 11 for that. > > Do you have a link to this? https://commitfest.postgresql.org/search/?searchterm=recovery.conf -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] pageinspect function to decode infomasks
On 10/14/2017 11:47 PM, Peter Geoghegan wrote: > On Sat, Oct 14, 2017 at 10:58 AM, Robert Haas <robertmh...@gmail.com> wrote: >> I think it's perfectly sensible to view those 2 bits as making up a >> 2-bit field with 4 states rather than displaying each bit >> individually, but you obviously disagree. Fair enough.> > I guess it is that simple. FWIW, my opinion falls in line with Robert's. Also, whichever way it goes, this is a patch I've been wanting for a long time. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] WIP Add ALWAYS DEFERRED option for constraints
On 10/03/2017 10:10 PM, Andreas Joseph Krogh wrote: > While we're in deferrable constraints land...; > I even more often need deferrable /conditional /unique-indexes. > In PG you now may have: > > ALTER TABLE email_folder ADD CONSTRAINT some_uk UNIQUE (owner_id, > folder_type, name) DEFERRABLE INITIALLY DEFERRED; > > > But this isn't supported: > > CREATE UNIQUE INDEX some_uk ON email_folder(owner_id, folder_type, name) > WHERE parent_id IS NULL DEFERRABLE INITIALLY DEFERRED; > > Are there any plans to support this? I don't want to hijack the thread, but you can do that with exclusion constraints. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Logging idle checkpoints
I recently had a sad because I noticed that checkpoint counts were
increasing in pg_stat_bgwriter, but weren't accounted for in my logs
with log_checkpoints enabled.
After some searching, I found that it was the idle checkpoints that
weren't being logged. I think this is a missed trick in 6ef2eba3f57.
Attached is a one-liner fix. I realize how imminent we are from
releasing v10 but I hope there is still time for such a minor issue as this.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c
index dd028a12a4..75f6bd4cc1 100644
--- a/src/backend/access/transam/xlog.c
+++ b/src/backend/access/transam/xlog.c
@@ -8724,7 +8724,7 @@ CreateCheckPoint(int flags)
WALInsertLockRelease();
LWLockRelease(CheckpointLock);
END_CRIT_SECTION();
- ereport(DEBUG1,
+ ereport(log_checkpoints ? LOG : DEBUG1,
(errmsg("checkpoint skipped because system is idle")));
return;
}
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] New partitioning - some feedback
On 07/07/2017 02:02 AM, Mark Kirkwood wrote: > I'd prefer *not* to see a table and its partitions all intermixed in the > same display (especially with nothing indicating which are partitions) - > as this will make for unwieldy long lists when tables have many > partitions. Also it would be good if the 'main' partitioned table and > its 'partitions' showed up as a different type in some way. I've just read through this thread, and I'm wondering why we can't just have something like \set SHOW_PARTITIONS true or something, and that would default to false. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Inconsistent syntax for NumericOnly grammar production
On 05/28/2017 11:16 PM, Tom Lane wrote: > The inconsistency here means that you can do, for example, > > regression=# set random_page_cost = +4; > SET > regression=# set random_page_cost = 4.2; > SET > > but not > > regression=# set random_page_cost = +4.2; > ERROR: syntax error at or near "4.2" > LINE 1: set random_page_cost = +4.2; > ^ > Any objections to allowing "+ FCONST" here? I'm inclined to > fix this and back-patch it as well. Seems like the right thing to do; no objections here. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Renaming a table to an array's autogenerated name
On 05/26/2017 03:20 PM, Tom Lane wrote: > Vik Fearing <vik.fear...@2ndquadrant.com> writes: >> On 05/25/2017 05:24 PM, Tom Lane wrote: >>> After some experimentation, I came up with the attached, which simply >>> skips the "recursive" step if it would apply to the same array type we >>> already moved. > >> This looks good to me. > > Pushed, thanks for reviewing. Thanks! -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Renaming a table to an array's autogenerated name
On 05/25/2017 05:24 PM, Tom Lane wrote: > Vik Fearing <vik.fear...@2ndquadrant.com> writes: >> In commit 9aa3c782c93, Tom fixed a bug in which creating a table _foo >> when an array type of that name already existed would make the array >> type change its name to get out of the way. But it missed a trick in >> that renaming a table would still cause a conflict. > > Good catch. > >> One interesting thing to note however, is that if you rename a table to >> its own array's name (which was my case when I found this bug), the new >> array name will be ___foo instead of just __foo. I don't know if it's >> worth worrying about that. > > After some experimentation, I came up with the attached, which simply > skips the "recursive" step if it would apply to the same array type we > already moved. This looks good to me. > I added some queries to the regression test case to show exactly what > happens to the array type names, and in that, you can see that the > behavior for the "normal" case with distinct array types is that neither > array type ends up with a name that is just the unsurprising case of > an underscore followed by its element type's name; they *both* have an > extra underscore compared to that. Maybe that's okay. We could possibly > rejigger the order of renaming so that one of them ends with an > unsurprising name, but I failed to make that happen without considerably > more surgery. I don't think this really matters to anyone in practice, so I'm fine with it. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Renaming a table to an array's autogenerated name
In commit 9aa3c782c93, Tom fixed a bug in which creating a table _foo
when an array type of that name already existed would make the array
type change its name to get out of the way. But it missed a trick in
that renaming a table would still cause a conflict.
Steps to reproduce:
postgres=# create table foo (id int);
CREATE TABLE
postgres=# create table bar (id int);
CREATE TABLE
postgres=# alter table bar rename to _foo;
ERROR: type "_foo" already exists
Attached is a patch that fixes this bug.
One interesting thing to note however, is that if you rename a table to
its own array's name (which was my case when I found this bug), the new
array name will be ___foo instead of just __foo. I don't know if it's
worth worrying about that.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
diff --git a/src/backend/catalog/pg_type.c b/src/backend/catalog/pg_type.c
index 04c10c6347..9e4e46f9f5 100644
--- a/src/backend/catalog/pg_type.c
+++ b/src/backend/catalog/pg_type.c
@@ -695,6 +695,7 @@ RenameTypeInternal(Oid typeOid, const char *newTypeName, Oid typeNamespace)
HeapTuple tuple;
Form_pg_type typ;
Oid arrayOid;
+ Oid typoid;
pg_type_desc = heap_open(TypeRelationId, RowExclusiveLock);
@@ -708,10 +709,22 @@ RenameTypeInternal(Oid typeOid, const char *newTypeName, Oid typeNamespace)
arrayOid = typ->typarray;
+ typoid = GetSysCacheOid2(TYPENAMENSP,
+ CStringGetDatum(newTypeName),
+ ObjectIdGetDatum(typeNamespace));
+
+ /*
+ * See if it's an autogenerated array type, and if so
+ * rename it out of the way.
+ */
+ if (OidIsValid(typoid) && get_typisdefined(typoid))
+ {
+ if (moveArrayTypeName(typoid, newTypeName, typeNamespace))
+ typoid = InvalidOid;
+ }
+
/* Just to give a more friendly error than unique-index violation */
- if (SearchSysCacheExists2(TYPENAMENSP,
- CStringGetDatum(newTypeName),
- ObjectIdGetDatum(typeNamespace)))
+ if (OidIsValid(typoid))
ereport(ERROR,
(errcode(ERRCODE_DUPLICATE_OBJECT),
errmsg("type \"%s\" already exists", newTypeName)));
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index c88fd76848..142a782bff 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -128,6 +128,15 @@ SELECT * FROM tmp_new2;
DROP TABLE tmp_new;
DROP TABLE tmp_new2;
+-- renaming to an array's autogenerated name (the array's name should get out of the way)
+CREATE TABLE tmp_array (id int);
+CREATE TABLE tmp_array2 (id int);
+ALTER TABLE tmp_array2 RENAME TO _tmp_array;
+DROP TABLE _tmp_array;
+DROP TABLE tmp_array;
+CREATE TABLE tmp_array (id int);
+ALTER TABLE tmp_array RENAME TO _tmp_array;
+DROP TABLE _tmp_array;
-- ALTER TABLE ... RENAME on non-table relations
-- renaming indexes (FIXME: this should probably test the index's functionality)
ALTER INDEX IF EXISTS __onek_unique1 RENAME TO tmp_onek_unique1;
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index c0e29720dc..1a893e9e2c 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -165,6 +165,16 @@ SELECT * FROM tmp_new2;
DROP TABLE tmp_new;
DROP TABLE tmp_new2;
+-- renaming to an array's autogenerated name (the array's name should get out of the way)
+CREATE TABLE tmp_array (id int);
+CREATE TABLE tmp_array2 (id int);
+ALTER TABLE tmp_array2 RENAME TO _tmp_array;
+DROP TABLE _tmp_array;
+DROP TABLE tmp_array;
+
+CREATE TABLE tmp_array (id int);
+ALTER TABLE tmp_array RENAME TO _tmp_array;
+DROP TABLE _tmp_array;
-- ALTER TABLE ... RENAME on non-table relations
-- renaming indexes (FIXME: this should probably test the index's functionality)
--
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] CTE inlining
On 05/03/2017 07:33 PM, Alvaro Herrera wrote: > 1) we switch unmarked CTEs as inlineable by default in pg11. What seems > likely to happen for a user that upgrades to pg11 is that 5 out of 10 > CTE-using queries are going to become faster than with pg10, and they > are going to be happy; 4 out of five are going to see no difference, but > they didn't have to do anything about it; and the remaining query is > going to become slower, either indistinguishably so (in which case they > don't care and they remain happy because of the other improvements) or > notably so, in which case they can easily figure where to add the > MATERIALIZED option and regain the original performance. I am overwhelmingly in favor of this option. I am in favor of an enable_inlinedcte guc in the same spirit as the other enable_* gucs, but violently opposed to any "compatibility" guc. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] password_encryption, default and 'plain' support
On 05/05/2017 02:42 PM, Michael Paquier wrote: > +This option is obsolete but still accepted for backwards > +compatibility. > Isn't that incorrect English? No. > It seems to me that this be non-plural, > as "for backward compatibility". "Backwards" is not plural, it's a regional variation of "backward" (or vice versa depending on which region you come from). Both are correct. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] new autovacuum criterion for visible pages
On Sun, Jan 22, 2017 at 4:45 PM, Stephen Frost <sfr...@snowman.net> wrote: > Amit, > > * Amit Kapila (amit.kapil...@gmail.com) wrote: > > On Sun, Jan 22, 2017 at 3:27 AM, Stephen Frost <sfr...@snowman.net> > wrote: > > > * Simon Riggs (si...@2ndquadrant.com) wrote: > > >> On 12 August 2016 at 01:01, Tom Lane <t...@sss.pgh.pa.us> wrote: > > >> > Michael Paquier <michael.paqu...@gmail.com> writes: > > >> >> In short, autovacuum will need to scan by itself the VM of each > > >> >> relation and decide based on that. > > >> > > > >> > That seems like a worthwhile approach to pursue. The VM is > supposed to be > > >> > small, and if you're worried it isn't, you could sample a few pages > of it. > > >> > I do not think any of the ideas proposed so far for tracking the > > >> > visibility percentage on-the-fly are very tenable. > > >> > > >> Sounds good, but we can't scan the VM for every table, every minute. > > >> We need to record something that will tell us how many VM bits have > > >> been cleared, which will then allow autovac to do a simple SELECT to > > >> decide what needs vacuuming. > > >> > > >> Vik's proposal to keep track of the rows inserted seems like the best > > >> approach to this issue. > > > > > > I tend to agree with Simon on this. I'm also worried that an approach > > > which was based off of a metric like "% of table not all-visible" might > > > result in VACUUM running over and over on a table because it isn't able > > > to actually make any progress towards improving that percentage. We'd > > > have to have some kind of "cool-off" period or something. > > > > > > Tracking INSERTs and then kicking off a VACUUM based on them seems to > > > address that in a natural way and also seems like something that users > > > would generally understand as it's very similar to what we do for > > > UPDATEs and DELETEs. > > > > > > Tracking the INSERTs as a reason to VACUUM is also very natural when > you > > > consider the need to update BRIN indexes. > > > > Another possible advantage of tracking INSERTs is for hash indexes > > where after split we need to remove tuples from buckets that underwent > > split recently. > > That's a good point also, and for clearing GIN pending lists too, now > that I think about it. > As much as I want my patch to go in, this is not an argument for it. The GIN pending lists will be handled by autoanalyze. > We really need to get this fixed for PG10. > I'm not sure what more I'm supposed to be doing, if anything. -- Vik Fearing +33 6 46 75 15 36http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Re: [HACKERS] Adding type info etc for inheritance errmsg: "child table is missing column ..."
On 01/07/2017 11:05 PM, Tom Lane wrote: > Vik Fearing <v...@2ndquadrant.fr> writes: >> On 01/07/2017 08:15 PM, Tom Lane wrote: >>> No, and TBH I would vote strongly against including that much detail in >>> this error message anyway. That info could be indefinitely long, and it's >>> not especially relevant to the stated error condition --- for example, the >>> presence of a default is *not* relevant to whether the column matches the >>> parent. I'm okay with shoehorning column type into this message, but not >>> much more than that. > >> I agree. > >> Perhaps the ERROR message should remain as is, and a DETAIL or HINT line >> could be emitted with the entire column definition (or close to it)? > > AFAICS, what Ryan is after would be better served by a separate tool that > would produce a "diff" of the two tables' schemas. Even if we were > willing to overload this error message with everything we know about the > parent column, do you really want to fix discrepancies one column at a > time? And what about properties that can't be uniquely associated with a > single column, such as constraints covering multiple columns? > > Also, I have a feeling that a suitable tool is already out there. Well personally, if I were trying to attach one table to another and it didn't work, I'd use good ol' \d. Maybe that's just me. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Adding type info etc for inheritance errmsg: "child table is missing column ..."
On 01/07/2017 08:15 PM, Tom Lane wrote: > Ryan Murphy <ryanfmur...@gmail.com> writes: >> I was hoping for >> user=# alter table temp inherit entity; >> ERROR: child table is missing column "id" uuid default uuid_generate_v1mc() >> Is there an easy way to get the string that includes all those additional >> constraints/defaults etc? > > No, and TBH I would vote strongly against including that much detail in > this error message anyway. That info could be indefinitely long, and it's > not especially relevant to the stated error condition --- for example, the > presence of a default is *not* relevant to whether the column matches the > parent. I'm okay with shoehorning column type into this message, but not > much more than that. I agree. Perhaps the ERROR message should remain as is, and a DETAIL or HINT line could be emitted with the entire column definition (or close to it)? -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Rename pg_switch_xlog to pg_switch_wal
On 12/30/2016 06:46 PM, David Fetter wrote: > On Fri, Dec 30, 2016 at 09:57:25AM -0500, Stephen Frost wrote: > >> Let's make this a clean break/change. > > +1 +1 -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Unusable SP-GiST index
While trying to find a case where spgist wins over btree for text, I
came across the following behavior which I would consider a bug:
CREATE TABLE texts (value text);
INSERT INTO texts SELECT repeat('a', (2^20)::integer);
CREATE INDEX ON texts USING spgist (value);
SET enable_seqscan = off;
TABLE texts;
That produces:
ERROR: index row requires 12024 bytes, maximum size is 8191
It seems to me the index should not be allowed to be created if it won't
be usable.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--
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] Mail thread references in commits
On 11/20/2016 03:59 PM, Andrew Dunstan wrote: > > On 11/20/2016 06:55 AM, Magnus Hagander wrote: >> >> We can replace the website part with a http://postgr.es/m/ >> which will make it a bit shorter and still as easy to generate from >> the client side and to search off. It'd be trivial to do, and since >> there is no state held at the forwarder for it, it wouldn't introduce >> a risk of "losing the forwards at some point in the future". But the >> bulk of the URL, being the messageid, would remain. > > That's 19 extra characters, less 2 if you remove the <> delimiters. I > think the extra usability makes the slight increase in ugliness worth > it. Can we just agree or disagree on this? If non-committers are voting, I'll add my +1 to this. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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 to implement pg_current_logfile() function
On 03/09/2016 07:32 PM, Gilles Darold wrote: > This patch implements the pg_current_logfile() function that can be > used as follow. The function returns NULL when logging_collector > is not active and outputs a warning. I'm really late to this discussion, and I apologize for that; but I'm wondering why we're doing all this through some random file on disk. Why not just use the stats collector and have everything we'd want in a pg_stat_logger view just like we have for pg_stat_archiver and others? That makes the most sense to me. We could then also count the number of rotations per time/size and whatnot. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Renaming of pg_xlog and pg_clog
On 10/22/2016 06:00 PM, David Steele wrote: > On 10/22/16 6:58 PM, Bruce Momjian wrote: >> On Sat, Oct 22, 2016 at 07:33:56AM +0900, Michael Paquier wrote: >>> On Sat, Oct 22, 2016 at 4:29 AM, Alvaro Herrera >>> >>>> Also +1 to renaming pg_subtrans to pg_subxact. >>> >>> Nice suggestion, good naming for consistency with the rest. >> >> Agreed. > > +1 +1 from me, too. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] VACUUM's ancillary tasks
On 10/01/2016 09:28 AM, Thomas Munro wrote: > On Mon, Aug 29, 2016 at 1:26 PM, Vik Fearing <v...@2ndquadrant.fr> wrote: >> The attached two patches scratch two itches I've been having for a >> while. I'm attaching them together because the second depends on the first. >> >> Both deal with the fact that [auto]vacuum has taken on more roles than >> its original purpose. >> >> >> Patch One: autovacuum insert-heavy tables >> >> If you have a table that mostly receives INSERTs, it will never get >> vacuumed because there are no (or few) dead rows. I have added an >> "inserts_since_vacuum" field to PgStat_StatTabEntry which works exactly >> the same way as "changes_since_analyze" does. >> >> The reason such a table needs to be vacuumed is currently twofold: the >> visibility map is not updated, slowing down index-only scans; and BRIN >> indexes are not maintained, rendering them basically useless. > > I'm aware of those two problems, but not very familiar with the > details. I don't feel qualified to say whether insert counting is the > best approach to the problem at this point. I looked into it a little > bit however, and had the following thoughts: > > About BRIN indexes: I couldn't find an explanation of why BRIN > indexes don't automatically create new summary tuples when you insert > a new tuple in an unsummarised page range. Is it deferred until > VACUUM time in order to untangle some otherwise unresolvable > interlocking or crash safety problem, or could that one day be done? > Assuming that it must be deferred for some technical reason and there > is no way around it, then I wonder if there is a more direct and > accurate way to figure out when it's necessary than counting inserts. > Counting inserts seems slightly bogus because you can't tell whether > those were inserts into an existing summarised block which is > self-maintaining or not. At first glance it looks a bit like > unsummarised ranges can only appear at the end of the table, is that > right? If so, couldn't you detect the number of unsummarised BRIN > blocks just by comparing the highest summarised BRIN block and the > current heap size? > > About visibility maps: How crazy would it be to estimate the number > of not-all-visible pages instead? It would be less work to count that > since it would only increase when the *first* tuple is inserted into a > page that is currently all visible (ie when the bit is cleared), not > for every tuple inserted into any page like your inserts_since_vacuum > counter. Another difference is that inserts_since_vacuum is reset > even if vacuum finds that it *can't* set the all-visible bit for a > given page yet because of some concurrent transaction. In that case > the bit is still not set but autovacuum has no reason to be triggered > again. Sure, I could handle each case separately, but the goal of this patch (as hinted at in the Subject) is to generalize all the different tasks we've been giving to VACUUM. The only missing piece is what the first patch addresses; which is insert-mostly tables never getting vacuumed. As for the second patch, I would like to withdraw it and redesign it, based on your comments. The redesign I have in mind will no longer be dependent on the first patch, so I'll submit it separately. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Sample configuration files
On 09/29/2016 05:55 AM, Michael Paquier wrote: > On Thu, Sep 29, 2016 at 2:25 AM, Robert Haas <robertmh...@gmail.com> wrote: >> So, anyone else have an opinion, pro or con? > > Going through this thread, I'd vote -1. This is a documentation effort > mainly, and installing those files has zero effect if they are not > loaded via include_if_exists or include in postgresql.conf. Just the other day, I needed this patch yet again but had to go look up the documentation instead. I wonder if it would be a good idea to have a postgresql.conf.d directory that postgresql.conf would include_dir by default. These could then live in there and all I would have had to do is uncomment the values I wanted. This patch doesn't do that, of course, but I could easily write a patch that does. Would that go over better with the -1ers? -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Quorum commit for multiple synchronous replication.
On 09/21/2016 08:30 AM, Michael Paquier wrote:
> On Sat, Sep 17, 2016 at 2:04 AM, Masahiko Sawada <sawada.m...@gmail.com>
> wrote:
>> Since we already released 9.6RC1, I understand that it's quite hard to
>> change syntax of 9.6.
>> But considering that we support the quorum commit, this could be one
>> of the solutions in order to avoid breaking backward compatibility and
>> to provide useful user interface.
>> So I attached these patches.
>
> standby_config:
> -standby_list{ $$ = create_syncrep_config("1", $1); }
> -| FIRST NUM '(' standby_list ')'{ $$ =
> create_syncrep_config($1, $4); }
> +standby_list{ $$ =
> create_syncrep_config("1", $1, SYNC_REP_PRIORITY); }
> +| ANY NUM '(' standby_list ')'{ $$ =
> create_syncrep_config($2, $4, SYNC_REP_QUORUM); }
> +| FIRST NUM '(' standby_list ')'{ $$ =
> create_syncrep_config($2, $4, SYNC_REP_PRIORITY); }
>
> Reading again the thread, it seems that my previous post [1] was a bit
> misunderstood. My position is to not introduce any new behavior
> changes in 9.6, so we could just make the FIRST NUM grammar equivalent
> to NUM.
>
> [1]:
> https://www.postgresql.org/message-id/CAB7nPqRDvJn18e54ccNpOP1A2_iUN6-iU=4njgmmgiagvcs...@mail.gmail.com
I misunderstood your intent, then. But I still stand by what I did
understand, namely that 'k (...)' should mean 'any k (...)'. It's much
more natural than having it mean 'first k (...)' and I also think it
will be more frequent in practice.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Project Policies
I often see mention of project policies for various things (the one triggering this email is in commit 967a7b0). Where can I find documentation for these policies? -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Quorum commit for multiple synchronous replication.
On 09/09/2016 03:28 AM, Michael Paquier wrote: > On Thu, Sep 8, 2016 at 6:26 PM, Masahiko Sawada <sawada.m...@gmail.com> wrote: >> "k (n1, n2, n3)" == "first k (n1, n2, n3)" doesn't break backward >> compatibility but most users would think "k(n1, n2, n3)" as quorum >> after introduced quorum. >> I wish we can change the s_s_names syntax of 9.6 to "first k(n1, n2, >> n3)" style before 9.6 releasing if we got consensus. > > Considering breaking backward-compatibility in the next release does > not sound like a good idea to me for a new feature that is going to be > GA soon. Indeed. I'll vote for pulling a fast one on 9.6 for this. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Sample configuration files
On 08/29/2016 03:34 AM, Vik Fearing wrote: > We have sample configuration files for postgresql.conf and > recovery.conf, but we do not have them for contrib modules. This patch > attempts to add them. > > Although the patch puts the sample configuration files in the tree, it > doesn't try to install them. That's partly because I think it would > need an extension version bump and that doesn't seem worth it. > > Also, while writing this patch, it crossed my mind that the plpgsql > variables should probably be in the main postgresql.conf file because we > install it by default. I will happily write that patch if desired, > independently of this patch. > > Current as of 26fa446, added to next commitfest. I noticed that this patch has been marked Waiting on Author with no comment. Peter, what more should I be doing right now while waiting for MartÃn's review? -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Long options for pg_ctl waiting
On 09/08/2016 01:05 AM, Tom Lane wrote: > Vik Fearing <v...@2ndquadrant.fr> writes: >> On 09/07/2016 11:39 PM, Gavin Flower wrote: >>> Possibly generate warningswhen the non hyphenated form is used? > >> I'm not quite sure how I got volunteered to do this work, but it's easy >> enough so I don't mind. >> Here is a new patch that emits a warning when --noclean and/or --nosync >> are used. > > I'm pretty much -1 on printing a warning. There's no ambiguity, and no > real reason for us ever to remove the old spellings. Standardizing on > "no-" going forward makes sense, but let's not slap people's wrists for > existing usage. (Or: if it ain't broke, don't break it.) One could also argue that 2 out of 53 "no" options omitting the dash is in fact broken, and a real reason to remove them. I don't see the warning as "slapping wrists" so much as saying that we're harmonizing our conventions and their scripts need to be updated for the better. That said, I'm not going to fight for this. My only goal here is to get --wait and --no-wait added to pg_ctl. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Long options for pg_ctl waiting
On 09/07/2016 11:39 PM, Gavin Flower wrote: > On 08/09/16 09:08, Vik Fearing wrote: >> On 09/07/2016 10:41 PM, Alvaro Herrera wrote: >>> Gavin Flower wrote: >>> >>>> possibly '--nosync' (& any similar) should have a '--no-sync' variation >>>> added, with the '--nosync' variation documented as depreciated? >>> I agree -- I would go as far as just documenting --no-sync only and >>> keeping the --nosync one working with minimal (if any) visibility in >>> docs. >> Okay, here's a patch to do that. I don't think it's the other patch's >> job to do it. >> >> I also changed --noclean to --no-clean, and --no-locale was already >> correct. > > Suggest a comment along the lines "Where flags of the form --xxx have a > negated form, then the preferred negated form is --no-xxx - and that any > existing use of the form --noxxx should be converted to --no-xxx, as the > non hyphenated form is now deprecated & will be removed in a future > version of Postgres." I have verified that these are the only two options anywhere in the tree that start with "no" and not "no-" so it should be pretty easy for future options to conform on their own. I don't see adding a comment like this to every long option definition block to be very helpful, and only adding it to initdb is just weird. So I don't see the need for it. > Possibly generate warningswhen the non hyphenated form is used? I'm not quite sure how I got volunteered to do this work, but it's easy enough so I don't mind. Here is a new patch that emits a warning when --noclean and/or --nosync are used. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support initdb_no_opts_02.patch Description: invalid/octet-stream -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Long options for pg_ctl waiting
On 09/07/2016 10:41 PM, Alvaro Herrera wrote: > Gavin Flower wrote: > >> possibly '--nosync' (& any similar) should have a '--no-sync' variation >> added, with the '--nosync' variation documented as depreciated? > > I agree -- I would go as far as just documenting --no-sync only and > keeping the --nosync one working with minimal (if any) visibility in > docs. Okay, here's a patch to do that. I don't think it's the other patch's job to do it. I also changed --noclean to --no-clean, and --no-locale was already correct. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support pg_ctl_no_opts_01.patch Description: invalid/octet-stream -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] INSERT .. SET syntax
On 09/05/2016 03:58 PM, Simon Riggs wrote: > On 3 July 2016 at 20:36, Marko Tiikkaja <ma...@joh.to> wrote: > >> Here's a patch for $SUBJECT. I'll probably work on the docs a bit more >> before the next CF, but I thought I'd post it anyway. > > I think this should be Returned With Feedback. You're probably right (even though you're quoting the wrong message), so I've changed it to that. Marko, please resubmit an updated patch. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Fun fact about autovacuum and orphan temp tables
On 09/05/2016 01:54 PM, Grigory Smolkin wrote: > What is the purpose of keeping orphan tables around and not dropping > them on the spot? You can read the discussion about it here: https://www.postgresql.org/message-id/flat/3507.1214581513%40sss.pgh.pa.us -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Long options for pg_ctl waiting
One thing that has been irking me ever since I came to PostgreSQL is the fact that pg_ctl -w (and -W) don't have longhand equivalents. I like to use the long version in scripts and such as extra documentation, and I've never been able to with these. What's more, I keep forgetting that --wait (and --no-wait) aren't a thing. Trivial patch attached. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support pg_ctl_wait-01.patch Description: invalid/octet-stream -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sequence data type
On 08/31/2016 06:22 AM, Peter Eisentraut wrote:
> Here is a patch that adds the notion of a data type to a sequence. So
> it might be CREATE SEQUENCE foo AS integer. The types are restricted to
> int{2,4,8} as now.
This patch does not apply cleanly to current master (=600dc4c).
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
--
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] INSERT .. SET syntax
On 08/31/2016 04:12 PM, Marko Tiikkaja wrote: > Hello hello, > > Here's a rebased and updated patch for $SUBJECT for the September commit > fest. Review: This patch is pretty straightforward, using mostly already existing infrastructure. I tried to break it in various ways and could not. I do have a few comments, though. In insert.sgml, some things are and some are . I don't expect this patch to fix those inconsistencies, but it should certainly not perpetuate them. This code comment in gram.y took me a while to figure out: +/* + * This is different from set_clause_list used in UPDATE because the SelectStmt + * syntax already does everything you might want to do in an in INSERT. + */ If the SelectStmt is all we need, why is this patch here? I would prefer wording such as "This is different from set_clause_list used in UPDATE because we don't want multiple_set_clause. The INSERT INTO ... SELECT variant may be more appropriate in such cases." Or something. Aside from those trivialities, the main question about this patch is if we actually want it. It is not standard SQL syntax, and the only other product I've located that uses it (or anything like it) is MySQL. However, I can see how it would be a huge win for very wide tables and so my personal opinion is to accept this syntax as a PostgreSQL extension to the standard. Marking ready for committer, the minor gripes above notwithstanding. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Sample configuration files
On 09/02/2016 08:58 AM, Robert Haas wrote: > On Mon, Aug 29, 2016 at 7:04 AM, Vik Fearing <v...@2ndquadrant.fr> wrote: >> We have sample configuration files for postgresql.conf and >> recovery.conf, but we do not have them for contrib modules. This patch >> attempts to add them. >> >> Although the patch puts the sample configuration files in the tree, it >> doesn't try to install them. That's partly because I think it would >> need an extension version bump and that doesn't seem worth it. > > I don't think that would need an extension version bump; we only need > that if we're changing which SQL objects get created. So my opinion > of this effort is: > > 1. If we're going to add these files, there's no reason not to install > them; in fact, not installing them makes the whole thing rather > pointless, as most people will only see the stuff that gets installed, > not uninstalled files in the source tree. Fair enough. The alternative is for packagers to install them, like they do for recovery.conf.sample. I'll update the patch as soon as we get consensus that this is wanted. > 2. But I'm not sure that this will actually be useful to people. It > seems like it might just be one more thing for patch authors to > maintain. I think that if somebody wants to set a parameter defined > for a contrib module, it's easy enough to just add an entry into > postgresql.conf, or use ALTER SYSTEM .. SET. Going and finding the > sample file (which only sets the value to the default) and then > putting that into your postgresql.conf seems like an extra step. I was imagining just using the "include" directive. I have heard the desire for annotated sample conf files for the contrib modules twice now from different people which is why I wrote the patch. If we decide that the extra documentation is too much of a burden, I can understand that, also. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] autonomous transactions
On 08/31/2016 03:09 PM, Joel Jacobson wrote: > On Wed, Aug 31, 2016 at 6:41 AM, Jaime Casanova > <jaime.casan...@2ndquadrant.com> wrote: >> >> On 30 August 2016 at 23:10, Joel Jacobson <j...@trustly.com> wrote: >>> >>> There should be a way to within the session and/or txn permanently >>> block autonomous transactions. >>> >> >> This will defeat one of the use cases of autonomous transactions: auditing > > My idea on how to deal with this would be to mark the function to be > "AUTONOMOUS" similar to how a function is marked to be "PARALLEL > SAFE", > and to throw an error if a caller that has blocked autonomous > transactions tries to call a function that is marked to be autonomous. > > That way none of the code that needs to be audited would ever get executed. Part of what people want this for is to audit what people *try* to do. We can already audit what they've actually done. With your solution, we still wouldn't know when an unauthorized attempt to do something happened. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Temporary tables that do not bloat pg_catalog (a.k.a fast temp tables)
On 08/24/2016 06:16 PM, Robert Haas wrote: > On Tue, Aug 23, 2016 at 6:11 PM, Tomas Vondra > <tomas.von...@2ndquadrant.com> wrote: >> Could someone please explain how the unlogged tables are supposed to fix the >> catalog bloat problem, as stated in the initial patch submission? We'd still >> need to insert/delete the catalog rows when creating/dropping the temporary >> tables, causing the bloat. Or is there something I'm missing? > > No, not really. Jim just asked if the idea of partitioning the > columns was completely dead in the water, and I said, no, you could > theoretically salvage it. Whether that does you much good is another > question. > > IMV, the point here is that you MUST have globally visible dependency > entries for this to work sanely. If they're not in a catalog, they > have to be someplace else, and backend-private memory isn't good > enough, because that's not globally visible. Until we've got a > strategy for that problem, this whole effort is going nowhere - even > though in other respects it may be a terrific idea. Why not just have a regular-looking table, with a "global temporary" relpersistence (I don't care which letter it gets) and when a backend tries to access it, it uses its own private relfilenode instead of whatever is in pg_class, creating one if necessary. That way the structure of the table is fixed, with all the dependencies and whatnot, but the content is private to each backend. What's wrong with this idea? -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Sample configuration files
We have sample configuration files for postgresql.conf and recovery.conf, but we do not have them for contrib modules. This patch attempts to add them. Although the patch puts the sample configuration files in the tree, it doesn't try to install them. That's partly because I think it would need an extension version bump and that doesn't seem worth it. Also, while writing this patch, it crossed my mind that the plpgsql variables should probably be in the main postgresql.conf file because we install it by default. I will happily write that patch if desired, independently of this patch. Current as of 26fa446, added to next commitfest. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support sample_confs_v01.patch Description: invalid/octet-stream -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] VACUUM's ancillary tasks
The attached two patches scratch two itches I've been having for a while. I'm attaching them together because the second depends on the first. Both deal with the fact that [auto]vacuum has taken on more roles than its original purpose. Patch One: autovacuum insert-heavy tables If you have a table that mostly receives INSERTs, it will never get vacuumed because there are no (or few) dead rows. I have added an "inserts_since_vacuum" field to PgStat_StatTabEntry which works exactly the same way as "changes_since_analyze" does. The reason such a table needs to be vacuumed is currently twofold: the visibility map is not updated, slowing down index-only scans; and BRIN indexes are not maintained, rendering them basically useless. Patch Two: autovacuum after table rewrites This patch addresses the absurdity that a standard VACUUM is required after a VACUUM FULL because the visibility map gets blown away. This is also the case for CLUSTER and some versions of ALTER TABLE that rewrite the table. I thought about having those commands do the same work themselves, but it seems better to have them simply trigger autovacuum than quadruplicate the work. I do this by having them fill in the "inserts_since_vacuum" field added in Patch One with the number of rows rewritten. This assumes that autovacuum_vacuum_scale_factor is < 1.0 which hopefully is a safe assumption. While doing this, I noticed that ALTER TABLE should also re-analyze the table for obvious reasons, so I have that one set "changes_since_analyze" to the number of rows rewritten as well. I have not included any kind of test suite here because I don't really have any ideas how to go about it in a sane way. Suggestions welcome. Attention reviewer: Please note that some of the documentation in the first patch gets removed by the second patch, in case they both don't get committed. I have added this to the imminent commitfest. These patches are rebased as of 26fa446. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support autovac_01_v01.patch Description: invalid/octet-stream autovac_02_v01.patch Description: invalid/octet-stream -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] new autovacuum criterion for visible pages
On 12/08/16 15:15, Peter Eisentraut wrote: > On 8/11/16 11:59 AM, Jeff Janes wrote: >> Insertions and HOT-updates clear vm bits but don't increment the >> counters that those existing parameters are compared to. >> >> Also, the relationship between number of updated/deleted rows and the >> number of hint-bits cleared can be hard to predict due to possible >> clustering of the updates into the same blocks. So it would be hard >> to know what to set the values to. > > Well, the current threshold formulas aren't an exact science either. > They just trigger autovacuum often enough relative to table size and > activity. Just fudging in the insert and HOT update counters times some > factor might be enough, and it would get this functionality out to all > users without more effort. I have a patch I wrote a while ago that does this. I haven't submitted it yet because the documentation is lacking. I will post it over the weekend (I had planned to do it before the commitfest anyway). -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Surprising behaviour of \set AUTOCOMMIT ON
On 08/08/16 17:02, Robert Haas wrote: > On Mon, Aug 8, 2016 at 10:10 AM, Rahila Syed <rahilasye...@gmail.com> wrote: >> Thank you for inputs everyone. >> >> The opinions on this thread can be classified into following >> 1. Commit >> 2. Rollback >> 3. Error >> 4. Warning >> >> As per opinion upthread, issuing implicit commit immediately after switching >> autocommit to ON, can be unsafe if it was not desired. While I agree that >> its difficult to judge users intention here, but if we were to base it on >> some assumption, the closest would be implicit COMMIT in my opinion.There is >> higher likelihood of a user being happy with issuing a commit when setting >> autocommit ON than a transaction being rolled back. Also there are quite >> some interfaces which provide this. >> >> As mentioned upthread, issuing a warning on switching back to autocommit >> will not be effective inside a script. It won't allow subsequent commands to >> be committed as set autocommit to ON is not committed. Scripts will have to >> be rerun with changes which will impact user friendliness. >> >> While I agree that issuing an ERROR and rolling back the transaction ranks >> higher in safe behaviour, it is not as common (according to instances stated >> upthread) as immediately committing any open transaction when switching back >> to autocommit. > > I think I like the option of having psql issue an error. On the > server side, the transaction would still be open, but the user would > receive a psql error message and the autocommit setting would not be > changed. So the user could type COMMIT or ROLLBACK manually and then > retry changing the value of the setting. This is my preferred action. > Alternatively, I also think it would be sensible to issue an immediate > COMMIT when the autocommit setting is changed from off to on. That > was my first reaction. I don't care for this very much. > Aborting the server-side transaction - with or without notice - > doesn't seem very reasonable. Agreed. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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 longer possible to query catalogs for index capabilities?
On 08/08/16 15:38, Tom Lane wrote: > Alvaro Herrera <alvhe...@2ndquadrant.com> writes: >> Tom Lane wrote: >>> Building on the has-property approach Andrew suggested, I wonder if >>> we need something like pg_index_column_has_property(indexoid, colno, >>> propertyname) with properties like "sortable", "desc", "nulls first". > >> This seems simple enough, on the surface. Why not run with this design? > > Well, the next step is to fill in the blanks: what properties need to be > queryable? That's less urgent; adding missing properties does not require a catversion bump. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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
On 27/07/16 06:11, David Fetter wrote: > On Wed, Jul 27, 2016 at 03:24:28AM +0200, Vik Fearing wrote: >> On 27/07/16 03:15, Peter Eisentraut wrote: >>> On 7/26/16 6:14 PM, Vik Fearing wrote: >>>> As mentioned elsewhere in the thread, you can just do WHERE true >>>> to get around it, so why on Earth have it PGC_SUSET? >>> >>> I'm not sure whether it's supposed to guard against typos and >>> possibly buggy SQL string concatenation in application code. So >>> it would help against accidental mistakes, whereas putting a WHERE >>> TRUE in there would be an intentional override. >> >> If buggy SQL string concatenation in application code is your >> argument, quite a lot of them add "WHERE true" so that they can just >> append a bunch of "AND ..." clauses without worrying if it's the >> first (or last, whatever), so I'm not sure this is protecting >> anything. > > I am sure that I'm not the only one who's been asked for this feature > because people other than me have piped up on this thread to that very > effect. Sure. I'm just saying that I think it is poorly designed. I think it would be far better to error out if the command affects x rows, or an estimated y% of the table. Doing that, and also allowing the user to turn it off, would solve the problem as I understand your presentation of it. > I understand that there may well be lots of really meticulous people > on this list, people who would never accidentally do an unqualified > DELETE on a table in production, but I can't claim to be one of them > because I have, and not just once. It's under once a decade, but even > that's too many. That doesn't mean that requiring a WHERE clause -- without even looking at what's in it -- is a good idea. Why not start by turning off autocommit, for example? > I'm not proposing to make this feature default, or even available by > default, but I am totally certain that this is the kind of feature > people would really appreciate, even if it doesn't prevent every > catastrophe. This kind of feature, why not. This feature, no. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Why we lost Uber as a user
On 27/07/16 05:45, Robert Haas wrote: > On Tue, Jul 26, 2016 at 8:27 PM, Stephen Frost <sfr...@snowman.net> wrote: >> * Joshua D. Drake (j...@commandprompt.com) wrote: >>> Hello, >>> >>> The following article is a very good look at some of our limitations >>> and highlights some of the pains many of us have been working >>> "around" since we started using the software. >>> >>> https://eng.uber.com/mysql-migration/ >>> >>> Specifically: >>> >>> * Inefficient architecture for writes >>> * Inefficient data replication >> >> The above are related and there are serious downsides to having an extra >> mapping in the middle between the indexes and the heap. >> >> What makes me doubt just how well they understood the issues or what is >> happening is the lack of any mention of hint bits of tuple freezing >> (requiring additional writes). > > Yeah. A surprising amount of that post seemed to be devoted to > describing how our MVCC architecture works rather than what problem > they had with it. I'm not saying we shouldn't take their bad > experience seriously - we clearly should - but I don't feel like it's > as clear as it could be about exactly where the breakdowns happened. There is some more detailed information in this 30-minute talk: https://vimeo.com/145842299 -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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
On 27/07/16 03:15, Peter Eisentraut wrote: > On 7/26/16 6:14 PM, Vik Fearing wrote: >> As mentioned elsewhere in the thread, you can just do WHERE true to get >> around it, so why on Earth have it PGC_SUSET? > > I'm not sure whether it's supposed to guard against typos and possibly > buggy SQL string concatenation in application code. So it would help > against accidental mistakes, whereas putting a WHERE TRUE in there would > be an intentional override. If buggy SQL string concatenation in application code is your argument, quite a lot of them add "WHERE true" so that they can just append a bunch of "AND ..." clauses without worrying if it's the first (or last, whatever), so I'm not sure this is protecting anything. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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
On 21/07/16 06:57, David Fetter wrote: > Folks, > > Please find attached a patch which makes it possible to disallow > UPDATEs and DELETEs which lack a WHERE clause. As this changes query > behavior, I've made the new GUCs PGC_SUSET. > > What say? I say I don't like this at all. As mentioned elsewhere in the thread, you can just do WHERE true to get around it, so why on Earth have it PGC_SUSET? I would rather, if we need nannying at all, have a threshold of number of rows affected. So if your update or delete exceeds that threshold, the query will be canceled. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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 longer possible to query catalogs for index capabilities?
On 25/07/16 21:20, Andrew Gierth wrote: > Here is my proposed code (first cut; obviously it needs docs too). > Opinions? I support the future of this patch, for 9.6. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] primary_conninfo missing from pg_stat_wal_receiver
On 19/06/16 13:02, Michael Paquier wrote: > On Sun, Jun 19, 2016 at 7:38 PM, Vik Fearing <v...@2ndquadrant.fr> wrote: >> On 19/06/16 12:28, Michael Paquier wrote: >>> On Sun, Jun 19, 2016 at 5:56 PM, Michael Paquier >>> Or in short the attached. >> >> The code looks good to me but why no documentation? > > Because that's a long day... Thanks! Now you have it. Quick bikeshed: I think the column should be called conninfo and not conn_info to match other places it's used. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] primary_conninfo missing from pg_stat_wal_receiver
On 19/06/16 13:02, Michael Paquier wrote: > On Sun, Jun 19, 2016 at 7:38 PM, Vik Fearing <v...@2ndquadrant.fr> wrote: >> On 19/06/16 12:28, Michael Paquier wrote: >>> On Sun, Jun 19, 2016 at 5:56 PM, Michael Paquier >>> Or in short the attached. >> >> The code looks good to me but why no documentation? > > Because that's a long day... Thanks! Now you have it. Thanks, this looks good. Could you please add it to the next commitfest so that it doesn't get lost and also so I can do an official review of it? -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] primary_conninfo missing from pg_stat_wal_receiver
On 19/06/16 12:28, Michael Paquier wrote: > On Sun, Jun 19, 2016 at 5:56 PM, Michael Paquier > <michael.paqu...@gmail.com> wrote: >> The new pg_stat_wal_receiver does not include primary_conninfo. >> Looking at that now, it looks almost stupid not to include it... >> Adding it now would require a catalog bump, so I am not sure if this >> is acceptable at this stage for 9.6... This definitely needs to go in, we get regular requests for it. The last one I've seen being https://www.postgresql.org/message-id/CAN1xZseiqNRh1ZE0seVk7UuHeWvFBEWG%2BFcW7Xfm_Nv%3Dd%2BfPGw%40mail.gmail.com Whether it goes into 9.6 or 10 is not for me to say. > Or in short the attached. The code looks good to me but why no documentation? -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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: integration bloat tables (indexes) to core
On 16/06/16 20:31, Jim Nasby wrote: > On 6/13/16 12:16 PM, Tom Lane wrote: >> At the same time, I'm pretty suspicious of putting stuff like this in >> core, because the expectations for cross-version compatibility go up >> by orders of magnitude as soon as we do that. > > On a first go-round, I don't think we should add entire views, but > rather functions that serve specific purposes. For table bloat that > means a function that returns what the heap size should be based on > pg_stats. For locking, it means providing information about which PID is > blocking which PID. After that, most everything else is just window > dressing. We already have that second one: pg_blocking_pids(integer) -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] 10.0
On 15/06/16 02:08, Cat wrote: > is it possible to introduce a JSONB output to it. No thanks. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Changed SRF in targetlist handling
On 06/06/16 18:30, David G. Johnston wrote: > To clarify, the present behavior is basically a combination of both of > Robert's results. > > If the SRFs return the same number of rows the first (zippered) result > is returned without an NULL padding. > > If the SRFs return a different number of rows the LCM behavior kicks in > and you get Robert's second result. No. > SELECT generate_series(1, 4), generate_series(1, 4) ORDER BY 1, 2; > is the same as > SELECT * FROM ROWS FROM ( generate_series(1, 4), generate_series(1, 4) ); > > BUT > > ​SELECT generate_series(1, 3), generate_series(1, 4) ORDER BY 1, 2; > is the same as > SELECT * FROM ROWS FROM generate_series(1, 3) a, LATERAL ROWS FROM > generate_series(1, 4) b; What would you do with: SELECT generate_series(1, 3), generate_series(1, 6); ? > Tom's 2.5 proposal basically says we make the former equivalence succeed > and have the later one fail. > > The rewrite would be unaware of the cardinality of the SRF and so it > cannot conditionally rewrite the query. One of the two must be chosen > and the incompatible behavior turned into an error. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rename synchronous_standby_names?
On 01/06/16 02:49, Michael Paquier wrote: > On Wed, Jun 1, 2016 at 3:56 AM, David G. Johnston > <david.g.johns...@gmail.com> wrote: >> On Tue, May 31, 2016 at 2:19 PM, Peter Eisentraut >> <peter.eisentr...@2ndquadrant.com> wrote: >>> >>> On 5/31/16 1:47 PM, Jaime Casanova wrote: >>>> >>>> Are we going to change synchronous_standby_names? Certainly the GUC is >>>> not *only* a list of names anymore. >>>> >>>> synchronous_standby_config? >>>> synchronous_standbys (adjust to correct english if necesary)? >>> >>> If the existing values are still going to be accepted, then I would leave >>> it as is. >> >> +1 > > +1. We've made quite a lot of deal to take an approach for the N-sync > that is 100% backward-compatible, it would be good to not break that > effort. We could always accept it like we do for archive/hot_standby->replica. I like synchronous_standby_config, so I vote for changing it. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Reviewing freeze map code
On 17/05/16 21:32, Alvaro Herrera wrote: > Is SCAN_ALL really the best we can do here? The business of having an > underscore in an option name has no precedent (other than > CURRENT_DATABASE and the like). ALTER DATABASE has options for ALLOW_CONNECTIONS, CONNECTION_LIMIT, and IS_TEMPLATE. > How about COMPLETE, TOTAL, or WHOLE? Sure, I'll play this game. I like EXHAUSTIVE. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] 10.0
On 05/13/2016 08:31 PM, Alvaro Herrera wrote: > Josh berkus wrote: > >> Anyway, can we come up with a consensus of some minimum changes it will >> take to make the next version 10.0? > > I think the next version should be 10.0 no matter what changes we put > in. +1 Let's even stamp it 10.0devel from the get-go. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Idle In Transaction Session Timeout, revived
On 03/16/2016 05:32 PM, Robert Haas wrote: > Committed with slight changes to the docs, and I added a flag variable > instead of relying on IdleInTransactionSessionTimeout not changing at > an inopportune time. Thank you! -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Idle In Transaction Session Timeout, revived
On 03/08/2016 10:42 PM, Robert Haas wrote: > On Sun, Jan 31, 2016 at 8:33 AM, Vik Fearing <v...@2ndquadrant.fr> wrote: >> Attached is a rebased and revised version of my >> idle_in_transaction_session_timeout patch from last year. >> >> This version does not suffer the problems the old one did where it would >> jump out of SSL code thanks to Andres' patch in commit >> 4f85fde8eb860f263384fffdca660e16e77c7f76. >> >> The basic idea is if a session remains idle in a transaction for longer >> than the configured time, that connection will be dropped thus releasing >> the connection slot and any locks that may have been held by the broken >> client. >> >> Added to the March commitfest. Attached is version 6 of this patch. > I see this patch has been marked Ready for Committer despite the lack > of any really substantive review. Generally, I think it looks good. > But I have a couple of questions/comments: > > - I really wonder if the decision to ignore sessions that are idle in > transaction (aborted) should revisited. Consider this: > > rhaas=# begin; > BEGIN > rhaas=# lock table pg_class; > LOCK TABLE > rhaas=# savepoint a; > SAVEPOINT > rhaas=# select 1/0; > ERROR: division by zero Revisited. All idle transactions are now affected, even aborted ones. I had not thought about subtransactions. > - I wonder if the documentation should mention potential advantages > related to holding back xmin. I guess I kind of punted on this in the new patch. I briefly mention it and then link to the routine-vacuuming docs. I can reword that if necessary. > - What's the right order of events in PostgresMain? Right now the > patch disables the timeout after checking for interrupts and clearing > DoingCommandRead, but maybe it would be better to disable the timeout > first, so that we can't have it happen that start to execute the > command and then, in medias res, bomb out because of the idle timeout. > Then again, maybe you had some compelling reason for doing it this > way, in which case we should document that in the comments. There is no better reason for putting it there than "it seemed like a good idea at the time". I've looked into it a bit more, and I don't see any danger of having it there, but I can certainly move it if you think I should. > - It would be nice if you reviewed someone else's patch in turn. I have been reviewing other, small patches. I am signed up for several in this commitfest that I will hopefully get to shortly, and I have reviewed others in recent fests where I had no patch of my own. I may be playing on the penny slots, but I'm still putting my coins in. > I'm attaching a lightly-edited version of your patch. I have incorporated your changes. I also changed the name IdleInTransactionTimeoutSessionPending to the thinko-free IdleInTransactionSessionTimeoutPending. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 6c73fb4..aaa3a71 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -6063,6 +6063,27 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; + + idle_in_transaction_session_timeout (integer) + + idle_in_transaction_session_timeout configuration parameter + + + + + Terminate any session with an open transaction that has been idle for + longer than the specified duration in milliseconds. This allows any + locks to be released and the connection slot to be reused. In particular, + it allows tuples visible only to this transaction to be vacuumed. See +for more details about this. + + + The default value of 0 means that sessions which are idle in transaction + will will not be terminated. + + + + vacuum_freeze_table_age (integer) diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml index dd3c775..6352e12 100644 --- a/doc/src/sgml/mvcc.sgml +++ b/doc/src/sgml/mvcc.sgml @@ -725,7 +725,9 @@ ERROR: could not serialize access due to read/write dependencies among transact Don't leave connections dangling idle in transaction - longer than necessary. + longer than necessary. The configuration parameter +may be used to + automatically disconnect lingering sessions. diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index 74ef419..a66e07b 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -58,6 +58,7 @@ int DeadlockTimeout = 1000; int StatementTimeout = 0; int LockTimeout = 0; +int IdleInTransactionSess
Re: [HACKERS] create opclass documentation outdated
On 02/25/2016 09:13 PM, Julien Rouhaud wrote: > Hello, > > I just saw that the CREATE OPERATOR CLASS documentation doesn't mention > that BRIN indexes also support the STORAGE parameter. Good catch! > Patch attached. I think this is the wrong approach; that parenthetical list now represents a full 50% of the available AMs. I submit it should be removed altogether. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Re: Add generate_series(date, date) and generate_series(date, date, integer)
On 02/21/2016 07:56 PM, Corey Huinker wrote: > > Other than that, the only difference is the ::date part. Is it > really worth adding extra code just for that? I would say not. > > > I would argue it belongs for the sake of completeness. So would I. +1 for adding this missing function. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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: make NOTIFY list de-duplication optional
On 02/08/2016 09:33 PM, Filip Rembiałkowski wrote: > Here is my next try, after suggestions from -perf and -hackers list: > > * no GUC > > * small addition to NOTIFY grammar: NOTIFY ALL/DISTINCT > > * corresponding, 3-argument version of pg_notify(text,text,bool) > > * updated the docs to include new syntax and clarify behavior > > * no hashtable in AsyncExistsPendingNotify > (I don't see much sense in that part; and it can be well done > separately from this) Please add this to the next commitfest: https://commitfest.postgresql.org/9/new/ -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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: make NOTIFY list de-duplication optional
On 02/07/2016 04:00 PM, Filip Rembiałkowski wrote: > On Sun, Feb 7, 2016 at 11:54 AM, Vik Fearing <v...@2ndquadrant.fr> wrote: >> I seem to remember some discussion about not using DEFAULT parameters in >> system functions so you should leave the old function alone and create a >> new function with your use_all parameter. I don't recall the exact >> reason why so hopefully someone else will enlighten me. > > I'm quite new to this; how do I pinpoint proper OID for a new catalog > object (function, in this case)? > Is there a better way than browsing fmgr files and guessing next available > oid? There is a shell script called `unused_oids` in src/include/catalog/. >> There is also no mention in the documentation about what happens if I do: >> >> NOTIFY ALL chan, 'msg'; >> NOTIFY ALL chan, 'msg'; >> NOTIFY DISTINCT chan, 'msg'; >> NOTIFY ALL chan, 'msg'; >> >> Without testing, I'd say I'd get two messages, but it should be >> explicitly mentioned somewhere. > > If it's four separate transactions, LISTEN'er should get four. The question was for one transaction, I should have been clearer about that. > If it's in one transaction, LISTEN'er should get three. This is surprising to me, I would think it would get only two. What is your rationale for three? Compare with the behavior of: select 1 union all select 1 union distinct select 1 union all select 1; -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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: make NOTIFY list de-duplication optional
On 02/07/2016 03:42 AM, Filip Rembiałkowski wrote: > +1 > > ... and a patch (only adding ALL keyword, no hash table implemented yet). Please stop top-posting, it's very disruptive. My comments are below, where they belong. > On Sat, Feb 6, 2016 at 2:35 PM, Brendan Jurd <dire...@gmail.com> wrote: >> On Sat, 6 Feb 2016 at 12:50 Tom Lane <t...@sss.pgh.pa.us> wrote: >>> >>> Robert Haas <robertmh...@gmail.com> writes: >>>> I agree with what Merlin said about this: >>>> >>>> http://www.postgresql.org/message-id/CAHyXU0yoHe8Qc=yc10ahu1nfia1tbhsg+35ds-oeueuapo7...@mail.gmail.com >>> >>> Yeah, I agree that a GUC for this is quite unappetizing. >> >> >> How would you feel about a variant for calling NOTIFY? >> >> The SQL syntax could be something like "NOTIFY [ALL] channel, payload" where >> the ALL means "just send the notification already, nobody cares whether >> there's an identical one in the queue". >> >> Likewise we could introduce a three-argument form of pg_notify(text, text, >> bool) where the final argument is whether you are interested in removing >> duplicates. >> >> Optimising the remove-duplicates path is still probably a worthwhile >> endeavour, but if the user really doesn't care at all about duplication, it >> seems silly to force them to pay any performance price for a behaviour they >> didn't want, no? On 02/07/2016 03:42 AM, Filip Rembiałkowski wrote: > +1 > > ... and a patch (only adding ALL keyword, no hash table implemented yet). I only read through the patch, I didn't compile it or test it, but I have a few comments: You left the duplicate behavior with subtransactions, but didn't mention it in the documentation. If I do NOTIFY DISTINCT chan, 'msg'; then I expect only distinct notifications but I'll get duplicates if I'm in a subtransaction. Either the documentation or the code needs to be fixed. I seem to remember some discussion about not using DEFAULT parameters in system functions so you should leave the old function alone and create a new function with your use_all parameter. I don't recall the exact reason why so hopefully someone else will enlighten me. There is also no mention in the documentation about what happens if I do: NOTIFY ALL chan, 'msg'; NOTIFY ALL chan, 'msg'; NOTIFY DISTINCT chan, 'msg'; NOTIFY ALL chan, 'msg'; Without testing, I'd say I'd get two messages, but it should be explicitly mentioned somewhere. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Idle In Transaction Session Timeout, revived
On 02/04/2016 02:24 PM, Fujii Masao wrote: > On Sun, Jan 31, 2016 at 10:33 PM, Vik Fearing <v...@2ndquadrant.fr> wrote: >> Attached is a rebased and revised version of my >> idle_in_transaction_session_timeout patch from last year. >> >> This version does not suffer the problems the old one did where it would >> jump out of SSL code thanks to Andres' patch in commit >> 4f85fde8eb860f263384fffdca660e16e77c7f76. >> >> The basic idea is if a session remains idle in a transaction for longer >> than the configured time, that connection will be dropped thus releasing >> the connection slot and any locks that may have been held by the broken >> client. > > +1 > > But, IIRC, one of the problems that prevent the adoption of this feature is > the addition of gettimeofday() call after every SQL command receipt. > Have you already resolved that problem? Or we don't need to care about > it because it's almost harmless? I guess it would be possible to look at MyBEEntry somehow and pull st_state_start_timestamp from it to replace the call to GetCurrentTimestamp(), but I don't know if it's worth doing that. The extra call only happens if the timeout is enabled anyway, so I don't think it matters enough to be a blocker. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Idle In Transaction Session Timeout, revived
On 02/03/2016 11:36 PM, Jim Nasby wrote: > On 2/3/16 4:05 PM, David Steele wrote: >> On 2/3/16 4:25 PM, Tom Lane wrote: >>> Robert Haas <robertmh...@gmail.com> writes: >>>> On Wed, Feb 3, 2016 at 3:41 PM, Jim Nasby <jim.na...@bluetreble.com> >>>> wrote: >>>>> Wouldn't it be more sensible to just roll the transaction back and not >>>>> disconnect? >>> >>> I'm not sure how messy this would be in practice. But if we think that >>> killing the whole session is not desirable but something we're doing for >>> expediency, then it would be worth looking into that approach. >> >> I think killing the session is a perfectly sensible thing to do in this >> case. Everything meaningful that was done in the session will be rolled >> back - no need to waste resources keeping the connection open. That was the consensus last time I presented this bikeshed for painting. > Except you end up losing stuff like every GUC you've set, existing temp > tables, etc. For an application that presumably doesn't matter, but for > a user connection it would be a PITA. > > I wouldn't put a bunch of effort into it though. Dropping the connection > is certainly better than nothing. You could always put SET idle_in_transaction_session_timeout = 0; in your .psqlrc file to exempt your manual sessions from it. Or change it just for your user or something. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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_dump data structures for triggers
On 02/04/2016 01:44 AM, Tom Lane wrote: > I'm looking into fixing the problem reported here: > http://www.postgresql.org/message-id/1445a624-d09f-4b51-9c41-46ba1e2d6...@neveragain.de > namely that if we split a view into a table + rule (because of circular > dependencies), parallel pg_restore fails to ensure that it creates any > triggers for the view only after creating the rule. If it tries to > create the triggers first, the backend may barf because they're the wrong > type of triggers for a plain table. > > While it's not that hard to make pg_dump add some more dependencies while > breaking a circular dependency, there's a small problem of finding the > view's triggers so we can attach those dependencies to them. AFAICS, with > the current pg_dump data structures, there is actually no better way to > do that than to iterate through every DumpableObject known to pg_dump to > see which of them are TriggerInfos pointing to the view relation :-(. > > What I have in mind is to add linked-list fields to TableInfo and > TriggerInfo to allow all the triggers of a table to be found by chasing > a list. The increment in the size of those data structures isn't much, > and we may have other uses for such an ability later. > > Anybody have an objection or better idea? No objections to this, but my "better idea" is simply to allow INSTEAD OF triggers on tables like discussed last year. http://www.postgresql.org/message-id/14c6fe168a9-1012-10...@webprd-a87.mail.aol.com -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Idle In Transaction Session Timeout, revived
Attached is a rebased and revised version of my
idle_in_transaction_session_timeout patch from last year.
This version does not suffer the problems the old one did where it would
jump out of SSL code thanks to Andres' patch in commit
4f85fde8eb860f263384fffdca660e16e77c7f76.
The basic idea is if a session remains idle in a transaction for longer
than the configured time, that connection will be dropped thus releasing
the connection slot and any locks that may have been held by the broken
client.
Added to the March commitfest.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index 392eb70..9cccf0e 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -5908,6 +5908,29 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
+
+ idle_in_transaction_session_timeout (integer)
+
+ idle_in_transaction_session_timeout configuration parameter
+
+
+
+
+ Terminate any session with an open transaction that has been idle for
+ longer than the specified duration in milliseconds. This allows any locks to
+ be released and the connection slot to be reused.
+
+
+ Sessions in the state "idle in transaction (aborted)" occupy a connection
+ slot but because they hold no locks, they are not considered by this
+ parameter.
+
+
+ The default value of 0 means that such sessions will not be terminated.
+
+
+
+
vacuum_freeze_table_age (integer)
diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml
index dd3c775..6352e12 100644
--- a/doc/src/sgml/mvcc.sgml
+++ b/doc/src/sgml/mvcc.sgml
@@ -725,7 +725,9 @@ ERROR: could not serialize access due to read/write dependencies among transact
Don't leave connections dangling idle in transaction
- longer than necessary.
+ longer than necessary. The configuration parameter
+may be used to
+ automatically disconnect lingering sessions.
diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c
index 3690753..b711da4 100644
--- a/src/backend/storage/lmgr/proc.c
+++ b/src/backend/storage/lmgr/proc.c
@@ -57,6 +57,7 @@
int DeadlockTimeout = 1000;
int StatementTimeout = 0;
int LockTimeout = 0;
+int IdleInTransactionSessionTimeout = 0;
bool log_lock_waits = false;
/* Pointer to this process's PGPROC and PGXACT structs, if any */
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 390816b..7f03d8e 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2978,6 +2978,11 @@ ProcessInterrupts(void)
}
}
+ if (IdleInTransactionTimeoutSessionPending)
+ ereport(FATAL,
+(errcode(ERRCODE_IDLE_IN_TRANSACTION_SESSION_TIMEOUT),
+ errmsg("terminating connection due to idle-in-transaction timeout")));
+
if (ParallelMessagePending)
HandleParallelMessages();
}
@@ -3947,6 +3952,11 @@ PostgresMain(int argc, char *argv[],
{
set_ps_display("idle in transaction", false);
pgstat_report_activity(STATE_IDLEINTRANSACTION, NULL);
+
+/* Start the idle-in-transaction timer */
+if (IdleInTransactionSessionTimeout > 0)
+ enable_timeout_after(IDLE_IN_TRANSACTION_SESSION_TIMEOUT,
+ IdleInTransactionSessionTimeout);
}
else
{
@@ -3987,7 +3997,13 @@ PostgresMain(int argc, char *argv[],
DoingCommandRead = false;
/*
- * (5) check for any other interesting events that happened while we
+ * (5) turn off the idle-in-transaction timeout
+ */
+ if (IdleInTransactionSessionTimeout > 0)
+disable_timeout(IDLE_IN_TRANSACTION_SESSION_TIMEOUT, false);
+
+ /*
+ * (6) check for any other interesting events that happened while we
* slept.
*/
if (got_SIGHUP)
@@ -3997,7 +4013,7 @@ PostgresMain(int argc, char *argv[],
}
/*
- * (6) process the command. But ignore it if we're skipping till
+ * (7) process the command. But ignore it if we're skipping till
* Sync.
*/
if (ignore_till_sync && firstchar != EOF)
diff --git a/src/backend/utils/errcodes.txt b/src/backend/utils/errcodes.txt
index 04c9c00..1a920e8 100644
--- a/src/backend/utils/errcodes.txt
+++ b/src/backend/utils/errcodes.txt
@@ -229,6 +229,7 @@ Section: Class 25 - Invalid Transaction State
25007EERRCODE_SCHEMA_AND_DATA_STATEMENT_MIXING_NOT_SUPPORTED schema_and_data_statement_mixing_not_supported
25P01EERRCODE_NO_ACTIVE_SQL_TRANSACTION no_active_sql_transaction
25P02EERRCODE_IN_FAILED_SQL_TRANSACTION in_failed_sql_transaction
+25P03EERRCODE_IDLE_IN_TRANSACTION_SESSION_TIMEOUTidle_in_tran
Re: [HACKERS] Extracting fields from 'infinity'::TIMESTAMP[TZ]
On 01/22/2016 04:28 AM, Tom Lane wrote: > Vik Fearing <v...@2ndquadrant.fr> writes: >> I looked around for other places where this code should be used and >> didn't find any. I am marking this patch Ready for Committer. > > I pushed this with some adjustments, mainly to make sure that the > erroneous-units errors exactly match those that would be thrown in > the main code line. Thanks! -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Extracting fields from 'infinity'::TIMESTAMP[TZ]
On 01/05/2016 09:07 AM, Vitaly Burovoy wrote: > On 1/4/16, Alvaro Herrera <alvhe...@2ndquadrant.com> wrote: >> It seems we got majority approval on the design of this patch, and no >> disagreement; the last submitted version appears to implement that. >> There's no documentation change in the patch though. I'm marking it as >> Waiting on Author; please resubmit with necessary doc changes. > > Thank you! > Version 3 of the patch with touched documentation in the attachment. > > I decided to mark it as a note, because that separation > (monotonic/oscillation fields) is not obvious and for most values the > function "extract" works as expected (e.g. does not give an error) > until special values are (casually?) passed. I have reviewed this patch. It applies and compiles cleanly and implements the behavior reached by consensus. The documentation is a little light, but I don't see what else needs to be said. The code is clean and well commented. All extraction options are supported. Regression tests are present and seemingly complete. I looked around for other places where this code should be used and didn't find any. I am marking this patch Ready for Committer. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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
On 10/28/2015 10:03 AM, Marko Tiikkaja wrote: > Hello hello, > > Since the default t/f output for booleans is not very user friendly, > attached is a patch which enables you to do for example the following: > > =# \pset true TRUE > Boolean TRUE display is "TRUE". > =# \pset false FALSE > Boolean FALSE display is "FALSE". > =# select true, false; > bool | bool > --+--- > TRUE | FALSE > (1 row) I think the battle is already lost, but I'm casting my vote in favor of this patch anyway. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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
On 11/12/2015 05:41 PM, Matthijs van der Vleuten wrote: > >> On 12 Nov 2015, at 14:21, Brendan Jurd <dire...@gmail.com> wrote: >> >> On Fri, 30 Oct 2015 at 00:51 Tom Lane <t...@sss.pgh.pa.us >> <mailto:t...@sss.pgh.pa.us>> wrote: >> The really key argument that hasn't been addressed here is why does such >> a behavior belong in psql, rather than elsewhere? Surely legibility >> problems aren't unique to psql users. Moreover, there are exactly >> parallel facilities for other datatypes on the server side: think >> DateStyle or bytea_output. So if you were trying to follow precedent >> rather than invent a kluge, you'd have submitted a patch to create a GUC >> that changes the output of boolout(). >> >> I find Tom's analogy to datestyle and bytea_output convincing. >> >> +1 for a GUC that changes the behaviour of boolout. > > -1 for changing boolout(). It will break anything that receives boolean > values from the server. How a client is going to display values (of any type) > is logic that should belong in the client, not in the protocol. I fully agree. This is something I feel should happen in the client. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions
On 11/05/2015 09:01 PM, Merlin Moncure wrote: > Tom noted earlier some caveats with the 'idle' timeout in terms of > implementation. Maybe that needs to be zeroed in on. AFAIK, those issues have already been solved by Andres some time ago. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Request: pg_cancel_backend variant that handles 'idle in transaction' sessions
On 10/30/2015 10:20 PM, Merlin Moncure wrote: > Idle hanging transactions from poorly written applications are the > bane of my existence. Several months back one of them took down one > of hour production websites for several hours. > > Unfortunately, the only way to deal with them is to terminate the > backend which is heavy handed and in some cases causes further damage. > Something like pg_cancel_transaction(pid) would be nice; it would > end the transaction regardless if in an actual statement or not. > Similarly, transaction_timeout would be a lot more effective than > statement_timeout. It's nice to think about a world where > applications don't do such things, but in this endless sea of > enterprise java soup I live it it's, uh, not realistic. This would be > lot cleaner than the cron driven sweep I'm forced to implement now, > and could be made to be part of the standard configuration across the > enterprise. I would like to request that no one work on this. I wrote a patch to do just that a year and a half ago[1] which was rejected for technical reasons. Since then, Andres has fixed those reasons, and prodded me last week at PGConf.EU to pick my patch back up. I am planning on resubmitting it for the next commitfest. I will also take into account the things said on this thread. [1] http://www.postgresql.org/message-id/538dc843.2070...@dalibo.com -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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.
On 09/15/2015 10:57 AM, David Rowley wrote: >> I agree, that form >> > CREATE UNIQUE INDEX i ON t (f1, f2, f3) INCLUDE (f4) >> > is clear. f4 will be used in row compare and actually planner will be able >> > to use it as unique index (f1, f2, f3) with additional f4 or as >> > as unique index (f1, f2, f3, f4), because uniqueness on (f1, f2, f3) gives >> > warranty of uniqueness on (f1, f2, f3, f4) >> > >> > > I'd vote for this too. However, INCLUDE does not seem to be a reserved word > at the moment. What about CREATE UNIQUE INDEX i ON t (f1, f2, f3) WITH (f4); ? -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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.
On 09/15/2015 12:45 PM, Anastasia Lubennikova wrote: > 15.09.2015 12:11, Vik Fearing: >> On 09/15/2015 10:57 AM, David Rowley wrote: >>>> I agree, that form >>>>> CREATE UNIQUE INDEX i ON t (f1, f2, f3) INCLUDE (f4) >>>>> is clear. f4 will be used in row compare and actually planner will >>>>> be able >>>>> to use it as unique index (f1, f2, f3) with additional f4 or as >>>>> as unique index (f1, f2, f3, f4), because uniqueness on (f1, f2, >>>>> f3) gives >>>>> warranty of uniqueness on (f1, f2, f3, f4) >>>>> >>>>> >>> I'd vote for this too. However, INCLUDE does not seem to be a >>> reserved word >>> at the moment. >> What about CREATE UNIQUE INDEX i ON t (f1, f2, f3) WITH (f4); ? > > WITH seems ambiguity to me. It refers to CTE, so I expect to see after > that a kind of query expression. But maybe that's just matter of habit. Not necessarily. See WITH ORDINALITY, for example. However, now that I've looked at it, index creation already takes a WITH clause for storage parameters, so that's out. > BTW, that's the first syntax change I'm working with. > Is there any convention in PostgreSQL about new keywords and so on? > Where can I find it? I don't think it's written anywhere except peppered throughout the archives. New keywords are greatly frowned upon. INCLUDING is already an unreserved keyword, and sounds more natural than INCLUDE anyway. Maybe that could work? -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- 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] Tab completion for CREATE SEQUENCE
On 08/04/2015 06:30 PM, Robert Haas wrote: On Mon, Aug 3, 2015 at 2:17 AM, Michael Paquier michael.paqu...@gmail.com wrote: On Tue, Jul 7, 2015 at 9:14 PM, Andres Freund and...@anarazel.de wrote: On 2015-06-19 06:41:19 +, Brendan Jurd wrote: I'm marking this Waiting on Author. Once the problems have been corrected, it should be ready for a committer. Vik, are you going to update the patch? Seeing no activity on this thread and as it would be a waste not to do it, I completed the patch as attached. The following things are done: - Fixed code indentation - Removal of RESTART, SET SCHEMA, OWNER TO, and RENAME TO in CREATE SEQUENCE - Added START WITH. - Added TEMP/TEMPORARY in the set of keywords tracked. I am switching at the same time this patch as Ready for committer. You didn't remove RESTART, so I did that. And this needed some more parentheses to make my compiler happy. Committed with those changes. Thank you, Brendan, Michael, and Robert for taking care of this while I was away. -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Insufficient locking for ALTER DEFAULT PRIVILEGES
I came across the following bug this week: Session 0: begin; create schema bug; alter default privileges in schema bug grant all on tables to postgres; commit; Session 1: begin; alter default privileges in schema bug grant all on tables to postgres; Session 2: alter default privileges in schema bug grant all on tables to postgres; hangs Session 1: commit; Session 2: ERROR: tuple concurrently updated -- Vik Fearing +33 6 46 75 15 36 http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Tab completion for CREATE SEQUENCE
While reviewing the seqam patches, I noticed that psql has tab
completion for ALTER SEQUENCE, but not for CREATE SEQUENCE.
The attached trivial patch fixes that.
--
Vik Fearing +33 6 46 75 15 36
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***
*** 2445,2450 psql_completion(const char *text, int start, int end)
--- 2445,2471
pg_strcasecmp(prev_wd, TO) == 0)
COMPLETE_WITH_SCHEMA_QUERY(Query_for_list_of_tables, NULL);
+ /* CREATE SEQUENCE name */
+ else if (pg_strcasecmp(prev3_wd, CREATE) == 0
+ pg_strcasecmp(prev2_wd, SEQUENCE) == 0)
+ {
+ static const char *const list_CREATESEQUENCE[] =
+ {INCREMENT, MINVALUE, MAXVALUE, RESTART, NO, CACHE, CYCLE,
+ SET SCHEMA, OWNED BY, OWNER TO, RENAME TO, NULL};
+
+ COMPLETE_WITH_LIST(list_CREATESEQUENCE);
+ }
+ /* CREATE SEQUENCE name NO */
+ else if (pg_strcasecmp(prev4_wd, CREATE) == 0
+ pg_strcasecmp(prev3_wd, SEQUENCE) == 0
+ pg_strcasecmp(prev_wd, NO) == 0)
+ {
+ static const char *const list_CREATESEQUENCE2[] =
+ {MINVALUE, MAXVALUE, CYCLE, NULL};
+
+ COMPLETE_WITH_LIST(list_CREATESEQUENCE2);
+ }
+
/* CREATE SERVER name */
else if (pg_strcasecmp(prev3_wd, CREATE) == 0
pg_strcasecmp(prev2_wd, SERVER) == 0)
--
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] Anonymous code block with parameters
On 09/18/2014 10:16 PM, Hannu Krosing wrote: On 09/18/2014 02:37 PM, Pavel Stehule wrote: if we would to need a single use function, then we should to implement it, and we should not to rape some different objects. Some, what has behave like function should be function. After some thinking, probably CTE design can be only one frame, where we can do it WITH FUNCTION f1(a int) RETURNS int AS $$ .. $$ LANGUAGE plpgsql, FUNCTION f2(a int) RETURNS SETOF int AS $$ .. $$ LANGUAGE plpgsql, SELECT f1(x) FROM f2(z) LATERAL We can generalize WITH clause, so there SEQENCES, VIEWS, .. can be defined for single usage +2 I just proposed the same thing in another branch of this discussion before reading this :) I guess it proves (a little) that WITH is the right place to do these kind of things ... I've been wanting this syntax for a few years now, so I certainly vote for it. -- Vik -- 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] Anonymous code block with parameters
On 09/16/2014 10:09 AM, Heikki Linnakangas wrote: On 09/16/2014 10:57 AM, Craig Ringer wrote: On 09/16/2014 03:15 PM, Pavel Stehule wrote: Why we don't introduce a temporary functions instead? I think that'd be a lot cleaner and simpler. It's something I've frequently wanted, and as Hekki points out it's already possible by creating the function in pg_temp, there just isn't the syntax sugar for CREATE TEMPORARY FUNCTION. So why not just add CREATE TEMPORARY FUNCTION? Sure, why not. Because you still have to do SELECT pg_temp.my_temp_function(blah); to execute it. It means two steps: CREATE TEMPORARY FUNCTION ... $$ $$; SELECT my_temp_function(blah); That won't work; see above. -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #10823: Better REINDEX syntax.
On 09/08/2014 06:17 AM, Stephen Frost wrote: * Vik Fearing (vik.fear...@dalibo.com) wrote: On 09/02/2014 10:17 PM, Marko Tiikkaja wrote: Yeah, I think I like this better than allowing all of them without the database name. Why? It's just a noise word! Eh, because it ends up reindexing system tables too, which is probably not what new folks are expecting. No behavior is changed at all. REINDEX DATABASE dbname; has always hit the system tables. Since dbname can *only* be the current database, there's no logic nor benefit in requiring it to be specified. Also, it's not required when you say 'user tables', so it's similar to your user_tables v1 patch in that regard. The fact that REINDEX USER TABLES; is the only one that doesn't require the dbname seems very inconsistent and confusing. Yes, I will update the patch. Still planning to do this..? Marking this back to waiting-for-author. Yes, but probably not for this commitfest unfortunately. -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #10823: Better REINDEX syntax.
On 09/02/2014 10:17 PM, Marko Tiikkaja wrote: On 2014-08-29 01:00, Alvaro Herrera wrote: Vik Fearing wrote: Here are two patches for this. The first one, reindex_user_tables.v1.patch, implements the variant that only hits user tables, as suggested by you. The second one, reindex_no_dbname.v1.patch, allows the three database-wide variants to omit the database name (voted for by Daniel Migowski, Bruce, and myself; voted against by you). This patch is to be applied on top of the first one. Not a fan. Here's a revised version that provides REINDEX USER TABLES, which can only be used without a database name; other modes are not affected i.e. they continue to require a database name. Yeah, I think I like this better than allowing all of them without the database name. Why? It's just a noise word! I also renamed your proposed reindexdb's --usertables to --user-tables. I agree with this change. Me, too. Oh, I just noticed that if you say reindexdb --all --user-tables, the latter is not honored. Must fix before commit. Definitely. Okay, I'll look at that. Is someone going to prepare an updated patch? Vik? Yes, I will update the patch. -- Vik -- 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] GSoC on WAL-logging hash indexes
On 08/20/2014 02:43 AM, Michael Paquier wrote: On Thu, Jun 19, 2014 at 6:40 PM, Vik Fearing vik.fear...@dalibo.com mailto:vik.fear...@dalibo.com wrote: On 04/30/2014 11:41 PM, Tom Lane wrote: We really oughta fix the WAL situation, not just band-aid around it. After re-reading this thread, it is not clear that anyone is going to work on it so I'll just ask: Is anyone working on this? If not, I'd like to put it on my plate. Vik, did you get time to look at that finally? Yes, I am (very slowly) working on this. I've got a decent learning curve for WAL replay to get over and I figured this can't be urgent considering how many years it's been like this so I'm sort of taking my time. -- Vik -- 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] ALTER SYSTEM RESET?
On 09/02/2014 04:12 PM, Alvaro Herrera wrote: Fujii Masao wrote: On Mon, Sep 1, 2014 at 10:54 PM, Amit Kapila amit.kapil...@gmail.com wrote: On Mon, Sep 1, 2014 at 3:57 PM, Fujii Masao masao.fu...@gmail.com wrote: On Sat, Aug 30, 2014 at 12:27 PM, Amit Kapila amit.kapil...@gmail.com wrote: On Wed, Aug 27, 2014 at 7:16 PM, Fujii Masao masao.fu...@gmail.com wrote: The patch looks good to me. One minor comment is; probably you need to update the tab-completion code. Thanks for the review. I have updated the patch to support tab-completion. As this is a relatively minor change, I will mark it as Ready For Committer rather than Needs Review. Thanks for updating the patch! One more minor comment is; what about applying the following change for the tab-completion for RESET ALL? This causes the tab-completion of even ALTER SYSTEM SET to display all and that's strange. But the tab-completion of SET has already had the same problem. So I think that we can live with that. Right and I have checked that behaviour is same for other similar statements like Alter Database database_name SET config_var or Alter User user_name SET config_var. So, the change made by you is on similar lines. OK. Applied. Uhm, are we agreed on the decision on not to backpatch this? I would think this should have been part of the initial ALTER SYSTEM SET patch and thus should be backpatched to 9.4. I think it belongs in 9.4 as well, especially if we're having another beta. -- Vik -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [BUGS] BUG #10823: Better REINDEX syntax.
On 07/30/2014 07:46 PM, Tom Lane wrote:
Bruce Momjian br...@momjian.us writes:
On Wed, Jul 30, 2014 at 01:29:31PM -0400, Tom Lane wrote:
I don't find it all that odd. We should not be encouraging routine
database-wide reindexes.
Uh, do we encourage database-wide VACUUM FULL or CLUSTER, as we use them
there with no parameter. Is there a reason REINDEX should be harder,
and require a dummy argument to run?
I believe that REINDEX on system catalogs carries a risk of deadlock
failures against other processes --- there was a recent example of that
in the mailing lists. VACUUM FULL has such risks too, but that's been
pretty well deprecated for many years. (I think CLUSTER is probably
relatively safe on this score because it's not going to think any system
catalogs are clustered.)
If there were a variant of REINDEX that only hit user tables, I'd be fine
with making that easy to invoke.
Here are two patches for this.
The first one, reindex_user_tables.v1.patch, implements the variant that
only hits user tables, as suggested by you.
The second one, reindex_no_dbname.v1.patch, allows the three
database-wide variants to omit the database name (voted for by Daniel
Migowski, Bruce, and myself; voted against by you). This patch is to be
applied on top of the first one.
--
Vik
*** a/doc/src/sgml/ref/reindex.sgml
--- b/doc/src/sgml/ref/reindex.sgml
***
*** 21,27 PostgreSQL documentation
refsynopsisdiv
synopsis
! REINDEX { INDEX | TABLE | DATABASE | SYSTEM } replaceable class=PARAMETERname/replaceable [ FORCE ]
/synopsis
/refsynopsisdiv
--- 21,27
refsynopsisdiv
synopsis
! REINDEX { INDEX | TABLE | DATABASE | SYSTEM | USER TABLES } replaceable class=PARAMETERname/replaceable [ FORCE ]
/synopsis
/refsynopsisdiv
***
*** 126,139 REINDEX { INDEX | TABLE | DATABASE | SYSTEM } replaceable class=PARAMETERnam
/varlistentry
varlistentry
termreplaceable class=PARAMETERname/replaceable/term
listitem
para
The name of the specific index, table, or database to be
reindexed. Index and table names can be schema-qualified.
! Presently, commandREINDEX DATABASE/ and commandREINDEX SYSTEM/
! can only reindex the current database, so their parameter must match
! the current database's name.
/para
/listitem
/varlistentry
--- 126,151
/varlistentry
varlistentry
+ termliteralUSER TABLES/literal/term
+ listitem
+ para
+ Recreate all indexes on user tables within the current database.
+ Indexes on system catalogs are not processed.
+ This form of commandREINDEX/command cannot be executed inside a
+ transaction block.
+ /para
+ /listitem
+/varlistentry
+
+varlistentry
termreplaceable class=PARAMETERname/replaceable/term
listitem
para
The name of the specific index, table, or database to be
reindexed. Index and table names can be schema-qualified.
! Presently, commandREINDEX DATABASE/, commandREINDEX SYSTEM/,
! and commandREINDEX USER TABLES/ can only reindex the current
! database, so their parameter must match the current database's name.
/para
/listitem
/varlistentry
*** a/doc/src/sgml/ref/reindexdb.sgml
--- b/doc/src/sgml/ref/reindexdb.sgml
***
*** 65,70 PostgreSQL documentation
--- 65,80
/group
arg choice=optreplaceabledbname/replaceable/arg
/cmdsynopsis
+
+ cmdsynopsis
+commandreindexdb/command
+arg rep=repeatreplaceableconnection-option/replaceable/arg
+group choice=plain
+ arg choice=plainoption--usertables/option/arg
+ arg choice=plainoption-u/option/arg
+/group
+arg choice=optreplaceabledbname/replaceable/arg
+ /cmdsynopsis
/refsynopsisdiv
***
*** 173,178 PostgreSQL documentation
--- 183,198
/listitem
/varlistentry
+ varlistentry
+ termoption-u//term
+ termoption--usertables//term
+ listitem
+para
+ Reindex database's user tables.
+/para
+ /listitem
+ /varlistentry
+
varlistentry
termoption-V//term
termoption--version//term
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***
*** 6984,6989 ReindexStmt:
--- 6984,6999
n-do_user = true;
$$ = (Node *)n;
}
+ | REINDEX USER TABLES name opt_force
+ {
+ ReindexStmt *n = makeNode(ReindexStmt);
+ n-kind = OBJECT_DATABASE;
+ n-name = $4;
+ n-relation = NULL;
+ n-do_system = false;
+ n-do_user = true;
+ $$ = (Node *)n;
+ }
;
reindex_type:
*** a/src/bin/psql/tab-complete.c
--- b/src/bin/psql/tab-complete.c
***
*** 3145,3151 psql_completion(const char *text, int start, int end)
else if (pg_strcasecmp(prev_wd, REINDEX) == 0)
{
static const char
Re: [HACKERS] Missing autocomplete for CREATE DATABASE
On 07/10/2014 09:32 PM, Magnus Hagander wrote: It seems psql is missing autocomplete entries for LC_COLLATE and LC_CTYPE for the CREATE DATABASE command. Attached patch adds that. I doubt this is important enough to backpatch - thoughts? It won't apply to current head, but otherwise I see no problem with it. I have no opinion on backpatching it. -- Vik -- 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] Cluster name in ps output
On 07/04/2014 08:50 AM, Fujii Masao wrote: On Wed, Jul 2, 2014 at 3:45 AM, Vik Fearing vik.fear...@dalibo.com wrote: Is there a reason for not using this in synchronous_standby_names, perhaps falling back to application_name if not set? You mean that if synchronous_standby_names is an empty it automatically should be set to cluster_name? Or, you mean that if application_name is not set in primary_conninfo the standby should automatically use its cluster_name as application_name in primary_conninfo? I'm afraid that those may cause the trouble such as that standby is unexpectedly treated as synchronous one even though a user want to set up it as asynchronous one by emptying synchronous_standby_names in the master. No, I mean that synchronous_standby_names should look at cluster_name first, and if it's not set then unfortunately look at application_name for backward compatibility. Using application_name for this always seems like a hack to me, and cluster_name is a much better fit. We should have created cluster_name back when we created synchronous_standby_names. -- Vik -- 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] DISTINCT with btree skip scan
On 07/05/2014 02:17 AM, Thomas Munro wrote: As an exercise I hacked up the simplest code I could think of that would demonstrate a faster DISTINCT based on skipping ahead to the next distinct value in an index-only scan. Please see the attached (extremely buggy) patch, and the example session below. (It's against my natural instinct to send such half-baked early hacking phase code to the list, but thought it would make sense to demo the concept and then seek advice, warnings, cease and desist notices etc before pressing on down that route!) I would be most grateful for any feedback you might have. This is called a Loose Index Scan in our wiki[1] which I believe is based on the terminology used for the same feature in MySQL although I haven't and shan't check. This is a feature I would really like to have. Your benchmarks look promising but unfortunately it blows up for me so I can't really test it. I have not yet read the patch nor debugged to see why, but please do keep up work on this. People more expert than I can tell you whether you're implementing it the right way. [1] http://wiki.postgresql.org/wiki/Loose_indexscan -- Vik -- 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] Aggregate function API versus grouping sets
On 07/02/2014 10:15 PM, Andrew Gierth wrote: (Had one request so far for a mode() variant that returns the unique modal value if one exists, otherwise null; so the current set of ordered-set aggs by no means exhausts the possible applications.) I resemble that remark. :) But seriously, am I the only one who doesn't want some random value when there is no dominant one? And the fact that I can't create my own without C code is a real bummer. -- Vik -- 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] Cluster name in ps output
On 06/29/2014 02:25 PM, Andres Freund wrote: On 2014-06-29 11:11:14 +0100, Thomas Munro wrote: On 29 June 2014 10:55, Andres Freund and...@2ndquadrant.com wrote: So, I'd looked at it with an eye towards committing it and found some more things. I've now * added the restriction that the cluster name can only be ASCII. It's shown from several clusters with differing encodings, and it's shown in process titles, so that seems wise. * moved everything to the LOGGING_WHAT category * added minimal documention to monitoring.sgml * expanded the config.sgml entry to mention the restriction on the name. * Changed the GUCs short description Thank you. I also think, but haven't done so, we should add a double colon after the cluster name, so it's not: postgres: server1 stats collector process but postgres: server1: stats collector process +1 Committed with the above changes. Thanks for the contribution! Is there a reason for not using this in synchronous_standby_names, perhaps falling back to application_name if not set? -- Vik -- 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] ALTER SYSTEM RESET?
On 06/27/2014 06:22 AM, Amit Kapila wrote:
On Thu, Jun 26, 2014 at 8:17 PM, Vik Fearing vik.fear...@dalibo.com
mailto:vik.fear...@dalibo.com wrote:
On 06/26/2014 05:07 AM, Amit Kapila wrote:
I think it will make sense if we support RESET ALL as well similar
to Alter Database .. RESET ALL syntax. Do you see any reason
why we shouldn't support RESET ALL syntax for Alter System?
Yes, that makes sense. I've added that in the attached version 2 of the
patch.
I think the idea used in patch to implement RESET ALL is sane. In passing
by, I noticed that modified lines in .sgml have crossed 80 char
boundary which
is generally preferred to be maintained..
Refer below modification.
!values to the filenamepostgresql.auto.conf/filename file.
Setting the parameter to
!literalDEFAULT/literal, or using the commandRESET/command
variant, removes the configuration entry from
I did that on purpose so that it's easy for a reviewer/committer to see
what changed.
This third patch reformats the documentation in the way I expected it to
be committed.
--
Vik
*** a/doc/src/sgml/ref/alter_system.sgml
--- b/doc/src/sgml/ref/alter_system.sgml
***
*** 22,27 PostgreSQL documentation
--- 22,30
refsynopsisdiv
synopsis
ALTER SYSTEM SET replaceable class=PARAMETERconfiguration_parameter/replaceable { TO | = } { replaceable class=PARAMETERvalue/replaceable | 'replaceable class=PARAMETERvalue/replaceable' | DEFAULT }
+
+ ALTER SYSTEM RESET replaceable class=PARAMETERconfiguration_parameter/replaceable
+ ALTER SYSTEM RESET ALL
/synopsis
/refsynopsisdiv
***
*** 30,37 ALTER SYSTEM SET replaceable class=PARAMETERconfiguration_parameter/replace
para
commandALTER SYSTEM/command writes the configuration parameter
!values to the filenamepostgresql.auto.conf/filename file. With
!literalDEFAULT/literal, it removes a configuration entry from
filenamepostgresql.auto.conf/filename file. The values will be
effective after reload of server configuration (SIGHUP) or in next
server start based on the type of configuration parameter modified.
--- 33,41
para
commandALTER SYSTEM/command writes the configuration parameter
!values to the filenamepostgresql.auto.conf/filename file.
!Setting the parameter to literalDEFAULT/literal, or using the
!commandRESET/command variant, removes the configuration entry from
filenamepostgresql.auto.conf/filename file. The values will be
effective after reload of server configuration (SIGHUP) or in next
server start based on the type of configuration parameter modified.
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***
*** 401,407 static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type istmt insert_rest
! %type vsetstmt generic_set set_rest set_rest_more SetResetClause FunctionSetResetClause
%type node TableElement TypedTableElement ConstraintElem TableFuncElement
%type node columnDef columnOptions
--- 401,407
%type istmt insert_rest
! %type vsetstmt generic_set set_rest set_rest_more generic_reset reset_rest SetResetClause FunctionSetResetClause
%type node TableElement TypedTableElement ConstraintElem TableFuncElement
%type node columnDef columnOptions
***
*** 1567,1605 NonReservedWord_or_Sconst:
;
VariableResetStmt:
! RESET var_name
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n-kind = VAR_RESET;
! n-name = $2;
! $$ = (Node *) n;
}
! | RESET TIME ZONE
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n-kind = VAR_RESET;
! n-name = timezone;
! $$ = (Node *) n;
}
! | RESET TRANSACTION ISOLATION LEVEL
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n-kind = VAR_RESET;
! n-name = transaction_isolation;
! $$ = (Node *) n;
}
! | RESET SESSION AUTHORIZATION
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n-kind = VAR_RESET;
! n-name = session_authorization;
! $$ = (Node *) n;
}
! | RESET ALL
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n-kind = VAR_RESET_ALL;
! $$ = (Node *) n;
}
;
--- 1567,1613
;
VariableResetStmt:
! RESET reset_rest { $$ = (Node *) $2; }
! ;
!
! reset_rest:
! generic_reset { $$ = $1; }
! | TIME ZONE
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n-kind = VAR_RESET;
! n-name = timezone;
! $$ = n;
}
! | TRANSACTION ISOLATION LEVEL
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n-kind = VAR_RESET;
! n-name = transaction_isolation;
! $$ = n;
}
! | SESSION AUTHORIZATION
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n-kind = VAR_RESET;
! n-name = session_authorization
Re: [HACKERS] ALTER SYSTEM RESET?
On 06/27/2014 08:49 AM, Vik Fearing wrote: This third patch reformats the documentation in the way I expected it to be committed. Amit, I added this to the next commitfest with your name as reviewer. https://commitfest.postgresql.org/action/patch_view?id=1495 Please update the status as you see fit. -- Vik -- 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] change alter user to be a true alias for alter role
On 06/19/2014 07:18 PM, Tom Lane wrote: Jov am...@amutu.com writes: the doc say: ALTER USER is now an alias for ALTER ROLEhttp://www.postgresql.org/docs/devel/static/sql-alterrole.html but alter user lack the following format: ... If we're going to have a policy that these commands be exactly equivalent, it seems like this patch is just sticking a finger into the dike. It does nothing to prevent anyone from making the same mistake again in future. What about collapsing both sets of productions into one, along the lines of role_or_user: ROLE | USER; AlterRoleSetStmt: ALTER role_or_user RoleId opt_in_database SetResetClause (and similarly to the latter for every existing ALTER ROLE variant). Because MAPPING is an unreserved keyword, I think that this approach might force us to also change ALTER USER MAPPING to ALTER role_or_user MAPPING, which is not contemplated by the SQL standard. But hey, it would satisfy the principle of least surprise no? Anyway we don't have to document that that would work. After a week of silence from Jov, I decided to do this myself since it didn't seem very hard. Many frustrating hours of trying to understand why I'm getting shift/reduce conflicts by the hundreds later, I've decided to give up for now. -- Vik -- 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] idle_in_transaction_timeout
On 06/26/2014 06:45 AM, Fujii Masao wrote: The point of this feature, AFAICS, is to detect clients that are failing to issue another query or close their transaction as a result of bad client logic. It's not to protect against network glitches. If so, the document should explain that cleanly. Otherwise users may misunderstand this parameter and try to use it to protect even long transaction generated by network glitches or client freeze. What does pg_stat_activity say for those cases? If it's able to say idle in transaction, then this patch covers it. If it isn't, then that seems like a different patch. -- Vik -- 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] ALTER SYSTEM RESET?
On 06/26/2014 05:07 AM, Amit Kapila wrote:
On Wed, Jun 25, 2014 at 9:56 PM, Vik Fearing vik.fear...@dalibo.com
mailto:vik.fear...@dalibo.com wrote:
On 06/25/2014 03:04 PM, Amit Kapila wrote:
Currently you can achieve that by
ALTER SYSTEM RESET guc = Default;.
However it will be good to have support for RESET as well. I think it
should not be too complicated to implement that syntax, I personally
don't have bandwidth to it immediately, but I would like to take care
of it unless you or someone wants to do it by the time I get some
bandwidth.
Would something like this suffice?
I think it will make sense if we support RESET ALL as well similar
to Alter Database .. RESET ALL syntax. Do you see any reason
why we shouldn't support RESET ALL syntax for Alter System?
Yes, that makes sense. I've added that in the attached version 2 of the
patch.
About patch:
+ | ALTER SYSTEM_P RESET var_name
+ {
+ AlterSystemStmt *n = makeNode(AlterSystemStmt);
+ n-setstmt = makeNode(VariableSetStmt);
+ n-setstmt-kind = VAR_RESET;
+ n-setstmt-name = $4;
+ $ = (Node *)n;
+ }
I think it would be better to have ALTER SYSTEM_P as generic and
then SET | RESET as different versions, something like below:
| SET reloptions
{
AlterTableCmd *n = makeNode(AlterTableCmd);
n-subtype = AT_SetRelOptions;
n-def = (Node *)$2;
$$ = (Node *)n;
}
/* ALTER TABLE name RESET (...) */
| RESET reloptions
{
AlterTableCmd *n = makeNode(AlterTableCmd);
n-subtype = AT_ResetRelOptions;
n-def = (Node *)$2;
$$ = (Node *)n;
}
Another point is that if we decide to support RESET ALL syntax, then
we might want reuse VariableResetStmt, may be by breaking into
generic and specific like we have done for generic_set.
I didn't quite follow your ALTER TABLE example because I don't think
it's necessary, but I did split out VariableResetStmt like you suggested
because I think that is indeed cleaner.
Thanks for working on patch.
You're welcome.
--
Vik
*** a/doc/src/sgml/ref/alter_system.sgml
--- b/doc/src/sgml/ref/alter_system.sgml
***
*** 22,27 PostgreSQL documentation
--- 22,30
refsynopsisdiv
synopsis
ALTER SYSTEM SET replaceable class=PARAMETERconfiguration_parameter/replaceable { TO | = } { replaceable class=PARAMETERvalue/replaceable | 'replaceable class=PARAMETERvalue/replaceable' | DEFAULT }
+
+ ALTER SYSTEM RESET replaceable class=PARAMETERconfiguration_parameter/replaceable
+ ALTER SYSTEM RESET ALL
/synopsis
/refsynopsisdiv
***
*** 30,37 ALTER SYSTEM SET replaceable class=PARAMETERconfiguration_parameter/replace
para
commandALTER SYSTEM/command writes the configuration parameter
!values to the filenamepostgresql.auto.conf/filename file. With
!literalDEFAULT/literal, it removes a configuration entry from
filenamepostgresql.auto.conf/filename file. The values will be
effective after reload of server configuration (SIGHUP) or in next
server start based on the type of configuration parameter modified.
--- 33,40
para
commandALTER SYSTEM/command writes the configuration parameter
!values to the filenamepostgresql.auto.conf/filename file. Setting the parameter to
!literalDEFAULT/literal, or using the commandRESET/command variant, removes the configuration entry from
filenamepostgresql.auto.conf/filename file. The values will be
effective after reload of server configuration (SIGHUP) or in next
server start based on the type of configuration parameter modified.
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***
*** 401,407 static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query);
%type istmt insert_rest
! %type vsetstmt generic_set set_rest set_rest_more SetResetClause FunctionSetResetClause
%type node TableElement TypedTableElement ConstraintElem TableFuncElement
%type node columnDef columnOptions
--- 401,407
%type istmt insert_rest
! %type vsetstmt generic_set set_rest set_rest_more generic_reset reset_rest SetResetClause FunctionSetResetClause
%type node TableElement TypedTableElement ConstraintElem TableFuncElement
%type node columnDef columnOptions
***
*** 1567,1605 NonReservedWord_or_Sconst:
;
VariableResetStmt:
! RESET var_name
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n-kind = VAR_RESET;
! n-name = $2;
! $$ = (Node *) n;
}
! | RESET TIME ZONE
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n-kind = VAR_RESET;
! n-name = timezone;
! $$ = (Node *) n;
}
! | RESET TRANSACTION ISOLATION LEVEL
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n-kind = VAR_RESET;
! n-name = transaction_isolation;
! $$ = (Node *) n;
}
! | RESET SESSION AUTHORIZATION
{
VariableSetStmt *n = makeNode(VariableSetStmt);
n-kind
Re: [HACKERS] SQL access to database attributes
On 06/23/2014 06:45 PM, Pavel Stehule wrote:
2014-06-23 18:39 GMT+02:00 Vik Fearing vik.fear...@dalibo.com
mailto:vik.fear...@dalibo.com:
On 06/23/2014 06:21 PM, Robert Haas wrote:
On Sun, Jun 22, 2014 at 2:59 PM, Pavel Stehule
pavel.steh...@gmail.com mailto:pavel.steh...@gmail.com wrote:
I found only one problem - first patch introduce a new property
CONNECTION_LIMIT and replace previously used CONNECTION LIMIT in
documentation. But CONNECTION LIMIT is still supported, but it
is not
documented. So for some readers it can look like breaking
compatibility, but
it is false. This should be documented better.
Yeah, I think the old syntax should be documented also.
Why do we want to document syntax that should eventually be deprecated?
It is fair to our users. It can be deprecated, ok, we can write in doc -
this feature will be deprecated in next three years. Don't use it, but
this should be documentated.
Okay, here is version two of the refactoring patch that documents that
the with-space version is deprecated but still accepted.
The feature patch is not affected by this and so I am not attaching a
new version of that.
--
Vik
*** a/contrib/sepgsql/expected/alter.out
--- b/contrib/sepgsql/expected/alter.out
***
*** 110,116 SET search_path = regtest_schema, regtest_schema_2, public;
--
-- misc ALTER commands
--
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION LIMIT 999;
LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:unconfined_t:s0 tcontext=unconfined_u:object_r:sepgsql_db_t:s0 tclass=db_database name=regtest_sepgsql_test_database
ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
ALTER TABLE regtest_table ADD COLUMN d float;
--- 110,116
--
-- misc ALTER commands
--
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION_LIMIT 999;
LOG: SELinux: allowed { setattr } scontext=unconfined_u:unconfined_r:unconfined_t:s0 tcontext=unconfined_u:object_r:sepgsql_db_t:s0 tclass=db_database name=regtest_sepgsql_test_database
ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
ALTER TABLE regtest_table ADD COLUMN d float;
*** a/contrib/sepgsql/sql/alter.sql
--- b/contrib/sepgsql/sql/alter.sql
***
*** 85,91 SET search_path = regtest_schema, regtest_schema_2, public;
--
-- misc ALTER commands
--
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION LIMIT 999;
ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
ALTER TABLE regtest_table ADD COLUMN d float;
--- 85,91
--
-- misc ALTER commands
--
! ALTER DATABASE regtest_sepgsql_test_database CONNECTION_LIMIT 999;
ALTER DATABASE regtest_sepgsql_test_database SET search_path TO regtest_schema, public; -- not supported yet
ALTER TABLE regtest_table ADD COLUMN d float;
*** a/doc/src/sgml/ref/alter_database.sgml
--- b/doc/src/sgml/ref/alter_database.sgml
***
*** 25,31 ALTER DATABASE replaceable class=PARAMETERname/replaceable [ [ WITH ] rep
phrasewhere replaceable class=PARAMETERoption/replaceable can be:/phrase
! CONNECTION LIMIT replaceable class=PARAMETERconnlimit/replaceable
ALTER DATABASE replaceable class=PARAMETERname/replaceable RENAME TO replaceablenew_name/replaceable
--- 25,31
phrasewhere replaceable class=PARAMETERoption/replaceable can be:/phrase
! CONNECTION_LIMIT replaceable class=PARAMETERconnection_limit/replaceable
ALTER DATABASE replaceable class=PARAMETERname/replaceable RENAME TO replaceablenew_name/replaceable
***
*** 107,117 ALTER DATABASE replaceable class=PARAMETERname/replaceable RESET ALL
/varlistentry
varlistentry
! termreplaceable class=parameterconnlimit/replaceable/term
listitem
para
How many concurrent connections can be made
to this database. -1 means no limit.
/para
/listitem
/varlistentry
--- 107,119
/varlistentry
varlistentry
! termreplaceable class=parameterconnection_limit/replaceable/term
listitem
para
How many concurrent connections can be made
to this database. -1 means no limit.
+ The deprecated spelling commandCONNECTION LIMIT/command is
+ still accepted.
/para
/listitem
/varlistentry
*** a/doc/src/sgml/ref/create_database.sgml
--- b/doc/src/sgml/ref/create_database.sgml
***
*** 28,34 CREATE DATABASE replaceable class=PARAMETERname/replaceable
[ LC_COLLATE [=] replaceable class=parameterlc_collate/replaceable ]
[ LC_CTYPE [=] replaceable class=parameterlc_ctype/replaceable ]
[ TABLESPACE [=] replaceable class
Re: [HACKERS] Window function optimisation, allow pushdowns of items matching PARTITION BY clauses
On 06/21/2014 02:03 PM, David Rowley wrote: I'm marking this Waiting on Author pending discussion on pushing down entire expressions, but on the whole I think this is pretty much ready. As I said above, I don't think playing around with that code is really work for this patch. It can be done later in another patch if people think it would be useful. I tend to agree. This latest patch is ready for a committer to look at now. The weird comments have been changed, superfluous regression tests removed, and nothing done about expression pushdown per (brief) discussion. -- Vik -- 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] Window function optimisation, allow pushdowns of items matching PARTITION BY clauses
On 06/27/2014 02:49 AM, Tom Lane wrote: Vik Fearing vik.fear...@dalibo.com writes: This latest patch is ready for a committer to look at now. The weird comments have been changed, superfluous regression tests removed, and nothing done about expression pushdown per (brief) discussion. I started to look at this patch and realized that there's an issue that isn't covered, which is not too surprising because the existing code fails to cover it too. Remember that the argument for pushing down being safe at all is that we expect the pushed-down qual to yield the same result at all rows of a given partition, so that we either include or exclude the whole partition and thereby don't change window function results. This means that not only must the qual expression depend only on partitioning columns, but *it had better not be volatile*. In exactly the same way, it isn't safe to push down quals into subqueries that use DISTINCT unless the quals are non-volatile. This consideration is missed by the current code, and I think that's a bug. (Pushing down volatile quals would also be unsafe in subqueries involving aggregation, except that we put them into HAVING so that they're executed only once per subquery output row anyway.) Are you going to take care of all this, or should David or I take a crack at it? The commitfest app still shows Ready for Committer. Given the lack of prior complaints, I'm not excited about back-patching a change to prevent pushing down volatile quals in the presence of DISTINCT; but I think we probably ought to fix it in 9.5, and maybe 9.4 too. Thoughts? I didn't test it myself, I'm just taking your word on it. If it's a bug, it should obviously be fixed in 9.5. As for 9.4, I have always viewed a beta as a time to fix bugs so I vote to backpatch it at least that far. Why wouldn't it go back all the way to 9.0? (assuming 8.4 is dead) -- Vik -- 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] ALTER SYSTEM RESET?
On 06/25/2014 03:04 PM, Amit Kapila wrote:
On Wed, Jun 25, 2014 at 6:20 PM, Christoph Berg c...@df7cb.de
mailto:c...@df7cb.de wrote:
Hi,
is there a reason there's no ALTER SYSTEM RESET?
The natural idiom to reset SET statements is RESET guc;, I don't
think SET guc = default; is in use much, so ALTER SYSTEM RESET guc;
would be the natural way to try.
Currently you can achieve that by
ALTER SYSTEM RESET guc = Default;.
However it will be good to have support for RESET as well. I think it
should not be too complicated to implement that syntax, I personally
don't have bandwidth to it immediately, but I would like to take care
of it unless you or someone wants to do it by the time I get some
bandwidth.
Would something like this suffice?
--
Vik
*** a/doc/src/sgml/ref/alter_system.sgml
--- b/doc/src/sgml/ref/alter_system.sgml
***
*** 22,27 PostgreSQL documentation
--- 22,28
refsynopsisdiv
synopsis
ALTER SYSTEM SET replaceable class=PARAMETERconfiguration_parameter/replaceable { TO | = } { replaceable class=PARAMETERvalue/replaceable | 'replaceable class=PARAMETERvalue/replaceable' | DEFAULT }
+ ALTER SYSTEM RESET replaceable class=PARAMETERconfiguration_parameter/replaceable
/synopsis
/refsynopsisdiv
***
*** 30,37 ALTER SYSTEM SET replaceable class=PARAMETERconfiguration_parameter/replace
para
commandALTER SYSTEM/command writes the configuration parameter
!values to the filenamepostgresql.auto.conf/filename file. With
!literalDEFAULT/literal, it removes a configuration entry from
filenamepostgresql.auto.conf/filename file. The values will be
effective after reload of server configuration (SIGHUP) or in next
server start based on the type of configuration parameter modified.
--- 31,38
para
commandALTER SYSTEM/command writes the configuration parameter
!values to the filenamepostgresql.auto.conf/filename file. Setting the parameter to
!literalDEFAULT/literal, or using the commandRESET/command variant, removes the configuration entry from
filenamepostgresql.auto.conf/filename file. The values will be
effective after reload of server configuration (SIGHUP) or in next
server start based on the type of configuration parameter modified.
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
***
*** 8486,8491 AlterSystemStmt:
--- 8486,8499
n-setstmt = $4;
$$ = (Node *)n;
}
+ | ALTER SYSTEM_P RESET var_name
+ {
+ AlterSystemStmt *n = makeNode(AlterSystemStmt);
+ n-setstmt = makeNode(VariableSetStmt);
+ n-setstmt-kind = VAR_RESET;
+ n-setstmt-name = $4;
+ $$ = (Node *)n;
+ }
;
*** a/src/backend/utils/misc/guc.c
--- b/src/backend/utils/misc/guc.c
***
*** 6720,6725 AlterSystemSetConfigFile(AlterSystemStmt *altersysstmt)
--- 6720,6726
break;
case VAR_SET_DEFAULT:
+ case VAR_RESET:
value = NULL;
break;
default:
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
