Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates
On 03/07/2015 07:18 PM, Petr Jelinek wrote: What I am wondering is if those numeric_int16_* functions that also deal with either the Int128AggState or NumericAggState should be renamed in similar fashion. You mean something like numeric_poly_sum instead of numeric_int16_sum? I personally am not fond of either name. While numeric_int16_* incorrectly implies we have a int16 SQL type numeric_poly_* does not tell us that this is an optimized version which uses a smaller state. The worst part of writing this patch has always been naming functions and types. :) Andreas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_trgm Memory Allocation logic
Beena Emerson wrote: In the pg_trgm module, within function generate_trgm, the memory for trigrams is allocated as follows: trg = (TRGM *) palloc(TRGMHDRSIZE + sizeof(trgm) * (slen / 2 + 1) *3); I have been trying to understand why this is so because it seems to be allocating more space than that is required. Maybe it's considering a worst-case for multibyte characteres? I don't really know if trgm supports multibyte, but I assume it does. If it does, then probably the trigrams consist of chars, not bytes. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_trgm Memory Allocation logic
On 03/09/2015 02:54 PM, Alvaro Herrera wrote: Beena Emerson wrote: In the pg_trgm module, within function generate_trgm, the memory for trigrams is allocated as follows: trg = (TRGM *) palloc(TRGMHDRSIZE + sizeof(trgm) * (slen / 2 + 1) *3); I have been trying to understand why this is so because it seems to be allocating more space than that is required. Maybe it's considering a worst-case for multibyte characteres? I don't really know if trgm supports multibyte, but I assume it does. If it does, then probably the trigrams consist of chars, not bytes. Nope. Trigrams are always three bytes, even ones containing multibyte characters. If there are any multibyte characters in the trigram, we store a 3-byte checksum of the three characters instead. That loses some information, you can have a collision where one multibyte trigram incorrectly matches another one, but the trigram algorithms are generally not too concerned about exact results. - Heikki -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [REVIEW] Re: Compression of full-page-writes
On Mon, Mar 9, 2015 at 4:29 PM, Fujii Masao masao.fu...@gmail.com wrote:
On Thu, Mar 5, 2015 at 10:08 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
On Thu, Mar 5, 2015 at 9:14 PM, Syed, Rahila rahila.s...@nttdata.com wrote:
Please find attached a patch. As discussed, flag to denote compression and
presence of hole in block image has been added in XLogRecordImageHeader
rather than block header.
Thanks for updating the patch! Attached is the refactored version of the
patch.
Cool. Thanks!
I have some minor comments:
+The default value is literaloff/
Dot at the end of this sentence.
+Turning this parameter on can reduce the WAL volume without
Turning valueon/ this parameter
+but at the cost of some extra CPU time by the compression during
+WAL logging and the decompression during WAL replay.
Isn't a verb missing here, for something like that:
but at the cost of some extra CPU spent on the compression during WAL
logging and on the decompression during WAL replay.
+ * This can reduce the WAL volume, but at some extra cost of CPU time
+ * by the compression during WAL logging.
Er, similarly some extra cost of CPU spent on the compression
+ if (blk-bimg_info BKPIMAGE_HAS_HOLE
+ (blk-hole_offset == 0 ||
+blk-hole_length == 0 ||
I think that extra parenthesis should be used for the first expression
with BKPIMAGE_HAS_HOLE.
+ if (blk-bimg_info BKPIMAGE_IS_COMPRESSED
+ blk-bimg_len == BLCKSZ)
+ {
Same here.
+ /*
+* cross-check that hole_offset == 0
and hole_length == 0
+* if the HAS_HOLE flag is set.
+*/
I think that you mean here that this happens when the flag is *not* set.
+ /*
+* If BKPIMAGE_HAS_HOLE and BKPIMAGE_IS_COMPRESSED,
+* an XLogRecordBlockCompressHeader follows
+*/
Maybe a struct should be added for an XLogRecordBlockCompressHeader
struct. And a dot at the end of the sentence should be added?
Regards,
--
Michael
--
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_trgm Memory Allocation logic
Beena Emerson memissemer...@gmail.com writes: In the pg_trgm module, within function generate_trgm, the memory for trigrams is allocated as follows: trg = (TRGM *) palloc(TRGMHDRSIZE + sizeof(trgm) * (slen / 2 + 1) *3); I have been trying to understand why this is so because it seems to be allocating more space than that is required. Consider input like 'X X X X X'. Each X produces 3 trigrams. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Object files generated by ecpg test suite not ignored on Windows
Alvaro Herrera alvhe...@2ndquadrant.com writes: Michael Paquier wrote: When running the test suite of ecpg, build generates a set of obj files and it happens that those files are not ignored in the code tree. Wouldn't this be simpler as a *.obj pattern somewhere? Maybe in ecpg/test/.gitignore add this line? Actually, if we are supporting toolchains that generate *.obj files, I'd expect the top-level .gitignore to ignore them, as it does *.o. But if that's the issue why have we not heard complaints before? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] MD5 authentication needs help -SCRAM
Hi Abhijit, I didn't realize you were involved in the IETF process on SCRAM :-). On 03/09/2015 09:21 AM, Abhijit Menon-Sen wrote: At 2015-03-08 12:48:44 -0700, j...@agliodbs.com wrote: Since SCRAM has been brought up a number of times here, I thought I'd loop in the PostgreSQL contributor who is co-author of the SCRAM standard to see if he has anything to say about implementing SCRAM as a built-in auth method for Postgres. I think it's a good idea. Having done some googling, SCRAM seems like a good choice to me too. Another one is SRP. The important difference between SRP and SCRAM is that in SRP, an eavesdropper cannot capture information needed to brute-force the password. The class of protocols that have that property are called Password-authenticated key agreement protocols (PAKE) [1]. SRP seems to be the most common one of those, although there are others. On the face of it, it seems like PAKE protocols are superior. There is an IETF draft for SRP as a SASL authentication mechanism [2], and even some implementations of that (e.g. Cyrus-SASL), but for some reason that draft never became a standard and expired. Do you have any insight on why the IETF working group didn't choose a PAKE protocol instead of or in addition to SCRAM, when SCRAM was standardized? [1] https://en.wikipedia.org/wiki/Password-authenticated_key_agreement [2] https://tools.ietf.org/html/draft-burdis-cat-srp-sasl-08 - Heikki -- 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] Object files generated by ecpg test suite not ignored on Windows
When running the test suite of ecpg, build generates a set of obj files and it happens that those files are not ignored in the code tree. Applied, thanks. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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_rewind in contrib
On 01/19/2015 07:38 AM, Michael Paquier wrote: Looking at the set of TAP tests, I think that those lines open again the door of CVE-2014-0067 (vulnerability with make check) on Windows: # Initialize master, data checksums are mandatory remove_tree($test_master_datadir); system_or_bail(initdb -N -A trust -D $test_master_datadir $log_path); IMO we should use standard_initdb in TestLib.pm instead as pg_regress --config-auth would be used for SSPI. standard_initdb should be extended a bit as well to be able to pass a path to logs with /dev/null as default. TAP tests do not run on Windows, still I think that it would be better to cover any eventuality in this area before we forget. Already mentioned by Peter, but I think as well that the new additions to TAP should be a separate patch. Agreed, fixed to use standard_initdb. . Random thought (not related to this patch), have a new option in initdb doing this legwork: + # Accept replication connections on master + append_to_file($test_master_datadir/pg_hba.conf, qq( +local replication all trust +host replication all 127.0.0.1/32 trust +host replication all ::1/128 trust +)); Yeah, that would be good. Perhaps as part of the pg_regress --config-auth. If it's an initdb, then it might make sense to have the same option to set wal_level=hot_standby, and max_wal_senders, so that the cluster is immediately ready for replication. But that's a different topic, I'm going to just leave it as it is in this pg_rewind patch. Attached is a new patch version, fixing all the little things you listed. I believe this is pretty much ready for commit. I'm going to read it through myself one more time before committing, but I don't have anything mind now that needs fixing anymore. I just pushed the change to split dbcommands.h into dbcommands.h and dbcommands_xlog.h, as that seems like a nice-to-have anyway. - Heikki pg_rewind-bin-7.patch.gz Description: application/gzip -- 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 functions
Dmitry Voronin wrote: divpreHello, /prepreI make an a patch, which adds 4 functions to sslinfo extension module:br /1) ssl_extension_names() --- get short names of X509v3 extensions from client certificate and it's values; 2) ssl_extension_value(text) --- get value of extension from certificate (argument --- short name of extension); 3) ssl_extension_is_critical(text) --- returns true, if extension is critical and false, if is not (argument --- short name of extension). You can view some information of certificate's extensions via those functions. Since you change default version from 1.0 to 1.1 you need to supply the sslinfo--1.1.sql script. You also need to supply a sslinfo--1.0--1.1.sql script which enables updates from version 1.0 to 1.1. (The test for the upgrade is to initdb 9.4, and in some database install the sslinfo extension; then pg_upgrade to a version with your patch. In the database with the extension then try to upgrade it to 1.1.) Please use C type bool not int when that's what the code means. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] pg_trgm Memory Allocation logic
In the pg_trgm module, within function generate_trgm, the memory for trigrams is allocated as follows: trg = (TRGM *) palloc(TRGMHDRSIZE + sizeof(trgm) * (slen / 2 + 1) *3); I have been trying to understand why this is so because it seems to be allocating more space than that is required. The following table shows the palloced size [(slen / 2 + 1) *3] and the actual trgm count for different string length. slenpalloc size actual trgm count 2 6 3 26 42 27 38 60 39 Can somebody please explain this to me. I had tried changing the allocation to slen + 1 and it seemed to be working without any problem. trg = (TRGM *) palloc(TRGMHDRSIZE + sizeof(trgm) * (slen + 1)); Maybe I am missisng some scenarios. Any help would be appreciated. Thank you, Beena Emerson - -- Beena Emerson -- View this message in context: http://postgresql.nabble.com/pg-trgm-Memory-Allocation-logic-tp5841088.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_trgm Memory Allocation logic
On 03/09/2015 03:33 PM, Tom Lane wrote:
Beena Emerson memissemer...@gmail.com writes:
In the pg_trgm module, within function generate_trgm, the memory for trigrams
is allocated as follows:
trg = (TRGM *) palloc(TRGMHDRSIZE + sizeof(trgm) * (slen / 2 + 1) *3);
I have been trying to understand why this is so because it seems to be
allocating more space than that is required.
Consider input like 'X X X X X'. Each X produces 3 trigrams.
No it won't. Only two:
postgres=# select show_trgm('a b c');
show_trgm
---
{ a, b, c, a , b , c }
(1 row)
If you manually set RPADDING 2 in trgm.h, then it will, but the
allocation probably should use LPADDING/RPADDING to get it right, rather
than assume the max values.
- Heikki
--
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] Object files generated by ecpg test suite not ignored on Windows
Michael Paquier wrote: Hi all, When running the test suite of ecpg, build generates a set of obj files and it happens that those files are not ignored in the code tree. Wouldn't this be simpler as a *.obj pattern somewhere? Maybe in ecpg/test/.gitignore add this line? **/*.obj -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Enforce creation of destination folders for source files in pg_regress (Was: pg_regress writes into source tree)
Michael Paquier wrote: On Sun, Mar 1, 2015 at 2:38 AM, Stephen Frost sfr...@snowman.net wrote: I'm going to mark this back to 'waiting on author' in case there's something material that I've missed which you can clarify. I had started this review thinking to help move it along but after re-reading the thread and thinking about it for a bit, I came around to the other side and therefore think it should probably be rejected. We certainly appreciate the effort, of course. Re-reading this thread there are 3 perplexed opinions from three different committers, and nobody pleading in favor of this patch except me, so let's simply mark this as rejected and move on. Well I also don't really like the existing behavior but it's not a big issue and so I'm OK with letting go. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] In-core regression tests for replication, cascading, archiving, PITR, etc.
On 3/8/15 6:19 AM, Michael Paquier wrote: On Mon, Dec 2, 2013 at 7:07 PM, Andres Freund and...@2ndquadrant.com wrote: On 2013-12-02 18:45:37 +0900, Michael Paquier wrote: On Mon, Dec 2, 2013 at 6:24 PM, Heikki Linnakangas hlinnakan...@vmware.com wrote: +1. The need for such a test suite has been mentioned every single time that a bug or new feature related to replication, PITR or hot standby has come up. So yes please! The only thing missing is someone to actually write the thing. So if you have the time and energy, that'd be great! I am sure you know who we need to convince in this case :) If you're alluding to Tom, I'd guess he doesn't need to be convinced of such a facility in general. I seem to remember him complaining about the lack of testing that as well. Maybe that it shouldn't be part of the main regression schedule... +many from me as well. I think the big battle will be how to do it, not if in general. (Reviving an old thread) So I am planning to seriously focus soon on this stuff, basically using the TAP tests as base infrastructure for this regression test suite. First, does using the TAP tests sound fine? On the top of my mind I got the following items that should be tested: - WAL replay: from archive, from stream - hot standby and read-only queries - node promotion - recovery targets and their interferences when multiple targets are specified (XID, name, timestamp, immediate) - timelines - recovery_target_action - recovery_min_apply_delay (check that WAL is fetch from a source at some correct interval, can use a special restore_command for that) - archive_cleanup_command (check that command is kicked at each restart point) - recovery_end_command (check that command is kicked at the end of recovery) - timeline jump of a standby after reconnecting to a promoted node If we're keeping a list, there's also hot_standby_feedback, max_standby_archive_delay and max_standby_streaming_delay. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] MD5 authentication needs help -SCRAM
At 2015-03-09 13:52:10 +0200, hlinn...@iki.fi wrote: Do you have any insight on why the IETF working group didn't choose a PAKE protocol instead of or in addition to SCRAM, when SCRAM was standardized? Hi Heikki. It was a long time ago, but I recall that SRP was patent-encumbered: https://datatracker.ietf.org/ipr/search/?rfc=2945submit=rfc The Wikipedia page says the relevant patents expired in 2011 and 2013. I haven't followed SRP development since then, maybe it's been revised. When SCRAM was being discussed, I can't recall any other proposals for PAKE protocols. Besides, as you may already know, anyone can submit an internet-draft about anything. It needs to gain general support for an extended period in order to advance through the standards process. Could you please explain what exactly you mean about a SCRAM eavesdropper gaining some advantage in being able to mount a dictionary attack? I didn't follow that part. -- Abhijit -- 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] One question about security label command
On Tue, Mar 3, 2015 at 5:01 AM, Kouhei Kaigai kai...@ak.jp.nec.com wrote: From standpoint of SQL syntax, yep, SECURITY LABEL command support the object types below, however, it fully depends on security label provider; sepgsql.so in this case. At this moment, it supports database, schema, function, tables and column are supported by sepgsql. So, it is expected behavior. If the core system supports labels on other object types and sepgsql does not, it should give a better error for those cases, like: ERROR: sepgsql provider does not support labels on roles Errors like ERROR: unsupported object type: 1260 are a good way to report a failure that is never expected to happen, but they shouldn't be used as user-facing error messages. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] New functions
You're right. I changed: - at sslinfo.contol return default module version to '1.0'; - function get_extension() returns now boolean (true, if we found extension, and false otherwise).09.03.2015, 16:43, "Alvaro Herrera" alvhe...@2ndquadrant.com: Dmitry Voronin wrote: divpreHello, /prepreI make an a patch, which adds 4 functions to sslinfo extension module:br /1) ssl_extension_names() --- get short names of X509v3 extensions from client certificate and it's values; 2) ssl_extension_value(text) --- get value of extension from certificate (argument --- short name of extension); 3) ssl_extension_is_critical(text) --- returns true, if extension is critical and false, if is not (argument --- short name of extension). You can view some information of certificate's extensions via those functions. Since you change default version from 1.0 to 1.1 you need to supply the sslinfo--1.1.sql script. You also need to supply a sslinfo--1.0--1.1.sql script which enables updates from version 1.0 to 1.1. (The test for the upgrade is to initdb 9.4, and in some database install the sslinfo extension; then pg_upgrade to a version with your patch. In the database with the extension then try to upgrade it to 1.1.) Please use C type bool not int when that's what the code means. -- Álvaro Herrera http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services-- Best regards, Dmitry Voronin*** a/contrib/sslinfo/sslinfo.c
--- b/contrib/sslinfo/sslinfo.c
***
*** 14,21
--- 14,23
#include miscadmin.h
#include utils/builtins.h
#include mb/pg_wchar.h
+ #include funcapi.h
#include openssl/x509.h
+ #include openssl/x509v3.h
#include openssl/asn1.h
PG_MODULE_MAGIC;
***
*** 23,28 PG_MODULE_MAGIC;
--- 25,31
static Datum X509_NAME_field_to_text(X509_NAME *name, text *fieldName);
static Datum X509_NAME_to_text(X509_NAME *name);
static Datum ASN1_STRING_to_text(ASN1_STRING *str);
+ bool get_extension(X509 *cert, const char *ext_name, X509_EXTENSION **extension);
/*
***
*** 354,356 ssl_issuer_dn(PG_FUNCTION_ARGS)
--- 357,581
PG_RETURN_NULL();
return X509_NAME_to_text(X509_get_issuer_name(MyProcPort-peer));
}
+
+
+ /*
+ * Returns extension object by given certificate and extension's name.
+ *
+ * Try to get extension from certificate by extension's name.
+ * We returns at extension param pointer to X509_EXTENSION.
+ *
+ * Returns true, if we have found extension in certificate and false, if we not.
+ */
+ bool get_extension(X509* cert, const char *ext_name, X509_EXTENSION **extension)
+ {
+ int nid = 0;
+ int loc = 0;
+
+ /* try to convert extension name to ObjectID */
+ nid = OBJ_txt2nid(ext_name);
+ /* Not success ? */
+ if (nid == NID_undef)
+ return false;
+
+ loc = X509_get_ext_by_NID(cert, nid, -1);
+
+ /* palloc memory for extension and copy it */
+ *extension = (X509_EXTENSION *) palloc(sizeof(X509_EXTENSION *));
+ memcpy(*extension, X509_get_ext(cert, loc), sizeof(X509_EXTENSION));
+
+ return true;
+ }
+
+
+ /* Returns value of extension
+ *
+ * This function returns value of extension by given name in client certificate.
+ *
+ * Returns text datum.
+ */
+ PG_FUNCTION_INFO_V1(ssl_extension_value);
+ Datum
+ ssl_extension_value(PG_FUNCTION_ARGS)
+ {
+ X509 *cert = MyProcPort-peer;
+ X509_EXTENSION *ext = NULL;
+ char *ext_name = text_to_cstring(PG_GETARG_TEXT_P(0));
+ BIO *membuf = NULL;
+ char *val = NULL;
+ char nullterm = '\0';
+ bool error = false;
+
+ /* If we have no ssl security */
+ if (cert == NULL)
+ PG_RETURN_NULL();
+
+ /* If extension's converting from text name to extension's OID failed (return NID_undef) */
+ if (OBJ_txt2nid(ext_name) == NID_undef)
+ ereport(ERROR,
+ (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+ errmsg(Unknown extension name \%s\, ext_name)));
+
+ /* Extension's name is correct, try to get extension object from certificate */
+ error = get_extension(cert, ext_name, ext);
+
+ /* Not found? */
+ if (!error)
+ PG_RETURN_NULL();
+
+ /* Print extension to BIO */
+ membuf = BIO_new(BIO_s_mem());
+ X509V3_EXT_print(membuf, ext, 0, 0);
+ BIO_write(membuf, nullterm, 1);
+ BIO_get_mem_data(membuf, val);
+
+ /* Copy value */
+ val = pstrdup(val);
+
+ /* Clear BIO */
+ BIO_free(membuf);
+
+ /* free extension */
+ if (ext)
+ pfree(ext);
+
+ PG_RETURN_TEXT_P(cstring_to_text(val));
+ }
+
+
+ /* Returns status of extension
+ *
+ * Returns true, if extension is critical and false, if it is not.
+ *
+ * Returns bool datum.
+ */
+ PG_FUNCTION_INFO_V1(ssl_extension_is_critical);
+ Datum
+ ssl_extension_is_critical(PG_FUNCTION_ARGS)
+ {
+ X509 *cert = MyProcPort-peer;
+ X509_EXTENSION *ext = NULL;
+ char *ext_name = text_to_cstring(PG_GETARG_TEXT_P(0));
+ int critical;
+ bool error = false;
+
+ /* If we have no ssl security */
+ if (cert ==
Re: [HACKERS] Rethinking the parameter access hooks for plpgsql's benefit
On Mon, Mar 9, 2015 at 12:26 PM, Joshua D. Drake j...@commandprompt.com wrote: From the reading the original post it seems like the patch was developed on Sales Force's time, not TGLs. I do not think we get to have an opinion on that. Salesforce gets to develop their patches whenever they want, but they don't get to control the community process for getting those patches committed to PostgreSQL. Thank you Tom for the efforts you made here, making the largest used PL better, faster, stronger is exactly the type of micro-changes we need to be looking at for further polish within the database. So, just to be clear, are you proposing that everybody is exempted from the CommitFest deadlines, or just Tom? Thanks, -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Reduce pinning in btree indexes
Kevin Grittner kgri...@ymail.com wrote: It doesn't seem worth posting to the list for the small changes since the last version; I'll wait until I update the comments and README files. If you want review or test the latest, you can peek at: https://github.com/kgrittn/postgres/tree/btnopin Here is v3, with the promised README and code comment changes. In the process of documenting the mark/restore area I noticed a subtlety that I had missed (in the case that there was a mark, advance to the next page, restore, advance within the page, and restore). I fixed that, and in the process gave the patched code an additional direct performance edge over unpatched code. For the 1000k marks, average timings are now: master: 970.999 ms, stdev: 4.043 patched: 940.460 ms, stdev: 4.874 So, in the micro-benchmark showing the biggest benefit the direct improvement is now just over 3%. It remains the case that the primary motivation for the patch is to reduce blocking of vacuum processes; but that's a nice side-benefit. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company bt-nopin-v3.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] Bootstrap DATA is a pita
On Sun, Mar 8, 2015 at 12:35 PM, Andres Freund and...@2ndquadrant.com wrote: On 2015-03-04 10:25:58 -0500, Robert Haas wrote: Another advantage of this is that it would probably make git less likely to fumble a rebase. If there are lots of places in the file where we have the same 10 lines in a row with occasional variations, rebasing a patch could easily pick the the wrong place to reapply the hunk. I would personally consider a substantial increase in the rate of such occurrences as being a cure far, far worse than the disease. If you keep the entry for each function on just a couple of lines the chances of this happening are greatly reduced, because you're much likely to get a false match to surrounding context. I'm not particularly worried about this. Especially with attribute defaults it seems unlikely that you often have the same three surrounding lines in both directions in a similar region of the file. That's woefully optimistic, and you don't need to have 3 lines. 1 or 2 will do fine. And even if it turns out to actually be bothersome, you can help yourself by passing -U 5/setting diff.context = 5 or something like that. I don't believe that for a minute. When you have your own private branch and you do 'git rebase master', how's that going to help? -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Object files generated by ecpg test suite not ignored on Windows
On 03/09/2015 11:31 AM, Michael Meskes wrote: Actually, if we are supporting toolchains that generate *.obj files, I'd expect the top-level .gitignore to ignore them, as it does *.o. But if that's the issue why have we not heard complaints before? ... +1 for adding a top level .gitignore entry. I don't have a Windows system to test on, but how come these files were only created in the ecpg testsuite? With the global .gitignore not mentioning *.obj it appears those files are not created anywhere else. Is the build process different for the rest of the tree? The MSVC build creates project directories which contain all the .obj files etc. The file locations for intermediate artefacts are quite different from the way a Unix build works. There is an ignore rule for these directories, which covers the .obj files there. But ecpg files are generated like in Unix builds. Since we have a global ignore rule for .o files it makes plenty of sense to have one for .obj files also. Certainly better than have one rule for each ecpg test case. cheers andrew -- 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] Object files generated by ecpg test suite not ignored on Windows
Andrew Dunstan and...@dunslane.net writes: On 03/09/2015 11:31 AM, Michael Meskes wrote: I don't have a Windows system to test on, but how come these files were only created in the ecpg testsuite? With the global .gitignore not mentioning *.obj it appears those files are not created anywhere else. Is the build process different for the rest of the tree? The MSVC build creates project directories which contain all the .obj files etc. The file locations for intermediate artefacts are quite different from the way a Unix build works. There is an ignore rule for these directories, which covers the .obj files there. But ecpg files are generated like in Unix builds. Ah. That's lots more plausible than no one's ever done any development on Windows. I can believe that not so many people have run the ecpg tests there. Since we have a global ignore rule for .o files it makes plenty of sense to have one for .obj files also. Certainly better than have one rule for each ecpg test case. Agreed. So we should revert the previous patch ... regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_rewind in contrib
Heikki Linnakangas wrote: Attached is a new patch version, fixing all the little things you listed. I believe this is pretty much ready for commit. I'm going to read it through myself one more time before committing, but I don't have anything mind now that needs fixing anymore. I do -- it's obvious that you've given some consideration to translatability, but it's not complete: anything that's using fprintf() and friends are not marked for translation with _(). There are lots of things using pg_fatal etc and those are probably fine. One big thing that's not currently translated is usage(), but there are others. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Object files generated by ecpg test suite not ignored on Windows
Actually, if we are supporting toolchains that generate *.obj files, I'd expect the top-level .gitignore to ignore them, as it does *.o. But if that's the issue why have we not heard complaints before? ... +1 for adding a top level .gitignore entry. I don't have a Windows system to test on, but how come these files were only created in the ecpg testsuite? With the global .gitignore not mentioning *.obj it appears those files are not created anywhere else. Is the build process different for the rest of the tree? Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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] Rethinking the parameter access hooks for plpgsql's benefit
On 03/09/2015 09:11 AM, Robert Haas wrote: On Sun, Mar 8, 2015 at 11:37 PM, Tom Lane t...@sss.pgh.pa.us wrote: Objections? Even better ideas? I object on the grounds that we're three weeks past the deadline for the last CommitFest, and that we should be trying to get committed those patches that were submitted on time, not writing new ones that will further increase the amount of reviewing that needs to be done before we can get to beta, and perhaps the bug count of that eventually when it arrives. In particular, I think that the fact that you haven't made more of an effort to give the GROUPING SETS patch a more detailed review is quite unfair to Andrew Gierth. But regardless of that, this is untimely and should be pushed to 9.6. From the reading the original post it seems like the patch was developed on Sales Force's time, not TGLs. I do not think we get to have an opinion on that. Thank you Tom for the efforts you made here, making the largest used PL better, faster, stronger is exactly the type of micro-changes we need to be looking at for further polish within the database. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, @cmdpromptinc Now I get it: your service is designed for a customer base that grew up with Facebook, watches Japanese seizure robot anime, and has the attention span of a gnat. I'm not that user., Tyler Riddle -- 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] failures with tuplesort and ordered set aggregates (due to 5cefbf5a6c44)
On Wed, Mar 4, 2015 at 1:55 PM, Peter Geoghegan p...@heroku.com wrote: On Wed, Mar 4, 2015 at 8:26 AM, Robert Haas robertmh...@gmail.com wrote: I think we should commit my patch, and if a future patch needs sortKeys set in more places, it can make that change itself. There's no reason why it's needed with the code as it is today, and no reason to let bits of future changes leak into a bug-fix patch. It's not as if I feel strongly about it. It's trivial to change the code to make the comment true, rather than change the comment to make the code true, and we need to do so anyway. I defer to you. I did it my way. Thanks. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Object files generated by ecpg test suite not ignored on Windows
On 03/09/2015 09:46 AM, Tom Lane wrote: Alvaro Herrera alvhe...@2ndquadrant.com writes: Michael Paquier wrote: When running the test suite of ecpg, build generates a set of obj files and it happens that those files are not ignored in the code tree. Wouldn't this be simpler as a *.obj pattern somewhere? Maybe in ecpg/test/.gitignore add this line? Actually, if we are supporting toolchains that generate *.obj files, I'd expect the top-level .gitignore to ignore them, as it does *.o. But if that's the issue why have we not heard complaints before? Probably because while there is lots of use on Windows there is probably very little development. +1 for adding a top level .gitignore entry. cheers andrew -- 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] Rethinking the parameter access hooks for plpgsql's benefit
On Sun, Mar 8, 2015 at 11:37 PM, Tom Lane t...@sss.pgh.pa.us wrote: Objections? Even better ideas? I object on the grounds that we're three weeks past the deadline for the last CommitFest, and that we should be trying to get committed those patches that were submitted on time, not writing new ones that will further increase the amount of reviewing that needs to be done before we can get to beta, and perhaps the bug count of that eventually when it arrives. In particular, I think that the fact that you haven't made more of an effort to give the GROUPING SETS patch a more detailed review is quite unfair to Andrew Gierth. But regardless of that, this is untimely and should be pushed to 9.6. On the technical side, I do agree that the requirement to allocate and zero an array for every new simple expression is unfortunate, but I'm not convinced that repeatedly invoking the hook-function is a good way to solve that problem. Calling the hook-function figures to be significantly more expensive than an array-fetch, so you can almost think of the ParamListInfo entries themselves as a cache of data retrieved from the hook function. In that view of the world, the problem here is that initializing the cache is too expensive. Your solution to that is to rip the cache out completely, but what would be better is keep the cache and make initializing it cheaper - e.g. by sharing one ParamListInfo across all expressions in the same scope; or by not zeroing the memory and instead tracking the the first N slots are the only ones we've initialized; or $your_idea_here. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates
On 09/03/15 13:39, Andreas Karlsson wrote: On 03/07/2015 07:18 PM, Petr Jelinek wrote: What I am wondering is if those numeric_int16_* functions that also deal with either the Int128AggState or NumericAggState should be renamed in similar fashion. You mean something like numeric_poly_sum instead of numeric_int16_sum? I personally am not fond of either name. While numeric_int16_* incorrectly implies we have a int16 SQL type numeric_poly_* does not tell us that this is an optimized version which uses a smaller state. Yes that's what I mean, since the int16 in the name is misleading given that in at least some builds the int16 won't be used. You could always have numeric function, int16 function and the poly function which decides between them but that's probably overkill. The worst part of writing this patch has always been naming functions and types. :) Naming is hard :) -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Question about lazy_space_alloc() / linux over-commit
Robert Haas wrote: On Sat, Mar 7, 2015 at 5:49 PM, Andres Freund and...@2ndquadrant.com wrote: On 2015-03-05 15:28:12 -0600, Jim Nasby wrote: I was thinking the simpler route of just repalloc'ing... the memcpy would suck, but much less so than the extra index pass. 64M gets us 11M tuples, which probably isn't very common. That has the chance of considerably increasing the peak memory usage though, as you obviously need both the old and new allocation during the repalloc(). And in contrast to the unused memory at the tail of the array, which will usually not be actually allocated by the OS at all, this is memory that's actually read/written respectively. Yeah, I'm not sure why everybody wants to repalloc() that instead of making several separate allocations as needed. That would avoid increasing peak memory usage, and would avoid any risk of needing to copy the whole array. Also, you could grow in smaller chunks, like 64MB at a time instead of 1GB or more at a time. Doubling an allocation that's already 1GB or more gets big in a hurry. Yeah, a chunk list rather than a single chunk seemed a good idea to me too. Also, I think the idea of starting with an allocation assuming some small number of dead tuples per page made sense -- and by the time that space has run out, you have a better estimate of actual density of dead tuples, so you can do the second allocation based on that new estimate (but perhaps clamp it at say 1 GB, just in case you just scanned a portion of the table with an unusually high percentage of dead tuples.) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Turning off HOT/Cleanup sometimes
On Wed, Dec 17, 2014 at 12:39 AM, Simon Riggs si...@2ndquadrant.com wrote: On 15 December 2014 at 20:26, Jeff Janes jeff.ja...@gmail.com wrote: I still get the compiler error in contrib: pgstattuple.c: In function 'pgstat_heap': pgstattuple.c:279: error: too few arguments to function 'heap_beginscan_strat' Should it pass false for the always_prune? Yes. New version attached. This no longer applies directly against head, but if I apply to an older checkout and then do git checkout -m origin it rolls forward cleanly. Did versions 7 and 8 of this patch address Andres' concern about performance regressions? Thanks, Jeff
Re: [HACKERS] Bootstrap DATA is a pita
On 2015-03-07 18:09:36 -0600, Jim Nasby wrote: How often does a normal user actually initdb? I don't think it's that incredibly common. Added time to our development cycle certainly is a concern though. There's many shops that run initdb as part of their test/CI systems. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rethinking the parameter access hooks for plpgsql's benefit
Joshua D. Drake j...@commandprompt.com writes: On 03/09/2015 09:11 AM, Robert Haas wrote: I object on the grounds that we're three weeks past the deadline for the last CommitFest, and that we should be trying to get committed those patches that were submitted on time, not writing new ones that will further increase the amount of reviewing that needs to be done before we can get to beta, and perhaps the bug count of that eventually when it arrives. In particular, I think that the fact that you haven't made more of an effort to give the GROUPING SETS patch a more detailed review is quite unfair to Andrew Gierth. But regardless of that, this is untimely and should be pushed to 9.6. From the reading the original post it seems like the patch was developed on Sales Force's time, not TGLs. I do not think we get to have an opinion on that. JD sees the situation correctly: this is $dayjob work, and it's going to get done now not in four months because I have a deadline to meet. I would like to push it into the community sources to reduce divergence between our copy and Salesforce's, but if I'm told it has to wait till 9.6, I may or may not remember to try to do something then. I will admit that I'm been slacking on commitfest work. This is not unrelated to the fact that we've been in commitfest mode continuously since last August. I'm afraid whatever enthusiasm I had for reviewing other peoples' patches burned out some time ago. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rethinking the parameter access hooks for plpgsql's benefit
* Andres Freund (and...@2ndquadrant.com) wrote: JD sees the situation correctly: this is $dayjob work, and it's going to get done now not in four months because I have a deadline to meet. I would like to push it into the community sources to reduce divergence between our copy and Salesforce's, but if I'm told it has to wait till 9.6, I may or may not remember to try to do something then. I think most of the committers are pretty much in that situation? Indeed.. And, further, the commitfest wasn't intended to prevent committers from working on things, at least as I understood it. Core sets a feature freeze date which is quite a different thing. Considering feature freeze is generally after the last commitfest it also seems counter-intuitive that committers are expected to have finished all of their work for the next release prior to the start of the last commitfest. I will admit that I'm been slacking on commitfest work. This is not unrelated to the fact that we've been in commitfest mode continuously since last August. I'm afraid whatever enthusiasm I had for reviewing other peoples' patches burned out some time ago. I can certainly relate to that :(. +1. Thanks, Stephen signature.asc Description: Digital signature
Re: [HACKERS] Rethinking the parameter access hooks for plpgsql's benefit
On Mon, Mar 9, 2015 at 12:41 PM, Tom Lane t...@sss.pgh.pa.us wrote: Joshua D. Drake j...@commandprompt.com writes: On 03/09/2015 09:11 AM, Robert Haas wrote: I object on the grounds that we're three weeks past the deadline for the last CommitFest, and that we should be trying to get committed those patches that were submitted on time, not writing new ones that will further increase the amount of reviewing that needs to be done before we can get to beta, and perhaps the bug count of that eventually when it arrives. In particular, I think that the fact that you haven't made more of an effort to give the GROUPING SETS patch a more detailed review is quite unfair to Andrew Gierth. But regardless of that, this is untimely and should be pushed to 9.6. From the reading the original post it seems like the patch was developed on Sales Force's time, not TGLs. I do not think we get to have an opinion on that. JD sees the situation correctly: this is $dayjob work, and it's going to get done now not in four months because I have a deadline to meet. I would like to push it into the community sources to reduce divergence between our copy and Salesforce's, but if I'm told it has to wait till 9.6, I may or may not remember to try to do something then. Sure, I have things that I've wanted to push into the community sources for the benefit of EnterpriseDB, too. Nobody's offered to let me ignore the community process when it happens to be good for EnterpriseDB. If we're changing that policy for patches submitted by Salesforce employes, I'm afraid I must object unless EnterpriseDB employees will get the same privilege. And I think 2ndQuadrant will want in on it, too. I will admit that I'm been slacking on commitfest work. This is not unrelated to the fact that we've been in commitfest mode continuously since last August. I'm afraid whatever enthusiasm I had for reviewing other peoples' patches burned out some time ago. Yeah, it would be so much easier to get the release out the door if people didn't keep submitting patches to make the product better. We should really try to discourage that. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Calling for a replacement committer for GROUPING SETS
Has there been anything controversial? What was causing it to take so long. I have time to work on it now On 9 Mar 2015 17:06, Andrew Gierth and...@tao11.riddles.org.uk wrote: In the light of Tom's comment in 22996.1425919...@sss.pgh.pa.us: Tom I will admit that I'm been slacking on commitfest work. This is Tom not unrelated to the fact that we've been in commitfest mode Tom continuously since last August. I'm afraid whatever enthusiasm I Tom had for reviewing other peoples' patches burned out some time ago. I'm now requesting that some other committer take on the grouping sets patch. Any takers? -- Andrew (irc:RhodiumToad) -- 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] Using 128-bit integers for sum, avg and statistics aggregates
On Mon, Mar 09, 2015 at 01:39:04PM +0100, Andreas Karlsson wrote: On 03/07/2015 07:18 PM, Petr Jelinek wrote: What I am wondering is if those numeric_int16_* functions that also deal with either the Int128AggState or NumericAggState should be renamed in similar fashion. You mean something like numeric_poly_sum instead of numeric_int16_sum? I personally am not fond of either name. While numeric_int16_* incorrectly implies we have a int16 SQL type numeric_poly_* does not tell us that this is an optimized version which uses a smaller state. Would it be simpler to write a separate patch to add an int16 SQL type so that this implication is correct? The worst part of writing this patch has always been naming functions and types. :) Cheers, David. -- David Fetter da...@fetter.org http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: david.fet...@gmail.com Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates
On 09/03/15 18:39, David Fetter wrote: On Mon, Mar 09, 2015 at 01:39:04PM +0100, Andreas Karlsson wrote: On 03/07/2015 07:18 PM, Petr Jelinek wrote: What I am wondering is if those numeric_int16_* functions that also deal with either the Int128AggState or NumericAggState should be renamed in similar fashion. You mean something like numeric_poly_sum instead of numeric_int16_sum? I personally am not fond of either name. While numeric_int16_* incorrectly implies we have a int16 SQL type numeric_poly_* does not tell us that this is an optimized version which uses a smaller state. Would it be simpler to write a separate patch to add an int16 SQL type so that this implication is correct? No, because then you'd need to emulate the type on platforms where it does not exist and the patch would be several times more complex for no useful reason. -- Petr Jelinek http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rethinking the parameter access hooks for plpgsql's benefit
JD sees the situation correctly: this is $dayjob work, and it's going to get done now not in four months because I have a deadline to meet. I would like to push it into the community sources to reduce divergence between our copy and Salesforce's, but if I'm told it has to wait till 9.6, I may or may not remember to try to do something then. I think most of the committers are pretty much in that situation? I will admit that I'm been slacking on commitfest work. This is not unrelated to the fact that we've been in commitfest mode continuously since last August. I'm afraid whatever enthusiasm I had for reviewing other peoples' patches burned out some time ago. I can certainly relate to that :(. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rethinking the parameter access hooks for plpgsql's benefit
On 03/09/2015 09:29 AM, Robert Haas wrote: On Mon, Mar 9, 2015 at 12:26 PM, Joshua D. Drake j...@commandprompt.com wrote: From the reading the original post it seems like the patch was developed on Sales Force's time, not TGLs. I do not think we get to have an opinion on that. Salesforce gets to develop their patches whenever they want, but they don't get to control the community process for getting those patches committed to PostgreSQL. Did I miss the part of the post that said, (speaking as Tom): FYI, I am ignoring the commit fest deadline and committing this? Are we supposed to hold patches when we are post-deadline and not seek feedback? That seems rather hostile. Thank you Tom for the efforts you made here, making the largest used PL better, faster, stronger is exactly the type of micro-changes we need to be looking at for further polish within the database. So, just to be clear, are you proposing that everybody is exempted from the CommitFest deadlines, or just Tom? I am not suggesting that anyone is exempted from a CommitFest deadline. I am actually rather hard line that we should be keeping them. I think it is ridiculous to post on the bad/good/timing of a patch submission unless there is a case being made that the process isn't actually being followed. I don't see that here. In short, soap boxing does nobody any good. JD -- Command Prompt, Inc. - http://www.commandprompt.com/ 503-667-4564 PostgreSQL Support, Training, Professional Services and Development High Availability, Oracle Conversion, @cmdpromptinc Now I get it: your service is designed for a customer base that grew up with Facebook, watches Japanese seizure robot anime, and has the attention span of a gnat. I'm not that user., Tyler Riddle -- 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] Rethinking the parameter access hooks for plpgsql's benefit
On Mon, Mar 9, 2015 at 1:03 PM, Andres Freund and...@2ndquadrant.com wrote: On 2015-03-09 12:54:44 -0400, Robert Haas wrote: If we're changing that policy for patches submitted by Salesforce employes, I'm afraid I must object unless EnterpriseDB employees will get the same privilege. And I think 2ndQuadrant will want in on it, too. Right. I'm not really sure how much actual policy there is around this, and how much (differently) perceived policy it is. Yes, that's fair. I do realize that long-time, highly-trusted committers, especially Tom, get more slack than newer committers or, say, first-time patch submitters, and that is fair too. At the same time, at some point we have to cut off 9.5 development. I will admit that I'm been slacking on commitfest work. This is not unrelated to the fact that we've been in commitfest mode continuously since last August. I'm afraid whatever enthusiasm I had for reviewing other peoples' patches burned out some time ago. Yeah, it would be so much easier to get the release out the door if people didn't keep submitting patches to make the product better. We should really try to discourage that. I don't think that's really the point. There's been awfully little progress on the committing side, and what did happen has been born by few shoulders. Which leaves little time that's officially dedicated to the fun parts. Yeah, sorry, I lost my temper there a bit. Sorry, Tom. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rethinking the parameter access hooks for plpgsql's benefit
I don't think Tom, or that matter anyone needs to forgo working on changes at any time. The only effect missing a commitfest deadline means is that other reviewers don't offer any promises to give any feedback on it before this round of the commitfest is done. We don't have a policy of requiring code reviews before changes are committed -- it's up to the commuters judgement whether the patch is type for committing. There has been the suggestion that commiters should concentrate on reviews rather than fresh development during commitfests but nobody is going to do 100% reviewing work for the whole time and nobody could seriously suggest Tom hasn't pulled his weight reviewing patches.
Re: [HACKERS] Rethinking the parameter access hooks for plpgsql's benefit
On Mon, Mar 9, 2015 at 12:53 PM, Joshua D. Drake j...@commandprompt.com wrote: I think it is ridiculous to post on the bad/good/timing of a patch submission unless there is a case being made that the process isn't actually being followed. I don't see that here. The CommitFest deadline was three weeks ago and I think it's abundantly clear that Tom is not submitting this for the June CommitFest. If that wasn't clear from his initial post, his follow-up points have removed any doubt on that point. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Calling for a replacement committer for GROUPING SETS
On 2015-03-09 17:17:14 +, Greg Stark wrote: Has there been anything controversial? What was causing it to take so long. I have time to work on it now If you want to, you probably need to read the relevant thread. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Calling for a replacement committer for GROUPING SETS
Greg == Greg Stark st...@mit.edu writes: Greg Has there been anything controversial? The major controversy is the idea of processing multiple sort orderings using a stacked chain of aggregate and sort nodes. I'll follow up shortly with links or a summary of the most significant criticisms; but note that the patch has already been fixed to keep the memory usage bounded regardless of the depth of the chain, so the issue is more about code and design aesthetics than about actually working. There is a description of the current approach in the block comment at the start of executor/nodeAgg.c. A less major issue is whether the introduction of GroupedVar was a good idea; I'm happy with ripping that out again, but if so the code needed to replace it depends on the resolution of the above point, so I've left it as originally submitted pending a decision on that. Greg What was causing it to take so long. Tom claimed the reviewer/committer spot for himself five or six months ago, and other than the inadequate review and subsequent discussion in December there has been no progress. We posted the latest patch shortly after that (there was some slight delay thanks to the holiday season). Greg I have time to work on it now Great. I'm actually about to post the latest patch with some tiny updates to match the recent changes in MemoryContextReset, but those are as much cosmetic as anything else (just removing no-longer-needed calls to MemoryContextDeleteChildren). -- Andrew (irc:RhodiumToad) -- 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] Object files generated by ecpg test suite not ignored on Windows
On 09.03.2015 16:58, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: The MSVC build creates project directories which contain all the .obj files etc. The file locations for intermediate artefacts are quite different from the way a Unix build works. There is an ignore rule for these directories, which covers the .obj files there. But ecpg files are generated like in Unix builds. Ah. That's lots more plausible than no one's ever done any development on Windows. I can believe that not so many people have run the ecpg tests there. Completely agreed. Since we have a global ignore rule for .o files it makes plenty of sense to have one for .obj files also. Certainly better than have one rule for each ecpg test case. Agreed. So we should revert the previous patch ... Done. Michael -- Michael Meskes Michael at Fam-Meskes dot De, Michael at Meskes dot (De|Com|Net|Org) Michael at BorussiaFan dot De, Meskes at (Debian|Postgresql) dot Org Jabber: michael.meskes at gmail dot com VfL Borussia! Força Barça! Go SF 49ers! Use Debian GNU/Linux, PostgreSQL -- 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] Rethinking the parameter access hooks for plpgsql's benefit
On 2015-03-09 12:54:44 -0400, Robert Haas wrote: If we're changing that policy for patches submitted by Salesforce employes, I'm afraid I must object unless EnterpriseDB employees will get the same privilege. And I think 2ndQuadrant will want in on it, too. Right. I'm not really sure how much actual policy there is around this, and how much (differently) perceived policy it is. I will admit that I'm been slacking on commitfest work. This is not unrelated to the fact that we've been in commitfest mode continuously since last August. I'm afraid whatever enthusiasm I had for reviewing other peoples' patches burned out some time ago. Yeah, it would be so much easier to get the release out the door if people didn't keep submitting patches to make the product better. We should really try to discourage that. I don't think that's really the point. There's been awfully little progress on the committing side, and what did happen has been born by few shoulders. Which leaves little time that's officially dedicated to the fun parts. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] File based Incremental backup v8
On Sat, Mar 7, 2015 at 5:45 PM, Gabriele Bartolini gabriele.bartol...@2ndquadrant.it wrote: By the way, unless I'm missing something, this patch only seems to include the code to construct an incremental backup, but no tools whatsoever to do anything useful with it once you've got it. As stated previously, Marco is writing a tool called pg_restorebackup (the prototype in Python has been already posted) to be included in the core. I am in Australia now and not in the office so I cannot confirm it, but I am pretty sure he had already written it and was about to send it to the list. Gabriele, the deadline for the last CommitFest was three weeks ago. Having a patch that you are about to send to the list is not good enough at this point. I think that's 100% unacceptable. I agree, that's why pg_restorebackup written in C is part of this patch. See: https://wiki.postgresql.org/wiki/Incremental_backup No, it *isn't* part of this patch. You may have a plan to add it to this patch, but that's not the same thing. The goal has always been to provide file-based incremental backup. I can assure that this has always been our compass and the direction to follow. Regardless of community feedback? OK. Let's see how that works out for you. I repeat that, using pg_restorebackup, this patch will transparently let users benefit from incremental backup even when it will be moved to an internal block-level logic. Users will continue to execute pg_basebackup and pg_restorebackup, ignoring that with - for example 9.5 - it is file-based (saving between 50-70% of space and time) of block level - for example 9.6. I understand that. But I also understand that in other cases it's going to be slower than a full backup. This problem has been pointed out several times, and you're just refusing to admit that it's a real issue. A user with a bunch of tables where only the rows near the end of the file get updated is going to repeatedly read those files until it hits the first modified block and then rewind and reread the whole file. I pointed this problem out back in early October and suggested some ways of fixing it; Heikki followed up with his own suggestions for modifying my idea. Instead of implementing any of that, or even discussing it, you're still plugging away on a design that no committer has endorsed and that several committers obviously have concerns about. It's also pretty clear that nobody likes the backup profile, at least in the form it exists today. But it's still here, many patch versions later. I think there's absolutely no point in spending more time on this for 9.5. At least 4 committers have looked at it and none of them are convinced by the current design; feedback from almost half a year ago hasn't been incorporated; obviously-needed parts of the patch (pg_restorebackup) are missing weeks after the last CF deadline. Let's mark this Rejected in the CF app and move on. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Calling for a replacement committer for GROUPING SETS
In the light of Tom's comment in 22996.1425919...@sss.pgh.pa.us: Tom I will admit that I'm been slacking on commitfest work. This is Tom not unrelated to the fact that we've been in commitfest mode Tom continuously since last August. I'm afraid whatever enthusiasm I Tom had for reviewing other peoples' patches burned out some time ago. I'm now requesting that some other committer take on the grouping sets patch. Any takers? -- Andrew (irc:RhodiumToad) -- 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] Question about lazy_space_alloc() / linux over-commit
On Sat, Mar 7, 2015 at 5:49 PM, Andres Freund and...@2ndquadrant.com wrote: On 2015-03-05 15:28:12 -0600, Jim Nasby wrote: I was thinking the simpler route of just repalloc'ing... the memcpy would suck, but much less so than the extra index pass. 64M gets us 11M tuples, which probably isn't very common. That has the chance of considerably increasing the peak memory usage though, as you obviously need both the old and new allocation during the repalloc(). And in contrast to the unused memory at the tail of the array, which will usually not be actually allocated by the OS at all, this is memory that's actually read/written respectively. Yeah, I'm not sure why everybody wants to repalloc() that instead of making several separate allocations as needed. That would avoid increasing peak memory usage, and would avoid any risk of needing to copy the whole array. Also, you could grow in smaller chunks, like 64MB at a time instead of 1GB or more at a time. Doubling an allocation that's already 1GB or more gets big in a hurry. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Question about lazy_space_alloc() / linux over-commit
On 3/9/15 12:28 PM, Alvaro Herrera wrote: Robert Haas wrote: On Sat, Mar 7, 2015 at 5:49 PM, Andres Freund and...@2ndquadrant.com wrote: On 2015-03-05 15:28:12 -0600, Jim Nasby wrote: I was thinking the simpler route of just repalloc'ing... the memcpy would suck, but much less so than the extra index pass. 64M gets us 11M tuples, which probably isn't very common. That has the chance of considerably increasing the peak memory usage though, as you obviously need both the old and new allocation during the repalloc(). And in contrast to the unused memory at the tail of the array, which will usually not be actually allocated by the OS at all, this is memory that's actually read/written respectively. Yeah, I'm not sure why everybody wants to repalloc() that instead of making several separate allocations as needed. That would avoid increasing peak memory usage, and would avoid any risk of needing to copy the whole array. Also, you could grow in smaller chunks, like 64MB at a time instead of 1GB or more at a time. Doubling an allocation that's already 1GB or more gets big in a hurry. Yeah, a chunk list rather than a single chunk seemed a good idea to me too. That will be significantly more code than a simple repalloc, but as long as people are OK with that I can go that route. Also, I think the idea of starting with an allocation assuming some small number of dead tuples per page made sense -- and by the time that space has run out, you have a better estimate of actual density of dead tuples, so you can do the second allocation based on that new estimate (but perhaps clamp it at say 1 GB, just in case you just scanned a portion of the table with an unusually high percentage of dead tuples.) I like the idea of using a fresh idea of dead tuple density when we need more space. We would also clamp this at maintenance_work_mem, not a fixed 1GB. Speaking of which... people have referenced allowing 1GB of dead tuples, which means allowing maintenance_work_mem MAX_KILOBYTES. The comment for that says: /* upper limit for GUC variables measured in kilobytes of memory */ /* note that various places assume the byte size fits in a long variable */ So I'm not sure how well that will work. I think that needs to be a separate patch. -- Jim Nasby, Data Architect, Blue Treble Consulting Data in Trouble? Get it in Treble! http://BlueTreble.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rethinking the parameter access hooks for plpgsql's benefit
Robert Haas robertmh...@gmail.com writes: On Mon, Mar 9, 2015 at 12:41 PM, Tom Lane t...@sss.pgh.pa.us wrote: JD sees the situation correctly: this is $dayjob work, and it's going to get done now not in four months because I have a deadline to meet. I would like to push it into the community sources to reduce divergence between our copy and Salesforce's, but if I'm told it has to wait till 9.6, I may or may not remember to try to do something then. Sure, I have things that I've wanted to push into the community sources for the benefit of EnterpriseDB, too. Nobody's offered to let me ignore the community process when it happens to be good for EnterpriseDB. If we're changing that policy for patches submitted by Salesforce employes, I'm afraid I must object unless EnterpriseDB employees will get the same privilege. And I think 2ndQuadrant will want in on it, too. As far as that goes, it has never been the case that we expected every patch to go through the commitfest review process. (If we did, our response time to bugs would be probably a couple orders of magnitude longer than it is.) The way I see the policy is that committers have authority to commit things that they feel are ready and that haven't been objected to by other developers. Depending on the particular committer and the particular patch, feeling that a patch is ready might or might not require that it's been through a commitfest review. There is no process-based restriction on committing ready patches except when we are in alpha/beta/RC states, which is surely months away for 9.5. If I were looking for a formal review on this one, I would certainly not expect that it would get one during the current CF. I wasn't though. For context, I have currently got half a dozen irons in the fire: * expanded objects (array performance fixes): needs review, was submitted timely to current CF * operator precedence changes: committable IMO, but it's not entirely clear if we have consensus * adjust loop count estimates for semijoins: committable IMO, waiting for objections * find stats on-the-fly for children of UNION ALL appendrels: WIP, far from done though * flatten empty sub-SELECTs and VALUES where feasible: committable IMO, waiting for objections * parameter fetch hook rethink: WIP, not as close to done as I thought last night I wasn't really planning to use the CF process for any but the first of these. If we start insisting that committers go through CF for even rather small patches like these other things, it's going to destroy our productivity completely. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] tracking commit timestamps
Petr Jelinek wrote:
On Thu, Dec 4, 2014 at 12:08 PM, Fujii Masao masao.fu...@gmail.com
wrote:
I got the following assertion failure when I executed
pg_xact_commit_timestamp()
in the standby server.
=# select pg_xact_commit_timestamp('1000'::xid);
TRAP: FailedAssertion(!(((oldestCommitTs) != ((TransactionId) 0)) ==
((newestCommitTs) != ((TransactionId) 0))), File: commit_ts.c,
Line: 315)
Attached patch fixes it, I am not sure how happy I am with the way I did
it though.
Pushed, thanks.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Calling for a replacement committer for GROUPING SETS
Andrew Gierth and...@tao11.riddles.org.uk writes: In the light of Tom's comment in 22996.1425919...@sss.pgh.pa.us: Tom I will admit that I'm been slacking on commitfest work. This is Tom not unrelated to the fact that we've been in commitfest mode Tom continuously since last August. I'm afraid whatever enthusiasm I Tom had for reviewing other peoples' patches burned out some time ago. I'm now requesting that some other committer take on the grouping sets patch. Any takers? FWIW, slacking doesn't mean I've abandoned Postgres work completely. I'm almost out from under some work-related deadlines (modulo the odd plpgsql performance improvement patch or two ;-)) and expect to get back to your patch and others in the CF before too long. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rethinking the parameter access hooks for plpgsql's benefit
On Mon, Mar 9, 2015 at 12:06 PM, Robert Haas robertmh...@gmail.com wrote: Yes, I understand. I don't really have anything more to say about this. Nothing here changes my basic feeling that we need to stop putting new irons in the fire and start working hard on taking irons out of the fire; and I think most if not all other committers are already doing that. Even to the extent that they are focusing on their own patches rather than other people's patches, I think it is mostly patches that they wrote some time ago, not new things that they have only just written. I must say that I share your concern here. I have no idea what's going to happen with my ON CONFLICT patch, 9.5-wise. I hope that at least the IGNORE variant makes it into 9.5, but I'm not sure that it will. The ON CONFLICT IGNORE/UPDATE patch has existed in essentially the same form since August, although research level work went into that project as long ago as 2012. I'm not blaming anyone, but it's quite discouraging that it has taken so much to get it to where it is now, even though it's something that there is a huge, tangible demand for (few other things are in the same category, and few of those have actually been implemented). An enormous amount of effort went into internals documentation (the Wiki pages), and into stress-testing (Jeff Janes' double entry bookkeeping stress test, among others). I hate to say it, but at some point I'll need to cut my loses. To any interested contributor: If there is something that I can do to help to advance the state of the ON CONFLICT UPDATE patch, let me know. Perhaps there can be some useful reciprocal arrangement with review time. That's not ideal, but I see no alternative. -- Peter Geoghegan -- 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] Calling for a replacement committer for GROUPING SETS
Tom == Tom Lane t...@sss.pgh.pa.us writes: I'm now requesting that some other committer take on the grouping sets patch. Any takers? Tom FWIW, slacking doesn't mean I've abandoned Postgres work Tom completely. I had taken your name off the patch in the CF and was planning to post this request even before I saw rhaas' post, much less your response. It's been almost six months since you claimed the patch, and three months since you said you would look into the question of the CTE-based approach per our discussion in December. So frankly I don't feel confident in your expect ... before too long ;-) So my request stands. I'll work with whoever gets to it first. -- Andrew (irc:RhodiumToad) -- 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 user/role CURRENT_USER
Alvaro Herrera wrote: With this patch applied, doing \h ALTER ROLE in psql looks quite odd: note how wide it has become. Maybe we should be doing this differently? (Hmm, why don't we accept ALL in the first SET line? Maybe that's just a mistake and the four lines should be all identical in the first half ...) I have fixed the remaining issues, completed the doc changes, and pushed. Given the lack of feedback I had to follow my gut on the best way to change the docs. I added the regression test you submitted with some additional changes, mainly to make sure they don't fail immediately when other databases exist; maybe some more concurrency or platform issues will show up there, but let's see what the buildfarm says. Thanks Horiguchi-san for the patch and everyone for the reviews. (It's probably worthwhile giving things an extra look.) -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rethinking the parameter access hooks for plpgsql's benefit
On Mon, Mar 9, 2015 at 2:33 PM, Tom Lane t...@sss.pgh.pa.us wrote: As far as that goes, it has never been the case that we expected every patch to go through the commitfest review process. (If we did, our response time to bugs would be probably a couple orders of magnitude longer than it is.) The way I see the policy is that committers have authority to commit things that they feel are ready and that haven't been objected to by other developers. Depending on the particular committer and the particular patch, feeling that a patch is ready might or might not require that it's been through a commitfest review. There is no process-based restriction on committing ready patches except when we are in alpha/beta/RC states, which is surely months away for 9.5. If I were looking for a formal review on this one, I would certainly not expect that it would get one during the current CF. I wasn't though. Yes, I understand. I don't really have anything more to say about this. Nothing here changes my basic feeling that we need to stop putting new irons in the fire and start working hard on taking irons out of the fire; and I think most if not all other committers are already doing that. Even to the extent that they are focusing on their own patches rather than other people's patches, I think it is mostly patches that they wrote some time ago, not new things that they have only just written. I also will say, in fairness, that I do not really care about this particular patch all that much one way or the other. I am less convinced than you that this will always work out to a win, but when it comes right down to it, you're a pretty smart guy, and if this gets complaints about some scenario you've overlooked, I have confidence you'll get around to repairing it. So whatever. What I do care about is that we as a project have to at some point be willing to begin closing the spigot on new patches and focusing on polishing and shipping the patches we've already got. I think it's perfectly reasonable to worry about where we are on that continuum when it's already several weeks after the start of the last CommitFest, but if I'm in the minority on that, so be it. But it's got to be done at some point, or we will not be able to ship a beta, or a release, on the anticipated timetable. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Rethinking the parameter access hooks for plpgsql's benefit
On 2015-03-09 13:15:59 -0700, Peter Geoghegan wrote: I must say that I share your concern here. I have no idea what's going to happen with my ON CONFLICT patch, 9.5-wise. I hope that at least the IGNORE variant makes it into 9.5, but I'm not sure that it will. The ON CONFLICT IGNORE/UPDATE patch has existed in essentially the same form since August, although research level work went into that project as long ago as 2012. I'm not blaming anyone, but it's quite discouraging that it has taken so much to get it to where it is now, even though it's something that there is a huge, tangible demand for (few other things are in the same category, and few of those have actually been implemented). An enormous amount of effort went into internals documentation (the Wiki pages), and into stress-testing (Jeff Janes' double entry bookkeeping stress test, among others). I hate to say it, but at some point I'll need to cut my loses. To any interested contributor: If there is something that I can do to help to advance the state of the ON CONFLICT UPDATE patch, let me know. Perhaps there can be some useful reciprocal arrangement with review time. That's not ideal, but I see no alternative. FWIW, I think you actually don't have much reason to complain. This work has probably gotten more attention in total than any other recent patch. Certainly, by far, more than any other in the 9.5 cycle. So far I've not seen a single version that could be considered 'ready for committer'. Even if there's points to be resolved you can make one (presumably yours) solution ready. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch : Allow toast tables to be moved to a different tablespace
On 03/03/2015 04:14 PM, Julien Tachoires wrote: On 30/12/2014 03:48, Andreas Karlsson wrote: - A test fails in create_view.out. I looked some into it and did not see how this could happen. [...] I can't reproduce this issue. Neither can I anymore. - pg_dump is broken pg_dump: [archiver (db)] query failed: ERROR: syntax error at or near ( LINE 1: ...nest(tc.reloptions) x), ', ') AS toast_reloptions (SELECT sp... Fixed. I still get a failing pg_dump test though when running make check-world. And it does not work when run manually either. + pg_dumpall -f /home/andreas/dev/postgresql/contrib/pg_upgrade/tmp_check/dump1.sql pg_dump: column number -1 is out of range 0..28 cannot duplicate null pointer (internal error) pg_dumpall: pg_dump failed on database postgres, exiting + pg_dumpall1_status=1 + [ /home/andreas/dev/postgresql != /home/andreas/dev/postgresql ] + /home/andreas/dev/postgresql/contrib/pg_upgrade/tmp_check/install//home/andreas/dev/postgresql-inst/bin/pg_ctl -m fast stop waiting for server to shut down done server stopped + [ -n ] + [ -n ] + [ -n 1 ] + echo pg_dumpall of pre-upgrade database cluster failed pg_dumpall of pre-upgrade database cluster failed + exit 1 + rm -rf /tmp/pg_upgrade_check-5A3wsI - I do not like how \d handles the toast tablespace. Having TOAST in pg_default and the table in another space looks the same as if there was no TOAST table at all. I think we should always print both tablespaces if either of them are not pg_default. If we do it that way, we should always show the tablespace even if it's pg_default in any case to be consistent. I'm pretty sure that we don't want that. I think we will have to agree to disagree here. I think it should be obvious when the toast table is in the default tablespace for tables outside it. - Would it be interesting to add syntax for moving the toast index to a separate tablespace? -1, we just want to be able to move TOAST heap, which is supposed to contain a lot of data and we want to move on a different storage device / filesystem. I think we should allow moving the indexes for consistency. With this patch we can move everything except for TOAST indexes. - There is no warning if you set the toast table space of a table lacking toast. I think there should be one. A notice is raised now in that case. Excellent, also add a test case for this. - Missing periods on the ALTER TABLE manual page after See also CREATE TABLESPACE (in two places). - Missing period last in the new paragraph added to the storage manual page. I don't understand those 2 last points ? I mean that TOAST table can be moved to a different tablespace with commandALTER TABLE SET TOAST TABLESPACE/ should be changed to TOAST table can be moved to a different tablespace with commandALTER TABLE SET TOAST TABLESPACE/. since a sentece should always end in .. = New comments - The patch does not apply cleanly anymore, I had to solve to minor conflicts. - Rewrap the documentation for SET TABLESPACE. - You have added a tab in alter_table.sgml. - Merge case AT_SetTableTableSpace: and case AT_SetToastTableSpace: where they both do the same thing, i.e. nothing. - Should it not be foo_mv in the query from the test suite below? SELECT spcname FROM pg_class c JOIN pg_class d ON (c.reltoastrelid=d.oid) JOIN pg_tablespace ON (d.reltablespace = pg_tablespace.oid) WHERE c.relname = 'foo2'; -- Andreas Karlsson -- 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] BRIN page type identifier
Alvaro Herrera alvhe...@2ndquadrant.com writes:
I wonder if this is permissible and whether it will do the right thing
on 32-bit systems:
/*
* Special area of BRIN pages.
*
* We add some padding bytes to ensure that 'type' ends up in the last two
* bytes of the page, for consumption by pg_filedump and similar utilities.
* (Special space is MAXALIGN'ed).
*/
typedef struct BrinSpecialSpace
{
charpadding[MAXALIGN(1) - 2 * sizeof(uint16)];
uint16 flags;
uint16 type;
} BrinSpecialSpace;
I should expect that to fail altogether on 32-bit machines, because
the declared array size will be zero.
You could try something like
typedef struct BrinSpecialSpace
{
uint16 vector[MAXALIGN(1) / sizeof(uint16)];
} BrinSpecialSpace;
and then some access macros to use the last and next-to-last
elements of that array.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates
On 03/07/2015 07:18 PM, Petr Jelinek wrote:
What I am wondering is if those numeric_int16_* functions that also deal
with either the Int128AggState or NumericAggState should be renamed in
similar fashion.
Here is a patch where I have renamed the functions.
int128-agg-v7.patch
- Rename numeric_int16_* to numeric_poly_*
- Rename static functions int{8,16}_to_numericvar to
int{64,128}_to_numericvar
- Fix typo in c-compile.m4 comment
--
Andreas Karlsson
diff --git a/config/c-compiler.m4 b/config/c-compiler.m4
index 509f961..48fcc68 100644
--- a/config/c-compiler.m4
+++ b/config/c-compiler.m4
@@ -125,6 +125,41 @@ undefine([Ac_cachevar])dnl
])# PGAC_TYPE_64BIT_INT
+# PGAC_HAVE___INT128
+# --
+# Check if __int128 is a working 128 bit integer type and
+# define HAVE___INT128 if so.
+AC_DEFUN([PGAC_HAVE___INT128],
+[AC_CACHE_CHECK(for __int128, pgac_cv_have___int128,
+[AC_TRY_RUN([
+__int128 a = 2001;
+__int128 b = 4005;
+
+int does_int128_work()
+{
+ __int128 c,d;
+
+ c = a * b;
+ d = (c + b) / b;
+ if (d != a+1)
+return 0;
+ return 1;
+}
+main() {
+ exit(! does_int128_work());
+}],
+[pgac_cv_have___int128=yes],
+[pgac_cv_have___int128=no],
+[# If cross-compiling, check the size reported by the compiler and
+# trust that the arithmetic works.
+AC_COMPILE_IFELSE([AC_LANG_BOOL_COMPILE_TRY([], [sizeof(__int128) == 16])],
+ pgac_cv_have___int128=yes,
+ pgac_cv_have___int128=no)])])
+
+if test x$pgac_cv_have___int128 = xyes ; then
+ AC_DEFINE(HAVE___INT128, 1, [Define to 1 if the system has the type `__int128'.])
+fi])# PGAC_TYPE_64BIT_INT
+
# PGAC_C_FUNCNAME_SUPPORT
# ---
diff --git a/configure b/configure
index fa271fe..1cd964d 100755
--- a/configure
+++ b/configure
@@ -13773,6 +13773,74 @@ _ACEOF
fi
+{ $as_echo $as_me:${as_lineno-$LINENO}: checking for __int128 5
+$as_echo_n checking for __int128... 6; }
+if ${pgac_cv_have___int128+:} false; then :
+ $as_echo_n (cached) 6
+else
+ if test $cross_compiling = yes; then :
+ # If cross-compiling, check the size reported by the compiler and
+# trust that the arithmetic works.
+cat confdefs.h - _ACEOF conftest.$ac_ext
+/* end confdefs.h. */
+
+int
+main ()
+{
+static int test_array [1 - 2 * !(sizeof(__int128) == 16)];
+test_array [0] = 0;
+return test_array [0];
+
+ ;
+ return 0;
+}
+_ACEOF
+if ac_fn_c_try_compile $LINENO; then :
+ pgac_cv_have___int128=yes
+else
+ pgac_cv_have___int128=no
+fi
+rm -f core conftest.err conftest.$ac_objext conftest.$ac_ext
+else
+ cat confdefs.h - _ACEOF conftest.$ac_ext
+/* end confdefs.h. */
+
+__int128 a = 2001;
+__int128 b = 4005;
+
+int does_int128_work()
+{
+ __int128 c,d;
+
+ c = a * b;
+ d = (c + b) / b;
+ if (d != a+1)
+return 0;
+ return 1;
+}
+main() {
+ exit(! does_int128_work());
+}
+_ACEOF
+if ac_fn_c_try_run $LINENO; then :
+ pgac_cv_have___int128=yes
+else
+ pgac_cv_have___int128=no
+fi
+rm -f core *.core core.conftest.* gmon.out bb.out conftest$ac_exeext \
+ conftest.$ac_objext conftest.beam conftest.$ac_ext
+fi
+
+fi
+{ $as_echo $as_me:${as_lineno-$LINENO}: result: $pgac_cv_have___int128 5
+$as_echo $pgac_cv_have___int128 6; }
+
+if test x$pgac_cv_have___int128 = xyes ; then
+
+$as_echo #define HAVE___INT128 1 confdefs.h
+
+fi
+
# We also check for sig_atomic_t, which *should* be defined per ANSI
# C, but is missing on some old platforms.
ac_fn_c_check_type $LINENO sig_atomic_t ac_cv_type_sig_atomic_t #include signal.h
diff --git a/configure.in b/configure.in
index e6a49d1..2ef7870 100644
--- a/configure.in
+++ b/configure.in
@@ -1761,6 +1761,8 @@ AC_DEFINE_UNQUOTED(MAXIMUM_ALIGNOF, $MAX_ALIGNOF, [Define as the maximum alignme
AC_CHECK_TYPES([int8, uint8, int64, uint64], [], [],
[#include stdio.h])
+PGAC_HAVE___INT128
+
# We also check for sig_atomic_t, which *should* be defined per ANSI
# C, but is missing on some old platforms.
AC_CHECK_TYPES(sig_atomic_t, [], [], [#include signal.h])
diff --git a/src/backend/utils/adt/numeric.c b/src/backend/utils/adt/numeric.c
index 715917b..042a94e 100644
--- a/src/backend/utils/adt/numeric.c
+++ b/src/backend/utils/adt/numeric.c
@@ -402,7 +402,10 @@ static void apply_typmod(NumericVar *var, int32 typmod);
static int32 numericvar_to_int4(NumericVar *var);
static bool numericvar_to_int8(NumericVar *var, int64 *result);
-static void int8_to_numericvar(int64 val, NumericVar *var);
+static void int64_to_numericvar(int64 val, NumericVar *var);
+#ifdef HAVE_INT128
+static void int128_to_numericvar(int128 val, NumericVar *var);
+#endif
static double numeric_to_double_no_overflow(Numeric num);
static double numericvar_to_double_no_overflow(NumericVar *var);
@@ -1414,7 +1417,7 @@ width_bucket_numeric(PG_FUNCTION_ARGS)
init_var(count_var);
/* Convert 'count' to a numeric, for ease of use later */
- int8_to_numericvar((int64) count, count_var);
+ int64_to_numericvar((int64) count, count_var);
switch
Re: [HACKERS] File based Incremental backup v8
On Tue, Mar 10, 2015 at 1:50 AM, Robert Haas robertmh...@gmail.com wrote: I think there's absolutely no point in spending more time on this for 9.5. At least 4 committers have looked at it and none of them are convinced by the current design; feedback from almost half a year ago hasn't been incorporated; obviously-needed parts of the patch (pg_restorebackup) are missing weeks after the last CF deadline. Let's mark this Rejected in the CF app and move on. Agreed. I lost a bit interest in this patch lately, but if all the necessary parts of the patch were not posted before the CF deadline that's not something we should consider for integration at this point. Let's give it a couple of months of fresh air and, Gabriele, I am sure you will be able to come back with something far more advanced for the first CF of 9.6. -- Michael -- 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] sepgsql and materialized views
Kohei KaiGai wrote: Unfortunately, I could not get consensus of design on selinux policy side. Even though my opinion is to add individual security class for materialized view to implement refresh permission, other people has different opinion. So, I don't want it shall be a blocker of v9.3 to avoid waste of time. Also, I'll remind selinux community on this issue again, and tries to handle in another way from what I proposed before. Did we get this fixed? 2013/7/5 Tom Lane t...@sss.pgh.pa.us: Noah Misch n...@leadboat.com writes: On Fri, Feb 08, 2013 at 02:51:40PM +0100, Kohei KaiGai wrote: I'll have a discussion about new materialized_view object class on selinux list soon, then I'll submit a patch towards contrib/sepgsql according to the consensus here. Has this progressed? Should we consider this a 9.3 release blocker? sepgsql already has a red box warning about its limitations, so adding the limitation that materialized views are unrestricted wouldn't be out of the question. Definitely -1 for considering it a release blocker. If KaiGai-san can come up with a fix before we otherwise would release 9.3, that's great, but there's no way that sepgsql has a large enough user community to justify letting it determine the release schedule. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Question about lazy_space_alloc() / linux over-commit
On 2015-03-09 17:12:22 -0500, Jim Nasby wrote: That will be significantly more code than a simple repalloc, but as long as people are OK with that I can go that route. I still would like to see some actual evidence of need for change before we invest more time/code here. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation of bt_page_items()'s ctid field
On Tue, Dec 30, 2014 at 8:59 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: On 12/30/2014 04:08 AM, Peter Geoghegan wrote: Attached documentation patch describes the purpose of bt_page_items()'s ctid field. This has come up enough times in disaster recovery or testing scenarios that I feel it's worth drawing particular attention to. How much detail on the b-tree internals do we want to put in the pageinspect documentation? I can see that being useful, but should we also explain e.g. that the first item on each (non-rightmost) page is the high key? How do I know if I am looking at a non-rightmost page? Thanks, Jeff
Re: [HACKERS] Documentation of bt_page_items()'s ctid field
On Mon, Mar 9, 2015 at 3:51 PM, Jeff Janes jeff.ja...@gmail.com wrote: How do I know if I am looking at a non-rightmost page? It has a right-link (that's the easiest way to tell). It will (as the docpatch points out) also necessarily have a high key item. We know that we have to move right if the high key doesn't bound the value we expected to find on the page (our scankey item - what the index scan is searching for). So the high key goes with having a rightlink, and in general we detect that we're looking at a non-rightmost page based on the presence of a right-link. The confusing thing is that some pages have a high-key *and* a minus infinity item. The latter has data that is undefined (cannot be interpreted as a value of any type). It is still logically a real item, that represents minus infinity for the purposes of binary searches. But the high key is not a real item, even though its data *is* a well-defined value of the underlying IndexTuple type or types. Typically, the high key data matches the first non-highkey real item on the next/right page...not including the minus infinity item, if any. Is that any clearer? Basically, high key items are relevant to all non-rightmost pages on all levels. OTOH, minus infinity items are only relevant to internal (non-leaf) pages (and a pre-first-split root page is considered a leaf page for this purpose). Internal, non-rightmost pages (internal pages are typically only a small minority of the total) have both. -- Peter Geoghegan -- 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] Rethinking the parameter access hooks for plpgsql's benefit
On Mon, Mar 9, 2015 at 4:03 PM, Andres Freund and...@2ndquadrant.com wrote: FWIW, I think you actually don't have much reason to complain. This work has probably gotten more attention in total than any other recent patch. Certainly, by far, more than any other in the 9.5 cycle. That has to be true, because the patch has been around in various forms for so long. So far I've not seen a single version that could be considered 'ready for committer'. Even if there's points to be resolved you can make one (presumably yours) solution ready. Of course there isn't, since (for example, hint hint) the logical decoding stuff isn't fully resolved (there is really only one other issue with unique index inference). Those issues are the only two real impediments to at least committing ON CONFLICT IGNORE that I'm aware of, and the former is by far the biggest. My frustration is down to not having anything I can do without feedback on these issues...I've run out of things to do without feedback entirely. It's not for me to decide whether or not I'm justified in complaining about that. I don't think it matters much either way, but as a matter of fact I am feeling very concerned about it. It would be very unfortunate if the UPSERT patch missed the 9.5 release, and for reasons that don't have much to do with me in particular. -- Peter Geoghegan -- 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 functions
On 03/08/2015 08:14 PM, Dmitry Voronin wrote: What do you think about it? Nice to see you back working on the patch. For reviewers: the previous discussion and review of the patch can be found at http://www.postgresql.org/message-id/53a88911.6060...@proxel.se. -- Andreas Karlsson -- 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] BRIN page type identifier
Heikki Linnakangas wrote:
The special area in a BRIN page looks like this:
/* special space on all BRIN pages stores a type identifier */
#define BRIN_PAGETYPE_META 0xF091
#define BRIN_PAGETYPE_REVMAP0xF092
#define BRIN_PAGETYPE_REGULAR 0xF093
...
typedef struct BrinSpecialSpace
{
uint16 flags;
uint16 type;
} BrinSpecialSpace;
I believe this is supposed to follow the usual convention that the last two
bytes of a page can be used to identify the page type. SP-GiST uses 0xFF82,
while GIN uses values 0x00XX.
However, because the special size is MAXALIGNed, the 'type' field are not
the last 2 bytes on the page, as intended. I'd suggest just adding char
padding[6] in BrinSpecialSpace, before 'flags'. That'll waste 4 bytes on
32-bit systems, but that seems acceptable.
Ouch. You're right. I don't understand why you suggest to use 6 bytes,
though -- that would make the struct size be 10 bytes, which maxaligns
to 16, and so we're back where we started. Using 4 bytes does the
trick.
I wonder if this is permissible and whether it will do the right thing
on 32-bit systems:
/*
* Special area of BRIN pages.
*
* We add some padding bytes to ensure that 'type' ends up in the last two
* bytes of the page, for consumption by pg_filedump and similar utilities.
* (Special space is MAXALIGN'ed).
*/
typedef struct BrinSpecialSpace
{
charpadding[MAXALIGN(1) - 2 * sizeof(uint16)];
uint16 flags;
uint16 type;
} BrinSpecialSpace;
It's a bit ugly, but it seems to work for me on x86-64 ...
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Object files generated by ecpg test suite not ignored on Windows
On Tue, Mar 10, 2015 at 2:50 AM, Michael Meskes mes...@postgresql.org wrote: On 09.03.2015 16:58, Tom Lane wrote: Andrew Dunstan and...@dunslane.net writes: The MSVC build creates project directories which contain all the .obj files etc. The file locations for intermediate artefacts are quite different from the way a Unix build works. There is an ignore rule for these directories, which covers the .obj files there. But ecpg files are generated like in Unix builds. Ah. That's lots more plausible than no one's ever done any development on Windows. I can believe that not so many people have run the ecpg tests there. Completely agreed. Since we have a global ignore rule for .o files it makes plenty of sense to have one for .obj files also. Certainly better than have one rule for each ecpg test case. Agreed. So we should revert the previous patch ... Done. Thanks for the fix. I did it as a global rule first, until noticing that each regression item was listed in their own separate .gitignore.. -- Michael -- 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 : Allow toast tables to be moved to a different tablespace
On 03/03/2015 04:14 PM, Julien Tachoires wrote: Sorry for the delay, I missed this thread. Here is a new version of this patch considering Andreas' comments. Please also add it to the next open commitfest so we do not lose the patch. -- Andreas Karlsson -- 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] Documentation of bt_page_items()'s ctid field
On Mon, Mar 9, 2015 at 4:06 PM, Peter Geoghegan p...@heroku.com wrote: On Mon, Mar 9, 2015 at 3:51 PM, Jeff Janes jeff.ja...@gmail.com wrote: How do I know if I am looking at a non-rightmost page? It has a right-link (that's the easiest way to tell). Meaning that btpo_next is not zero? Should we say that in the patch in so many words? I think it will be hard to explain the page_items more without also explaining the page_stats more. It will (as the docpatch points out) also necessarily have a high key item. We know that we have to move right if the high key doesn't bound the value we expected to find on the page (our scankey item - what the index scan is searching for). So the high key goes with having a rightlink, and in general we detect that we're looking at a non-rightmost page based on the presence of a right-link. So if I understand this correctly, if there is a high key it is itemoffset 1, but to know whether itemoffset 1 is a high key I first have to look at btpo_next. And if there is a minus infinity, it will either be itemoffset 2 or 1, depending on whether there is a high key or not. Thanks, Jeff
Re: [HACKERS] InvokeObjectPostAlterHook() vs. CommandCounterIncrement()
Robert Haas wrote: On Sun, Jul 21, 2013 at 4:44 AM, Ants Aasma ants.aa...@eesti.ee wrote: On Jul 21, 2013 4:06 AM, Noah Misch n...@leadboat.com wrote: If these hooks will need to apply to a larger operation, I think that mandates a different means to reliably expose the before/after object states. I haven't checked the code to see how it would fit the API, but what about taking a snapshot before altering and passing this to the hook. Would there be other issues besides performance? If the snapshot is taken only when there is a hook present then the performance can be fixed later. I had the idea of finding a way to pass either the old tuple, or perhaps just the TID of the old tuple. Not sure if passing a snapshot is better. It seems this issue was forgotten. Any takers? -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training Services -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Documentation of bt_page_items()'s ctid field
On Mon, Mar 9, 2015 at 5:18 PM, Jeff Janes jeff.ja...@gmail.com wrote:
It has a right-link (that's the easiest way to tell).
Meaning that btpo_next is not zero? Should we say that in the patch in so
many words? I think it will be hard to explain the page_items more without
also explaining the page_stats more.
Yes. And my wording above was sloppy: By definition, a non-rightmost
page is a page that has a rightlink (it will also have a highkey, but
that's certainly not how we distinguish them).
Attached patch adds a definition of non-rightmost in parenthesis,
which I think helps.
So if I understand this correctly, if there is a high key it is itemoffset
1, but to know whether itemoffset 1 is a high key I first have to look at
btpo_next.
Yes. The logic for caring about minus infinity items within internal
pages is hardcoded into _bt_compare(), so it's kind of a low-level
detail, even by these standards...but it comes up with pageinspect,
since they consume an IndexTuple. Confusion ensues.
And if there is a minus infinity, it will either be itemoffset 2 or 1,
depending on whether there is a high key or not.
That's correct. So data can be (has data), (has no data), (has
data), (has data) on a given page, which could freak users out.
--
Peter Geoghegan
diff --git a/doc/src/sgml/pageinspect.sgml b/doc/src/sgml/pageinspect.sgml
index 6f51dc6..502903c 100644
--- a/doc/src/sgml/pageinspect.sgml
+++ b/doc/src/sgml/pageinspect.sgml
@@ -192,6 +192,17 @@ test=# SELECT * FROM bt_page_items('pg_cast_oid_index', 1);
7 | (0,7) | 12 | f | f| 29 27 00 00
8 | (0,8) | 12 | f | f| 2a 27 00 00
/screen
+ In a B-tree leaf page, structfieldctid/ points to a heap
+ tuple. In an internal page, it points to another page in the
+ index itself, and the offset number part (the second number) of
+ the structfieldctid/ field is ignored. Note that the first
+ item on any non-rightmost page (any page lacking a
+ structfieldbtpo_next/ field) is the page quotehigh
+ key/quote, meaning its structfielddata/ serves as an upper
+ bound on all items appearing on the page. Also, on non-leaf
+ pages, the first data item (the first item that is not a high
+ key) is a quoteminus infinity/quote item, implying that its
+ structfielddata/ field lacks any value.
/para
/listitem
/varlistentry
--
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] EvalPlanQual behaves oddly for FDW queries involving system columns
On 2015/03/09 16:02, Ashutosh Bapat wrote: I tried reproducing the issue with the steps summarised. Thanks for the review! Here's my setup Sorry, my explanation was not enough, but such was not my intention. I'll re-summarize the steps below: [Create a test environment] $ createdb mydatabase $ psql mydatabase mydatabase=# create table mytable (a int); $ psql postgres postgres=# create extension postgres_fdw; postgres=# create server loopback foreign data wrapper postgres_fdw options (dbname 'mydatabase'); postgres=# create user mapping for current_user server loopback; postgres=# create foreign table ftable (a int) server loopback options (table_name 'mytable'); postgres=# insert into ftable values (1); postgres=# create table ltable (a int, b int); postgres=# insert into ltable values (1, 1); [Run concurrent transactions] In terminal1: $ psql postgres postgres=# begin; BEGIN postgres=# update ltable set b = b * 2; UPDATE 1 In terminal2: $ psql postgres postgres=# select tableoid, ctid, * from ftable; tableoid | ctid | a --+---+--- 16394 | (0,1) | 1 (1 row) postgres=# select f.tableoid, f.ctid, f.* from ftable f, ltable l where f.a = l.a for update; In terminal1: postgres=# commit; COMMIT In terminal2:(When committing the UPDATE query in terminal1, psql in terminal2 will display the result for the SELECT-FOR-UPDATE query as shown below.) tableoid | ctid | a --++--- 0 | (4294967295,0) | 1 (1 row) Best regards, Etsuro Fujita -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] TABLESAMPLE patch
On Mon, Mar 9, 2015 at 3:08 PM, Petr Jelinek p...@2ndquadrant.com wrote: On 09/03/15 04:51, Amit Kapila wrote: On Sat, Mar 7, 2015 at 10:37 PM, Petr Jelinek p...@2ndquadrant.com Double checking for tuple visibility is the only downside I can think of. That will happen if we use heapgetpage and the way currently code is written in patch, however we can easily avoid double checking if we don't call heapgetpage and rather do the required work at caller's place. What's the point of pagemode then if the caller code does the visibility checks still one by one on each call. I thought one of the points of pagemode was to do this in one step (and one buffer lock). You only need one buffer lock for doing at caller's location as well something like we do in acquire_sample_rows(). And if the caller will try to do it in one step and cache the visibility info then we'll end up with pretty much same structure as rs_vistuples - there isn't saner way to cache this info other than ordered vector of tuple offsets, unless we assume that most pages have close to MaxOffsetNumber of tuples which they don't, so why not just use the heapgetpage directly and do the binary search over rs_vistuples. The downside of doing it via heapgetpage is that it will do visibility test for tuples which we might not even need (I think we should do visibility test for tuples retrurned by tsmnexttuple). With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] sepgsql and materialized views
Stephen Frost sfr...@snowman.net wrote: Will look into it and try to provide an update soon. Any further thoughts or suggestions would be appreciated. My recollection is that there were two ways that were being considered, and I posted a patch for each of them, but the security community couldn't reach a consensus, so neither was pushed. Of course someone might come up with one or more other ideas, but barring that it might just be a matter of finding the right patch in the archives and curing any bit rot. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel Seq Scan
On Tue, Mar 10, 2015 at 6:50 AM, Haribabu Kommi kommi.harib...@gmail.com wrote: On Tue, Mar 10, 2015 at 1:38 AM, Amit Kapila amit.kapil...@gmail.com wrote: Assuming previous patch is in right direction, I have enabled join support for the patch and done some minor cleanup of patch which leads to attached new version. Is this patch handles the cases where the re-scan starts without finishing the earlier scan? Do you mean to say cases like ANTI, SEMI Join (in nodeNestLoop.c) where we scan the next outer tuple and rescan inner table without completing the previous scan of inner table? I have currently modelled it based on existing rescan for seqscan (ExecReScanSeqScan()) which means it will begin the scan again. Basically if the workers are already started/initialized by previous scan, then re-initialize them (refer function ExecReScanFunnel() in patch). Can you elaborate more if you think current handling is not sufficient for any case? With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
Re: [HACKERS] BRIN page type identifier
[ paths crossed in mail ] I wrote: This way, accesses to flags require no source code changes. You still need a macro to map type onto the last element of the vector, but there's probably about one reference to type in the source code so it shouldn't be too painful. Ah, nevermind, I see you already did the work to do it the other way. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] sepgsql and materialized views
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Kohei KaiGai wrote: Unfortunately, I could not get consensus of design on selinux policy side. Even though my opinion is to add individual security class for materialized view to implement refresh permission, other people has different opinion. So, I don't want it shall be a blocker of v9.3 to avoid waste of time. Also, I'll remind selinux community on this issue again, and tries to handle in another way from what I proposed before. Did we get this fixed? I don't think so, but it's something I'm interested in and have an envrionment where I can work on it. Will look into it and try to provide an update soon. Any further thoughts or suggestions would be appreciated. Ah, yes, the issue has been kept unhandled. May I remind selinux folks again, to add db_materialized_view class? Or, Stephan, do you have idea to apply equivalent checks on refresh operation? Thanks, -- NEC OSS Promotion Center / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Parallel Seq Scan
On Tue, Mar 10, 2015 at 3:09 PM, Amit Kapila amit.kapil...@gmail.com wrote: On Tue, Mar 10, 2015 at 6:50 AM, Haribabu Kommi kommi.harib...@gmail.com wrote: On Tue, Mar 10, 2015 at 1:38 AM, Amit Kapila amit.kapil...@gmail.com wrote: Assuming previous patch is in right direction, I have enabled join support for the patch and done some minor cleanup of patch which leads to attached new version. Is this patch handles the cases where the re-scan starts without finishing the earlier scan? Do you mean to say cases like ANTI, SEMI Join (in nodeNestLoop.c) where we scan the next outer tuple and rescan inner table without completing the previous scan of inner table? Yes. I have currently modelled it based on existing rescan for seqscan (ExecReScanSeqScan()) which means it will begin the scan again. Basically if the workers are already started/initialized by previous scan, then re-initialize them (refer function ExecReScanFunnel() in patch). Can you elaborate more if you think current handling is not sufficient for any case? From ExecReScanFunnel function it seems that the re-scan waits till all the workers has to be finished to start again the next scan. Are the workers will stop the current ongoing task? otherwise this may decrease the performance instead of improving as i feel. I am not sure if it already handled or not, when a worker is waiting to pass the results, whereas the backend is trying to start the re-scan? Regards, Hari Babu Fujitsu Australia -- 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] Parallel Seq Scan
On Tue, Mar 10, 2015 at 1:38 AM, Amit Kapila amit.kapil...@gmail.com wrote: Assuming previous patch is in right direction, I have enabled join support for the patch and done some minor cleanup of patch which leads to attached new version. Is this patch handles the cases where the re-scan starts without finishing the earlier scan? Regards, Hari Babu Fujitsu Australia -- 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: knowing detail of config files via SQL
Sawada,
* Sawada Masahiko (sawada.m...@gmail.com) wrote:
Thank you for your review!
Attached file is the latest version (without document patch. I making it now.)
As per discussion, there is no change regarding of super user permission.
Ok. Here's another review.
diff --git a/src/backend/catalog/system_views.sql
b/src/backend/catalog/system_views.sql
index 5e69e2b..94ee229 100644
--- a/src/backend/catalog/system_views.sql
+++ b/src/backend/catalog/system_views.sql
@@ -414,6 +414,11 @@ CREATE RULE pg_settings_n AS
GRANT SELECT, UPDATE ON pg_settings TO PUBLIC;
+CREATE VIEW pg_file_settings AS
+ SELECT * FROM pg_show_all_file_settings() AS A;
+
+REVOKE ALL on pg_file_settings FROM public;
+
This suffers from a lack of pg_dump support, presuming that the
superuser is entitled to change the permissions on this function. As it
turns out though, you're in luck in that regard since I'm working on
fixing that for a mostly unrelated patch. Any feedback you have on what
I'm doing to pg_dump here:
http://www.postgresql.org/message-id/20150307213935.go29...@tamriel.snowman.net
Would certainly be appreciated.
diff --git a/src/backend/utils/misc/guc-file.l
b/src/backend/utils/misc/guc-file.l
[...]
+ * Calculate size of guc_array and allocate it. From the secound time
to allcate memory,
+ * we should free old allocated memory.
+ */
+ guc_array_size = num_guc_file_variables * sizeof(struct
ConfigFileVariable);
+ if (!guc_file_variables)
+ {
+ /* For the first call */
+ guc_file_variables = (ConfigFileVariable *) guc_malloc(FATAL,
guc_array_size);
+ }
+ else
+ {
+ guc_array = guc_file_variables;
+ for (item = head; item; item = item-next, guc_array++)
+ {
+ free(guc_array-name);
+ free(guc_array-value);
+ free(guc_array-filename);
It's a minor nit-pick, as the below loop should handle anything we care
about, but I'd go ahead and reset guc_array-sourceline to be -1 or
something too, just to show that everything's being handled here with
regard to cleanup. Or perhaps just add a comment saying that the
sourceline for all currently valid entries will be updated.
+ guc_file_variables = (ConfigFileVariable *) guc_realloc(FATAL,
guc_file_variables, guc_array_size);
+ }
Nasby made a comment, I believe, that we might actually be able to use
memory contexts here, which would certainly be much nicer as then you'd
be able to just throw away the whole context and create a new one. Have
you looked at that approach at all? Offhand, I'm not sure if it'd work
or not (I have my doubts..) but it seems worthwhile to consider.
Otherwise, it seems like this would address my previous concerns that
this would end up leaking memory, and even if it's a bit slower than one
might hope, it's not an operation we should be doing very frequently.
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
[...]
/*
+ * Return the total number of GUC File variables
+ */
+int
+GetNumConfigFileOptions(void)
+{
+ return num_guc_file_variables;
+}
What's the point of this function..? I'm not convinced it's the best
idea, but it certainly looks like the function and the variable are only
used from this file anyway?
+ if (call_cntr max_calls)
+ {
+ char *values[NUM_PG_FILE_SETTINGS_ATTS];
+ HeapTuple tuple;
+ Datum result;
+ ConfigFileVariable conf;
+ charbuffer[256];
Isn't that buffer a bit.. excessive in size?
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index d3100d1..ebb96cc 100644
--- a/src/include/utils/guc.h
+++ b/src/include/utils/guc.h
@@ -133,6 +133,14 @@ typedef struct ConfigVariable
struct ConfigVariable *next;
} ConfigVariable;
+typedef struct ConfigFileVariable
+{
+ char*name;
+ char*value;
+ char*filename;
+ int sourceline;
+} ConfigFileVariable;
+
[...]
+extern int GetNumConfigFileOptions(void);
These aren't actually used anywhere else, are they? Not sure that
there's any need to expose them beyond guc.c when that's the only place
they're used.
Thanks!
Stephen
signature.asc
Description: Digital signature
Re: [HACKERS] BRIN page type identifier
I wrote:
You could try something like
typedef struct BrinSpecialSpace
{
uint16 vector[MAXALIGN(1) / sizeof(uint16)];
} BrinSpecialSpace;
and then some access macros to use the last and next-to-last
elements of that array.
On second thought, consider
typedef struct BrinSpecialSpace
{
uint16 flags;
uint16 vector[MAXALIGN(1) / sizeof(uint16) - 1];
} BrinSpecialSpace;
This way, accesses to flags require no source code changes.
You still need a macro to map type onto the last element of
the vector, but there's probably about one reference to type
in the source code so it shouldn't be too painful.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] New functions
On Tue, Mar 10, 2015 at 7:24 AM, Andreas Karlsson andr...@proxel.se wrote: On 03/08/2015 08:14 PM, Dmitry Voronin wrote: What do you think about it? Nice to see you back working on the patch. For reviewers: the previous discussion and review of the patch can be found at http://www.postgresql.org/message-id/53a88911.6060...@proxel.se. Worth noticing as well, this has been added to CF 2015-06: https://commitfest.postgresql.org/5/192/ (I am going to remove the duplicate entry 191..) -- Michael -- 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] sepgsql and materialized views
* Alvaro Herrera (alvhe...@2ndquadrant.com) wrote: Kohei KaiGai wrote: Unfortunately, I could not get consensus of design on selinux policy side. Even though my opinion is to add individual security class for materialized view to implement refresh permission, other people has different opinion. So, I don't want it shall be a blocker of v9.3 to avoid waste of time. Also, I'll remind selinux community on this issue again, and tries to handle in another way from what I proposed before. Did we get this fixed? I don't think so, but it's something I'm interested in and have an envrionment where I can work on it. Will look into it and try to provide an update soon. Any further thoughts or suggestions would be appreciated. Thanks! Stephen signature.asc Description: Digital signature
Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API)
The attached patch integrates a suggestion from Ashutosh Bapat. It allows to track set of relations involved in a join, but replaced by foreign-/custom-scan. It enables to make correct EXPLAIN output, if FDW/CSP driver makes human readable symbols using deparse_expression() or others. Differences from v7 is identical with what I posted on the join push-down support thread. Thanks, -- NEC OSS Promotion Center / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com -Original Message- From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Kouhei Kaigai Sent: Wednesday, March 04, 2015 11:42 AM To: Shigeru Hanada Cc: Robert Haas; Tom Lane; pgsql-hackers@postgreSQL.org Subject: Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API) Obviously FDW can add multiple paths at a time, like GetForeignPaths, so IMO it should be renamed to GetForeignJoinPaths, with plural form. In addition to that, new member of RelOptInfo, fdw_handler, should be initialized explicitly in build_simple_rel. Please see attached a patch for these changes. Thanks for your checks. Yep, the name of FDW handler should be ...Paths(), instead of Path(). The attached one integrates Hanada-san's updates. -- NEC OSS Promotion Center / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com -Original Message- From: Shigeru Hanada [mailto:shigeru.han...@gmail.com] Sent: Tuesday, March 03, 2015 9:26 PM To: Kaigai Kouhei(海外 浩平) Cc: Robert Haas; Tom Lane; pgsql-hackers@postgreSQL.org Subject: Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API) Kaigai-san, The v6 patch was cleanly applied on master branch. I'll rebase my patch onto it, but before that I have a comment about name of the new FDW API handler GetForeignJoinPath. Obviously FDW can add multiple paths at a time, like GetForeignPaths, so IMO it should be renamed to GetForeignJoinPaths, with plural form. In addition to that, new member of RelOptInfo, fdw_handler, should be initialized explicitly in build_simple_rel. Please see attached a patch for these changes. I'll review the v6 path afterwards. 2015-03-03 20:20 GMT+09:00 Kouhei Kaigai kai...@ak.jp.nec.com: Sorry, I misoperated on patch creation. Attached one is the correct version. -- NEC OSS Promotion Center / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com -Original Message- From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers-ow...@postgresql.org] On Behalf Of Kouhei Kaigai Sent: Tuesday, March 03, 2015 6:31 PM To: Kaigai Kouhei(海外 浩平); Robert Haas Cc: Tom Lane; pgsql-hackers@postgreSQL.org; Shigeru Hanada Subject: Re: Custom/Foreign-Join-APIs (Re: [HACKERS] [v9.5] Custom Plan API) The attached version of custom/foreign-join interface patch fixes up the problem reported on the join-pushdown support thread. The previous version referenced *_ps_tlist on setrefs.c, to check whether the Custom/ForeignScan node is associated with a particular base relation, or not. This logic considered above nodes performs base relation scan, if *_ps_tlist is valid. However, it was incorrect in case when underlying pseudo-scan relation has empty targetlist. Instead of the previous logic, it shall be revised to check scanrelid itself. If zero, it means Custom/ForeignScan node is not associated with a particular base relation, thus, its slot descriptor for scan shall be constructed based on *_ps_tlist. Also, I noticed a potential problem if CSP/FDW driver want to displays expression nodes using deparse_expression() but varnode within this expression does not appear in the *_ps_tlist. For example, a remote query below shall return rows with two columns. SELECT atext, btext FROM tbl_a, tbl_b WHERE aid = bid; Thus, ForeignScan will perform like as a scan on relation with two columns, and FDW driver will set two TargetEntry on the fdw_ps_tlist. If FDW is designed to keep the join condition (aid = bid) using expression node form, it is expected to be saved on custom/fdw_expr variable, then setrefs.c rewrites the varnode according to *_ps_tlist. It means, we also have to add *_ps_tlist both of aid and bid to avoid failure on variable lookup. However, these additional entries changes the definition of the slot descriptor. So, I adjusted ExecInitForeignScan and ExecInitCustomScan to use ExecCleanTypeFromTL(), not ExecTypeFromTL(), when it construct the slot descriptor based on the *_ps_tlist. It expects CSP/FDW drivers to add target-entries with resjunk=true, if it wants to have additional entries for variable lookups on EXPLAIN command. Fortunately or unfortunately, postgres_fdw keeps its remote query in cstring form, so it does not need to add junk entries on the fdw_ps_tlist. Thanks, --
Re: [HACKERS] Join push-down support for foreign tables
Thanks for finding out what we oversight. Here is still a problem because the new 'relids' field is not updated on setrefs.c (scanrelid is incremented by rtoffset here). It is easy to shift the bitmapset by rtoffset, however, I also would like to see another approach. I just made it work for explain, but other parts still need work. Sorry about that. If we follow INDEX_VAR, we should be able to get there. I tried to modify your patch a bit as below: * add adjustment of bitmap fields on setrefs.c * add support on outfuncs.c and copyfuncs.c. * add bms_shift_members() in bitmapset.c I think it is a reasonable enhancement, however, it is not tested with real-life code, like postgres_fdw. Hanada-san, could you add a feature to print name of foreign-tables which are involved in remote queries, on postgresExplainForeignScan()? ForeignScan-fdw_relids bitmap and ExplainState-rtable_names will tell you the joined foreign tables replaced by the (pseudo) foreign-scan. Soon, I'll update the interface patch also. My idea adds 'List *fdw_sub_paths' field in ForeignPath to inform planner underlying foreign-scan paths (with scanrelid 0). The create_foreignscan_plan() will call create_plan_recurse() to construct plan nodes based on the path nodes being attached. Even though these foreign-scan nodes are not actually executed, setrefs.c can update scanrelid in usual way and ExplainPreScanNode does not need to take exceptional handling on Foreign/CustomScan nodes. In addition, it allows to keep information about underlying foreign table scan, even if planner will need some other information in the future version (not only relids). How about your thought? I am not sure about keeping planner nodes, which are not turned into execution nodes. There's no precedence for that in current code. It could be risky. Indeed, it is a fair enough opinion. At this moment, no other code makes plan node but shall not be executed actually. Please forget above idea. Thanks, -- NEC OSS Promotion Center / PG-Strom Project KaiGai Kohei kai...@ak.jp.nec.com add_fdw_custom_relids.patch Description: add_fdw_custom_relids.patch -- 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] BRIN page type identifier
Tom Lane wrote:
Alvaro Herrera alvhe...@2ndquadrant.com writes:
typedef struct BrinSpecialSpace
{
charpadding[MAXALIGN(1) - 2 * sizeof(uint16)];
uint16 flags;
uint16 type;
} BrinSpecialSpace;
I should expect that to fail altogether on 32-bit machines, because
the declared array size will be zero.
Hah, of course.
You could try something like
typedef struct BrinSpecialSpace
{
uint16 vector[MAXALIGN(1) / sizeof(uint16)];
} BrinSpecialSpace;
and then some access macros to use the last and next-to-last
elements of that array.
Ah, thanks, that works fine on x86-64. Here's a patch I intend to push
tomorrow.
Heikki suggested that the comment above GinPageOpaqueData be moved to
some better place, but I couldn't find any such. If there are other
ideas, I'm all ears.
--
Álvaro Herrerahttp://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training Services
commit 8532bfeffdaeffa1d49c390ba6b7b0b46a20afb7
Author: Alvaro Herrera alvhe...@alvh.no-ip.org
Date: Mon Mar 9 23:18:16 2015 -0300
Move BRIN page type to page's last two bytes
... which is the usual convention, so that pg_filedump and similar
utilities can tell apart pages of different AMs.
Per note from Heikki at
http://www.postgresql.org/message-id/546a16ef.9070...@vmware.com
diff --git a/contrib/pageinspect/brinfuncs.c b/contrib/pageinspect/brinfuncs.c
index 630dda4..1b15a7b 100644
--- a/contrib/pageinspect/brinfuncs.c
+++ b/contrib/pageinspect/brinfuncs.c
@@ -58,12 +58,9 @@ brin_page_type(PG_FUNCTION_ARGS)
{
bytea *raw_page = PG_GETARG_BYTEA_P(0);
Page page = VARDATA(raw_page);
- BrinSpecialSpace *special;
char *type;
- special = (BrinSpecialSpace *) PageGetSpecialPointer(page);
-
- switch (special-type)
+ switch (BrinPageType(page))
{
case BRIN_PAGETYPE_META:
type = meta;
@@ -75,7 +72,7 @@ brin_page_type(PG_FUNCTION_ARGS)
type = regular;
break;
default:
- type = psprintf(unknown (%02x), special-type);
+ type = psprintf(unknown (%02x), BrinPageType(page));
break;
}
@@ -91,7 +88,6 @@ verify_brin_page(bytea *raw_page, uint16 type, const char *strtype)
{
Page page;
int raw_page_size;
- BrinSpecialSpace *special;
raw_page_size = VARSIZE(raw_page) - VARHDRSZ;
@@ -104,13 +100,12 @@ verify_brin_page(bytea *raw_page, uint16 type, const char *strtype)
page = VARDATA(raw_page);
/* verify the special space says this page is what we want */
- special = (BrinSpecialSpace *) PageGetSpecialPointer(page);
- if (special-type != type)
+ if (BrinPageType(page) != type)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg(page is not a BRIN page of type \%s\, strtype),
errdetail(Expected special type %08x, got %08x.,
- type, special-type)));
+ type, BrinPageType(page;
return page;
}
diff --git a/src/backend/access/brin/brin_pageops.c b/src/backend/access/brin/brin_pageops.c
index 4e9392b..acb64bd 100644
--- a/src/backend/access/brin/brin_pageops.c
+++ b/src/backend/access/brin/brin_pageops.c
@@ -53,7 +53,6 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
BrinTuple *oldtup;
Size oldsz;
Buffer newbuf;
- BrinSpecialSpace *special;
bool extended = false;
newsz = MAXALIGN(newsz);
@@ -113,8 +112,6 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
return false;
}
- special = (BrinSpecialSpace *) PageGetSpecialPointer(oldpage);
-
/*
* Great, the old tuple is intact. We can proceed with the update.
*
@@ -124,7 +121,7 @@ brin_doupdate(Relation idxrel, BlockNumber pagesPerRange,
* caller told us there isn't, if a concurrent update moved another tuple
* elsewhere or replaced a tuple with a smaller one.
*/
- if (((special-flags BRIN_EVACUATE_PAGE) == 0)
+ if (((BrinPageFlags(oldpage) BRIN_EVACUATE_PAGE) == 0)
brin_can_do_samepage_update(oldbuf, origsz, newsz))
{
if (BufferIsValid(newbuf))
@@ -374,12 +371,9 @@ brin_doinsert(Relation idxrel, BlockNumber pagesPerRange,
void
brin_page_init(Page page, uint16 type)
{
- BrinSpecialSpace *special;
-
PageInit(page, BLCKSZ, sizeof(BrinSpecialSpace));
- special = (BrinSpecialSpace *) PageGetSpecialPointer(page);
- special-type = type;
+ BrinPageType(page) = type;
}
/*
@@ -420,7 +414,6 @@ brin_start_evacuating_page(Relation idxRel, Buffer buf)
{
OffsetNumber off;
OffsetNumber maxoff;
- BrinSpecialSpace *special;
Page page;
page = BufferGetPage(buf);
@@ -428,8 +421,6 @@ brin_start_evacuating_page(Relation idxRel, Buffer buf)
if (PageIsNew(page))
return false;
- special = (BrinSpecialSpace *) PageGetSpecialPointer(page);
-
maxoff = PageGetMaxOffsetNumber(page);
for (off = FirstOffsetNumber; off = maxoff; off++)
{
@@ -439,7 +430,7 @@ brin_start_evacuating_page(Relation idxRel, Buffer buf)
if (ItemIdIsUsed(lp))
{
/* prevent other backends from adding more
Re: [HACKERS] Using 128-bit integers for sum, avg and statistics aggregates
On Mon, Mar 9, 2015 at 5:37 PM, Andreas Karlsson andr...@proxel.se wrote: int128-agg-v7.patch I see a spelling error: + * On platforms which support 128-bit integers some aggergates instead use a Other than that, the patch looks pretty good to me. You're generalizing from the example of existing routines for int128_to_numericvar(), and other such code in a fairly mechanical way. The performance improvements are pretty real and tangible. You say: /* * Integer data types use Numeric accumulators to share code and avoid * risk of overflow. For int2 and int4 inputs, Numeric accumulation * is overkill for the N and sum(X) values, but definitely not overkill * for the sum(X*X) value. Hence, we use int2_accum and int4_accum only * for stddev/variance --- there are faster special-purpose accumulator * routines for SUM and AVG of these datatypes. * * Similarily we can, where available, use 128-bit integer accumulators * for sum(X) for int8 and sum(X*X) for int2 and int4, but not sum(X*X) * for int8. */ I think you should talk about the new thing first (just after the extant, first sentence Integer data types use Numeric...). Refer to where 128-bit integers are used and how, and only then the other stuff (exceptions). After that, put the PolyNumAggState struct definition and basic functions. Then int2_accum() and so on. -- Peter Geoghegan -- 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 : REINDEX xxx VERBOSE
On Fri, Mar 6, 2015 at 11:07 AM, Jim Nasby jim.na...@bluetreble.com wrote:
On 3/2/15 10:58 AM, Sawada Masahiko wrote:
On Wed, Feb 25, 2015 at 4:58 PM, Jim Nasby jim.na...@bluetreble.com
wrote:
On 2/24/15 8:28 AM, Sawada Masahiko wrote:
According to the above discussion, VACUUM and REINDEX should have
trailing options. Tom seems (to me) suggesting that SQL-style
(bare word preceded by WITH) options and Jim suggesting '()'
style options? (Anyway VACUUM gets the*third additional* option
sytle, but it is the different discussion from this)
Well, almost everything does a trailing WITH. We need to either stick
with
that for consistency, or add leading () as an option to those WITH
commands.
Does anyone know why those are WITH? Is it ANSI?
As a refresher, current commands are:
VACUUM (ANALYZE, VERBOSE) table1 (col1);
REINDEX INDEX index1 FORCE;
COPY table1 FROM 'file.txt' WITH (FORMAT csv);
CREATE MATERIALIZED VIEW mv1 WITH (storageparam, ...) AS qry WITH DATA;
CREATE EXTENSION ext1 WITH SCHEMA s1 VERSION v1 FROM over;
CREATE ROLE role WITH LOGIN;
GRANT WITH GRANT OPTION;
CREATE VIEW v1 AS qry WITH CASCADED CHECK OPTION;
ALTER DATABASE db1 WITH CONNECTION LIMIT 50;
DECLARE c1 INSENSITIVE SCROLL CURSOR WITH HOLD;
BTW, I'm fine with Tom's bare-word with WITH idea. That seems to be the most
consistent with everything else. Is there a problem with doing that? I know
getting syntax is one of the hard parts of new features, but it seems like
we reached consensus here...
Attached is latest version patch based on Tom's idea as follows.
REINDEX { INDEX | ... } name WITH ( options [, ...] )
We have discussed about this option including FORCE option, but I
think there are not user who want to use both FORCE and VERBOSE option
at same time.
I find that very hard to believe... I would expect a primary use case for
VERBOSE to be I ran REINDEX, but it doesn't seem to have done anything...
what's going on? and that's exactly when you might want to use FORCE.
In currently code, nothing happens even if FORCE option is specified.
This option completely exist for backward compatibility.
But this patch add new syntax including FORCE option for now.
Todo
- tab completion
- reindexdb command
Regards,
---
Sawada Masahiko
diff --git a/doc/src/sgml/ref/reindex.sgml b/doc/src/sgml/ref/reindex.sgml
index 0a4c7d4..a4109aa 100644
--- a/doc/src/sgml/ref/reindex.sgml
+++ b/doc/src/sgml/ref/reindex.sgml
@@ -22,6 +22,12 @@ PostgreSQL documentation
refsynopsisdiv
synopsis
REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } replaceable class=PARAMETERname/replaceable [ FORCE ]
+REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } replaceable class=PARAMETERname/replaceable WITH ( replaceable class=PARAMETERoptions/replaceable [, ...] )
+
+phrasewhere replaceable class=PARAMETERoption/replaceable can be one of:/phrase
+
+FORCE [ replaceable class=PARAMETERboolean/replaceable ]
+VERBOSE [ replaceable class=PARAMETERboolean/replaceable ]
/synopsis
/refsynopsisdiv
@@ -159,6 +165,29 @@ REINDEX { INDEX | TABLE | SCHEMA | DATABASE | SYSTEM } replaceable class=PARAM
/para
/listitem
/varlistentry
+
+ varlistentry
+termliteralVERBOSE/literal/term
+listitem
+ para
+ Prints a progress report as each index is reindexed.
+ /para
+/listitem
+ /varlistentry
+
+ varlistentry
+termreplaceable class=parameterboolean/replaceable/term
+listitem
+ para
+ Specifies whether the selected option should be turned on or off.
+ You can write literalTRUE/literal, literalON/, or
+ literal1/literal to enable the option, and literalFALSE/literal,
+ literalOFF/, or literal0/literal to disable it. The
+ replaceable class=parameterboolean/replaceable value can also
+ be omitted, in which case literalTRUE/literal is assumed.
+ /para
+/listitem
+ /varlistentry
/variablelist
/refsect1
diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index f85ed93..786f173 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -63,6 +63,7 @@
#include utils/inval.h
#include utils/lsyscache.h
#include utils/memutils.h
+#include utils/pg_rusage.h
#include utils/syscache.h
#include utils/tuplesort.h
#include utils/snapmgr.h
@@ -3130,13 +3131,18 @@ IndexGetRelation(Oid indexId, bool missing_ok)
* reindex_index - This routine is used to recreate a single index
*/
void
-reindex_index(Oid indexId, bool skip_constraint_checks, char persistence)
+reindex_index(Oid indexId, bool skip_constraint_checks, char persistence,
+bool verbose)
{
Relation iRel,
heapRelation;
Oid heapId;
IndexInfo *indexInfo;
volatile bool skipped_constraint = false;
+ int elevel = verbose ? INFO : DEBUG2;
+ PGRUsage ru0;
+
+ pg_rusage_init(ru0);
/*
* Open and lock the parent heap relation. ShareLock is sufficient since
@@ -3280,6 +3286,13 @@
Re: [HACKERS] Install shared libs in lib/ and bin/ with MSVC (Was: install libpq.dll in bin directory on Windows / Cygwin)
On Wed, Mar 4, 2015 at 4:13 AM, Michael Paquier wrote:
Those entries are removed as well in the patch.
Please find attached a new version of the patch, fixing a failure for
plperl installation that contains GNUmakefile instead of Makefile.
--
Michael
diff --git a/src/tools/msvc/Install.pm b/src/tools/msvc/Install.pm
index eba9aa0..fd6080f 100644
--- a/src/tools/msvc/Install.pm
+++ b/src/tools/msvc/Install.pm
@@ -91,7 +91,6 @@ sub Install
}
CopySolutionOutput($conf, $target);
- lcopy($target . '/lib/libpq.dll', $target . '/bin/libpq.dll');
my $sample_files = [];
my @top_dir = (src);
@top_dir = (src\\bin, src\\interfaces) if ($insttype eq client);
@@ -108,12 +107,8 @@ sub Install
$target . '/lib/',
$conf\\,
postgres\\postgres.lib,
- libpq\\libpq.lib,
- libecpg\\libecpg.lib,
libpgcommon\\libpgcommon.lib,
- libpgport\\libpgport.lib,
- libpgtypes\\libpgtypes.lib,
- libecpg_compat\\libecpg_compat.lib);
+ libpgport\\libpgport.lib);
CopyContribFiles($config, $target);
CopyIncludeFiles($target);
@@ -236,8 +231,9 @@ sub CopySolutionOutput
while ($sln =~ $rem)
{
my $pf = $1;
- my $dir;
+ my @dirs;
my $ext;
+ my $is_sharedlib = 0;
$sln =~ s/$rem//;
@@ -247,17 +243,42 @@ sub CopySolutionOutput
my $proj = read_file($pf.$vcproj)
|| croak Could not open $pf.$vcproj\n;
+
+ # Check if this project uses a shared library by looking if
+ # SO_MAJOR_VERSION is defined in its Makefile, whose path
+ # can be found using the resource file of this project.
+ if (($vcproj eq 'vcxproj'
+ $proj =~ qr{ResourceCompile\s*Include=([^]+)}) ||
+ ($vcproj eq 'vcproj'
+ $proj =~ qr{File\s*RelativePath=([^\]+)\.rc}))
+ {
+ my $projpath = dirname($1);
+ my $mfname = -e $projpath/GNUmakefile ?
+$projpath/GNUmakefile : $projpath/Makefile;
+ my $mf = read_file($mfname) ||
+croak Could not open $mfname\n;
+
+ if ($mf =~ /^SO_MAJOR_VERSION\s*=\s*(.*)$/mg)
+ {
+$is_sharedlib = 1;
+ }
+ }
+
if ($vcproj eq 'vcproj' $proj =~ qr{ConfigurationType=([^]+)})
{
if ($1 == 1)
{
-$dir = bin;
+@dirs = qw(bin);
$ext = exe;
}
elsif ($1 == 2)
{
-$dir = lib;
+@dirs = qw(lib);
$ext = dll;
+if ($is_sharedlib)
+{
+ push(@dirs, 'bin');
+}
}
else
{
@@ -271,13 +292,17 @@ sub CopySolutionOutput
{
if ($1 eq 'Application')
{
-$dir = bin;
+@dirs = qw(bin);
$ext = exe;
}
elsif ($1 eq 'DynamicLibrary')
{
-$dir = lib;
+@dirs = qw(lib);
$ext = dll;
+if ($is_sharedlib)
+{
+ push(@dirs, 'bin');
+}
}
else# 'StaticLibrary'
{
@@ -290,8 +315,11 @@ sub CopySolutionOutput
{
croak Could not parse $pf.$vcproj\n;
}
- lcopy($conf\\$pf\\$pf.$ext, $target\\$dir\\$pf.$ext)
- || croak Could not copy $pf.$ext\n;
+ foreach my $dir (@dirs)
+ {
+ lcopy($conf\\$pf\\$pf.$ext, $target\\$dir\\$pf.$ext)
+|| croak Could not copy $pf.$ext\n;
+ }
lcopy($conf\\$pf\\$pf.pdb, $target\\symbols\\$pf.pdb)
|| croak Could not copy $pf.pdb\n;
print .;
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] improving speed of make check-world
On Sun, Mar 8, 2015 at 10:46 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
Speaking of which, attached is a patch rewritten in-line with those
comments, simplifying a bit the whole at the same time. Note this
patch changes ecpgcheck as it should be patched, but as ecpgcheck test
is broken even on HEAD, I'll raise a separate thread about that with a
proper fix (for example bowerbird skips this test).
Correction: HEAD is fine, but previous patch was broken because it
tried to use --top-builddir. Also for ecpgcheck it looks that tricking
PATH is needed to avoid dll loading errors related to libecpg.dll and
libecpg_compat.dll. Updated patch is attached.
Bonus track: pg_regress.c is missing the description of --bindir in help().
--
Michael
From 5b729058335cf4a77e22de25bce4c90fa6d686c8 Mon Sep 17 00:00:00 2001
From: Michael Paquier mich...@otacoo.com
Date: Sun, 8 Mar 2015 22:39:10 -0700
Subject: [PATCH] Make vcregress use common installation path for all tests
installcheck is let as-is as it depends on psql being present in PATH.
---
src/tools/msvc/vcregress.pl | 66 -
1 file changed, 42 insertions(+), 24 deletions(-)
diff --git a/src/tools/msvc/vcregress.pl b/src/tools/msvc/vcregress.pl
index bd3dd2c..c7ce5aa 100644
--- a/src/tools/msvc/vcregress.pl
+++ b/src/tools/msvc/vcregress.pl
@@ -16,6 +16,7 @@ my $startdir = getcwd();
chdir ../../.. if (-d ../../../src/tools/msvc);
my $topdir = getcwd();
+my $tmp_installdir = $topdir/tmp_install;
require 'src/tools/msvc/config_default.pl';
require 'src/tools/msvc/config.pl' if (-f 'src/tools/msvc/config.pl');
@@ -94,7 +95,7 @@ sub installcheck
my @args = (
../../../$Config/pg_regress/pg_regress,
--dlpath=.,
- --psqldir=../../../$Config/psql,
+ --bindir=../../../$Config/psql,
--schedule=${schedule}_schedule,
--encoding=SQL_ASCII,
--no-locale);
@@ -106,15 +107,19 @@ sub installcheck
sub check
{
+ chdir $startdir;
+
+ InstallTemp();
+ chdir ${topdir}/src/test/regress;
+
my @args = (
- ../../../$Config/pg_regress/pg_regress,
+ ${tmp_installdir}/bin/pg_regress,
--dlpath=.,
- --psqldir=../../../$Config/psql,
+ --bindir=${tmp_installdir}/bin,
--schedule=${schedule}_schedule,
--encoding=SQL_ASCII,
--no-locale,
- --temp-install=./tmp_check,
- --top-builddir=\$topdir\);
+ --temp-instance=./tmp_check);
push(@args, $maxconn) if $maxconn;
push(@args, $temp_config) if $temp_config;
system(@args);
@@ -128,18 +133,20 @@ sub ecpgcheck
system(msbuild ecpg_regression.proj /p:config=$Config);
my $status = $? 8;
exit $status if $status;
+ InstallTemp();
chdir $topdir/src/interfaces/ecpg/test;
+
+ $ENV{PATH} = ${tmp_installdir}/bin;${tmp_installdir}/lib;$ENV{PATH};
$schedule = ecpg;
my @args = (
- ../../../../$Config/pg_regress_ecpg/pg_regress_ecpg,
- --psqldir=../../../$Config/psql,
+ ${tmp_installdir}/bin/pg_regress_ecpg,
+ --bindir=,
--dbname=regress1,connectdb,
--create-role=connectuser,connectdb,
--schedule=${schedule}_schedule,
--encoding=SQL_ASCII,
--no-locale,
- --temp-install=./tmp_chk,
- --top-builddir=\$topdir\);
+ --temp-instance=./tmp_chk);
push(@args, $maxconn) if $maxconn;
system(@args);
$status = $? 8;
@@ -148,12 +155,14 @@ sub ecpgcheck
sub isolationcheck
{
- chdir ../isolation;
- copy(../../../$Config/isolationtester/isolationtester.exe,
- ../../../$Config/pg_isolation_regress);
+ chdir $startdir;
+
+ InstallTemp();
+ chdir ${topdir}/src/test/isolation;
+
my @args = (
- ../../../$Config/pg_isolation_regress/pg_isolation_regress,
- --psqldir=../../../$Config/psql,
+ ${tmp_installdir}/bin/pg_isolation_regress,
+ --bindir=${tmp_installdir}/bin,
--inputdir=.,
--schedule=./isolation_schedule);
push(@args, $maxconn) if $maxconn;
@@ -164,7 +173,10 @@ sub isolationcheck
sub plcheck
{
- chdir ../../pl;
+ chdir $startdir;
+
+ InstallTemp();
+ chdir ${topdir}/src/pl;
foreach my $pl (glob(*))
{
@@ -201,8 +213,8 @@ sub plcheck
\n;
print Checking $lang\n;
my @args = (
- ../../../$Config/pg_regress/pg_regress,
- --psqldir=../../../$Config/psql,
+ ${tmp_installdir}/bin/pg_regress,
+ --bindir=${tmp_installdir}/bin/psql,
--dbname=pl_regression, @lang_args, @tests);
system(@args);
my $status = $? 8;
@@ -236,8 +248,8 @@ sub contribcheck
my @tests = fetchTests();
my @opts = fetchRegressOpts();
my @args = (
- ../../$Config/pg_regress/pg_regress,
- --psqldir=../../$Config/psql,
+ ${tmp_installdir}/bin/pg_regress,
+ --bindir=${tmp_installdir}/bin,
--dbname=contrib_regression, @opts, @tests);
system(@args);
my $status = $? 8;
@@ -251,8 +263,8 @@ sub contribcheck
sub standard_initdb
{
return (
- system('initdb', '-N') == 0 and system(
- $topdir/$Config/pg_regress/pg_regress, '--config-auth',
+ system(${tmp_installdir}/bin/initdb, '-N') == 0 and system(
+
Re: [HACKERS] Clamping reulst row number of joins.
Hello, thank you for the considerations by all of you, but I have described incorrectly the situation. I'm terribly sorry for the imprecise description and for your additional work based on it. The point of this issue is not VAULES but the nature of Append and NestLoop. Nested Loop can fail to have correct estimation when it contains Apeend node on insufficiently analyzed inhertance parent or UNION ALL. The latter is not saved by Tom's patch nor proper analyzing. Let me try to redescribe the issue. At Sun, 08 Mar 2015 19:30:36 -0400, Tom Lane t...@sss.pgh.pa.us wrote in 21519.1425857...@sss.pgh.pa.us I wrote: Stephen Frost sfr...@snowman.net writes: I've certainly seen and used values() constructs in joins for a variety of reasons and I do think it'd be worthwhile for the planner to know how to pull up a VALUES construct. The query reported to me didn't included VALUES. I implicitly used it as an shortcut to make Append node that cannot retrive statistics on itself. It was the main cause of this confision. Addition to that, there was somehow no statistics on the inheritance parent (not as a child), which I didn't mentioned. I don't know how the situation was made but I can guess manual analyzes as a cause. After all, the exact situation will emerge by following steps. CREATE TABLE p (a int); CREATE TABLE c1 (like p) INHERITS (p); INSERT INTO c1 (SELECT (a%2)*50 + a / 2 FROM generate_series(0, 100) a); CREATE INDEX i_c1_a on c1 (a); CREATE TABLE t1 (a int); INSERT INTO t1 VALUES (100); DELETE FROM pg_statistics; ANALYZE t1; ANALYZE c1; SET join_collapse_limit TO 1; SET random_page_cost TO 8.0; SET seq_page_cost TO 0.5; EXPLAIN analyzE SELECT a FROM p t WHERE a IN (select a from t1); - Nested Loop (cost=1.01..9.49 rows=51 width=4) (actual time=0.041..0.041 rows=0 loops=1) - HashAggregate (cost=1.01..1.02 rows=1 width=4) (actual time=0.017..0.017 rows=1 loops=1) Group Key: t1.a - Seq Scan on t1 (cost=0.00..1.01 rows=1 width=4) (actual time=0.007..0.009 rows=1 loops=1) - Append (cost=0.00..8.45 rows=2 width=4) (actual time=0.018..0.018 rows=0 loops=1) - Seq Scan on p t (cost=0.00..0.00 rows=1 width=4) (actual time=0.000..0.000 rows=0 loops=1) Filter: (t1.a = a) - Index Only Scan using i_c1_a on c1 t_1 (cost=0.42..8.45 rows=1 width=4) (actual time=0.012..0.012 rows=0 loops=1) Index Cond: (a = t1.a) Heap Fetches: 0 Planning time: 0.408 ms Execution time: 0.135 ms (12 rows) The nested loop above decided that rows to be 51 because seleqfunc_semi couldn't get MVC list for p (as the parent) and returns 0.5 as the default join selectivity. Therefore, ANALYZE p makes this work sane for the case. ANALYZE p; EXPLAIN analyzE SELECT a FROM p t WHERE a IN (select a from t1); - Nested Loop (cost=1.01..9.49 rows=1 width=4) I thought that the case of apparently wrong row numbers would be saved by clampling the number of result rows as I mentioned but since it is saved by the proper analyzes, the necessity is rather low if analyze works always. Still, the following example cannot be saved even by analyze. CREATE TABLE c2 (LIKE p); ANALYZE p; EXPLAIN analyzE select a FROM (SELECT a FROM c1 UNION ALL SELECT a from c2) t WHERE a IN (select a from t1); QUERY PLAN --- Nested Loop (cost=1.44..51.48 rows=501276 width=4) (actual time=0.046..0.046 rows=0 loops=1) - HashAggregate (cost=1.01..1.02 rows=1 width=4) (actual time=0.018..0.019 rows=1 loops=1) Group Key: t1.a - Seq Scan on t1 (cost=0.00..1.01 rows=1 width=4) (actual time=0.008..0.010 rows=1 loops=1) - Append (cost=0.42..50.32 rows=14 width=4) (actual time=0.023..0.023 rows=0 loops=1) - Index Only Scan using i_c1_a on c1 (cost=0.42..8.45 rows=1 width=4) (actual time=0.016..0.016 rows=0 loops=1) Index Cond: (a = t1.a) Heap Fetches: 0 - Seq Scan on c2 (cost=0.00..41.88 rows=13 width=4) (actual time=0.001..0.001 rows=0 loops=1) Filter: (t1.a = a) Planning time: 0.384 ms Execution time: 0.185 ms (12 rows) 14 * 1 = 501276... Of course this cannot be saved by the values-flatten patch unfortunately... I think this is a not so rare case.. == I spent a bit of time looking at this, and realized that the blocker is the
[HACKERS] Object files generated by ecpg test suite not ignored on Windows
Hi all, When running the test suite of ecpg, build generates a set of obj files and it happens that those files are not ignored in the code tree. Patch is attached. Regards, -- Michael diff --git a/src/interfaces/ecpg/test/compat_informix/.gitignore b/src/interfaces/ecpg/test/compat_informix/.gitignore index f97706b..6086676 100644 --- a/src/interfaces/ecpg/test/compat_informix/.gitignore +++ b/src/interfaces/ecpg/test/compat_informix/.gitignore @@ -1,18 +1,27 @@ /charfuncs /charfuncs.c +/charfuncs.obj /dec_test /dec_test.c +/dec_test.obj /describe /describe.c +/describe.obj /rfmtdate /rfmtdate.c +/rfmtdate.obj /rfmtlong /rfmtlong.c +/rfmtlong.obj /rnull /rnull.c +/rnull.obj /sqlda /sqlda.c +/sqlda.obj /test_informix /test_informix.c +/test_informix.obj /test_informix2 /test_informix2.c +/test_informix2.obj diff --git a/src/interfaces/ecpg/test/connect/.gitignore b/src/interfaces/ecpg/test/connect/.gitignore index e0639f3..fe33bba 100644 --- a/src/interfaces/ecpg/test/connect/.gitignore +++ b/src/interfaces/ecpg/test/connect/.gitignore @@ -1,10 +1,15 @@ /test1 /test1.c +/test1.obj /test2 /test2.c +/test2.obj /test3 /test3.c +/test3.obj /test4 /test4.c +/test4.obj /test5 /test5.c +/test5.obj diff --git a/src/interfaces/ecpg/test/pgtypeslib/.gitignore b/src/interfaces/ecpg/test/pgtypeslib/.gitignore index 2987fef..787c6b2 100644 --- a/src/interfaces/ecpg/test/pgtypeslib/.gitignore +++ b/src/interfaces/ecpg/test/pgtypeslib/.gitignore @@ -1,10 +1,15 @@ /dt_test /dt_test.c +/dt_test.obj /dt_test2 /dt_test2.c +/dt_test2.obj /nan_test /nan_test.c +/nan_test.obj /num_test /num_test.c +/num_test.obj /num_test2 /num_test2.c +/num_test2.obj diff --git a/src/interfaces/ecpg/test/preproc/.gitignore b/src/interfaces/ecpg/test/preproc/.gitignore index ffca98e..f274216 100644 --- a/src/interfaces/ecpg/test/preproc/.gitignore +++ b/src/interfaces/ecpg/test/preproc/.gitignore @@ -1,24 +1,36 @@ /array_of_struct /array_of_struct.c +/array_of_struct.obj /autoprep /autoprep.c +/autoprep.obj /comment /comment.c +/comment.obj /cursor /cursor.c +/cursor.obj /define /define.c +/define.obj /init /init.c +/init.obj /outofscope /outofscope.c +/outofscope.obj /pointer_to_struct /pointer_to_struct.c +/pointer_to_struct.obj /strings /strings.c +/strings.obj /type /type.c +/type.obj /variable /variable.c +/variable.obj /whenever /whenever.c +/whenever.obj diff --git a/src/interfaces/ecpg/test/sql/.gitignore b/src/interfaces/ecpg/test/sql/.gitignore index cd6f342..5d4fe7f 100644 --- a/src/interfaces/ecpg/test/sql/.gitignore +++ b/src/interfaces/ecpg/test/sql/.gitignore @@ -1,40 +1,60 @@ /array /array.c +/array.obj /binary /binary.c +/binary.obj /code100 /code100.c +/code100.obj /copystdout /copystdout.c +/copystdout.obj /define /define.c +/define.obj /desc /desc.c +/desc.obj /describe /describe.c +/describe.obj /dynalloc /dynalloc.c +/dynalloc.obj /dynalloc2 /dynalloc2.c +/dynalloc2.obj /dyntest /dyntest.c +/dyntest.obj /execute /execute.c +/execute.obj /fetch /fetch.c +/fetch.obj /func /func.c +/func.obj /indicators /indicators.c +/indicators.obj /insupd /insupd.c +/insupd.obj /oldexec /oldexec.c +/oldexec.obj /parser /parser.c +/parser.obj /quote /quote.c +/quote.obj /show /show.c +/show.obj /sqlda /sqlda.c +/sqlda.obj diff --git a/src/interfaces/ecpg/test/thread/.gitignore b/src/interfaces/ecpg/test/thread/.gitignore index 12ff319..6c16f4e 100644 --- a/src/interfaces/ecpg/test/thread/.gitignore +++ b/src/interfaces/ecpg/test/thread/.gitignore @@ -1,10 +1,15 @@ /alloc /alloc.c +/alloc.obj /descriptor /descriptor.c +/descriptor.obj /prep /prep.c +/prep.obj /thread /thread.c +/thread.obj /thread_implicit /thread_implicit.c +/thread_implicit.obj -- 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] MD5 authentication needs help -SCRAM
At 2015-03-08 12:48:44 -0700, j...@agliodbs.com wrote: Since SCRAM has been brought up a number of times here, I thought I'd loop in the PostgreSQL contributor who is co-author of the SCRAM standard to see if he has anything to say about implementing SCRAM as a built-in auth method for Postgres. I think it's a good idea. -- Abhijit -- 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] EvalPlanQual behaves oddly for FDW queries involving system columns
Hi Fujita-san, I tried reproducing the issue with the steps summarised. Here's my setup postgres=# \d ft Foreign table public.ft Column | Type | Modifiers | FDW Options +-+---+- a | integer | | Server: loopback FDW Options: (table_name 'lbt') postgres=# \d lbt Table public.lbt Column | Type | Modifiers +-+--- a | integer | The select (without for update) returns me two rows (one inserted in lbt and one in ft), whereas in your description there is only one row. For me, I am getting following error postgres=# select ft.tableoid, ft.ctid, ft.* from lbt, ft where lbt.a = ft.a for update; ERROR: could not serialize access due to concurrent update CONTEXT: Remote SQL command: SELECT a, ctid FROM public.lbt FOR UPDATE postgres=# after commit on terminal 1. Am I missing something? -- Best Wishes, Ashutosh Bapat EnterpriseDB Corporation The Postgres Database Company
Re: [HACKERS] [REVIEW] Re: Compression of full-page-writes
On Thu, Mar 5, 2015 at 10:08 PM, Michael Paquier
michael.paqu...@gmail.com wrote:
On Thu, Mar 5, 2015 at 9:14 PM, Syed, Rahila rahila.s...@nttdata.com wrote:
Please find attached a patch. As discussed, flag to denote compression and
presence of hole in block image has been added in XLogRecordImageHeader
rather than block header.
Thanks for updating the patch! Attached is the refactored version of the patch.
Regards,
--
Fujii Masao
*** a/contrib/pg_xlogdump/pg_xlogdump.c
--- b/contrib/pg_xlogdump/pg_xlogdump.c
***
*** 359,376 XLogDumpCountRecord(XLogDumpConfig *config, XLogDumpStats *stats,
rec_len = XLogRecGetDataLen(record) + SizeOfXLogRecord;
/*
! * Calculate the amount of FPI data in the record. Each backup block
! * takes up BLCKSZ bytes, minus the hole length.
*
* XXX: We peek into xlogreader's private decoded backup blocks for the
! * hole_length. It doesn't seem worth it to add an accessor macro for
! * this.
*/
fpi_len = 0;
for (block_id = 0; block_id = record-max_block_id; block_id++)
{
if (XLogRecHasBlockImage(record, block_id))
! fpi_len += BLCKSZ - record-blocks[block_id].hole_length;
}
/* Update per-rmgr statistics */
--- 359,375
rec_len = XLogRecGetDataLen(record) + SizeOfXLogRecord;
/*
! * Calculate the amount of FPI data in the record.
*
* XXX: We peek into xlogreader's private decoded backup blocks for the
! * bimg_len indicating the length of FPI data. It doesn't seem worth it to
! * add an accessor macro for this.
*/
fpi_len = 0;
for (block_id = 0; block_id = record-max_block_id; block_id++)
{
if (XLogRecHasBlockImage(record, block_id))
! fpi_len += record-blocks[block_id].bimg_len;
}
/* Update per-rmgr statistics */
***
*** 465,473 XLogDumpDisplayRecord(XLogDumpConfig *config, XLogReaderState *record)
blk);
if (XLogRecHasBlockImage(record, block_id))
{
! printf( (FPW); hole: offset: %u, length: %u\n,
! record-blocks[block_id].hole_offset,
! record-blocks[block_id].hole_length);
}
putchar('\n');
}
--- 464,485
blk);
if (XLogRecHasBlockImage(record, block_id))
{
! if (record-blocks[block_id].bimg_info
! BKPIMAGE_IS_COMPRESSED)
! {
! printf( (FPW); hole: offset: %u, length: %u, compression saved: %u\n,
! record-blocks[block_id].hole_offset,
! record-blocks[block_id].hole_length,
! BLCKSZ -
! record-blocks[block_id].hole_length -
! record-blocks[block_id].bimg_len);
! }
! else
! {
! printf( (FPW); hole: offset: %u, length: %u\n,
! record-blocks[block_id].hole_offset,
! record-blocks[block_id].hole_length);
! }
}
putchar('\n');
}
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
***
*** 2282,2287 include_dir 'conf.d'
--- 2282,2311
/listitem
/varlistentry
+ varlistentry id=guc-wal-compression xreflabel=wal_compression
+ termvarnamewal_compression/varname (typeboolean/type)
+ indexterm
+primaryvarnamewal_compression/ configuration parameter/primary
+ /indexterm
+ /term
+ listitem
+para
+ When this parameter is literalon/, the productnamePostgreSQL/
+ server compresses a full page image written to WAL when
+ xref linkend=guc-full-page-writes is on or during a base backup.
+ A compressed page image will be decompressed during WAL replay.
+ The default value is literaloff/
+/para
+
+para
+ Turning this parameter on can reduce the WAL volume without
+ increasing the risk of unrecoverable data corruption,
+ but at the cost of some extra CPU time by the compression during
+ WAL logging and the decompression during WAL replay.
+/para
+ /listitem
+ /varlistentry
+
varlistentry id=guc-wal-buffers xreflabel=wal_buffers
termvarnamewal_buffers/varname (typeinteger/type)
indexterm
*** a/src/backend/access/transam/xlog.c
--- b/src/backend/access/transam/xlog.c
***
*** 89,94 char *XLogArchiveCommand = NULL;
--- 89,95
bool EnableHotStandby = false;
bool fullPageWrites = true;
bool wal_log_hints = false;
+ bool wal_compression = false;
bool log_checkpoints = false;
int sync_method = DEFAULT_SYNC_METHOD;
int wal_level = WAL_LEVEL_MINIMAL;
*** a/src/backend/access/transam/xloginsert.c
--- b/src/backend/access/transam/xloginsert.c
***
*** 24,35
--- 24,39
#include access/xlog_internal.h
#include access/xloginsert.h
#include catalog/pg_control.h
+ #include common/pg_lzcompress.h
#include miscadmin.h
#include storage/bufmgr.h
#include storage/proc.h
#include utils/memutils.h
#include pg_trace.h
+ /* Buffer size required to store a compressed version of
