Re: [HACKERS] Minmax indexes
On Mon, Sep 16, 2013 at 3:47 AM, Thom Brown t...@linux.com wrote: On 15 September 2013 01:14, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Hi, Here's a reviewable version of what I've dubbed Minmax indexes. Thanks for the patch, but I seem to have immediately hit a snag: pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax (aid); PANIC: invalid xlog record length 0 fwiw, this seems to be triggered by ANALYZE. At least i can trigger it by executing ANALYZE on the table (attached is a stacktrace of a backend exhibiting the failure) Another thing is this messages i got when compiling: mmxlog.c: In function ‘minmax_xlog_revmap_set’: mmxlog.c:161:14: warning: unused variable ‘blkno’ [-Wunused-variable] bufpage.c: In function ‘PageIndexDeleteNoCompact’: bufpage.c:1066:18: warning: ‘lastused’ may be used uninitialized in this function [-Wmaybe-uninitialized] -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157 Program received signal SIGABRT, Aborted. 0x7f4428819475 in *__GI_raise (sig=optimized out) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 64 ../nptl/sysdeps/unix/sysv/linux/raise.c: No existe el fichero o el directorio. (gdb) bt #0 0x7f4428819475 in *__GI_raise (sig=optimized out) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 #1 0x7f442881c6f0 in *__GI_abort () at abort.c:92 #2 0x0076d6ac in errfinish (dummy=dummy@entry=0) at elog.c:546 #3 0x0076fdca in elog_finish (elevel=elevel@entry=22, fmt=fmt@entry=0x7d2aa7 invalid xlog record length %u) at elog.c:1304 #4 0x004e1eba in XLogInsert (rmid=rmid@entry=17 '\021', info=info@entry=48 '0', rdata=rdata@entry=0x7fffc836ea10) at xlog.c:966 #5 0x004a9bb9 in mmSetHeapBlockItemptr (rmAccess=rmAccess@entry=0x1b3af38, heapBlk=heapBlk@entry=0, blkno=blkno@entry=6, offno=offno@entry=1) at mmrevmap.c:169 #6 0x004a73e8 in mm_doinsert (idxrel=0x7f4429054c88, rmAccess=0x1b3af38, buffer=buffer@entry=0x7f441f7e718c, heapblkno=0, tup=tup@entry=0x7f441f84cff8, itemsz=16) at minmax.c:1410 #7 0x004a9464 in rerun_summarization (numnonsummarized=22, nonsummarized=0x1b3a408, rmAccess=0x1b3af38, heapRel=0x7f4429052e68, idxRel=0x7f4429054c88) at minmax.c:1205 #8 mmvacuumcleanup (fcinfo=optimized out) at minmax.c:1268 #9 0x0077388f in FunctionCall2Coll (flinfo=flinfo@entry=0x7fffc836f0a0, collation=collation@entry=0, arg1=arg1@entry=140736552432368, arg2=arg2@entry=0) at fmgr.c:1326 #10 0x004a6c2d in index_vacuum_cleanup (info=info@entry=0x7fffc836f2f0, stats=stats@entry=0x0) at indexam.c:715 #11 0x005570d1 in do_analyze_rel (onerel=onerel@entry=0x7f4429052e68, acquirefunc=0x556020 acquire_sample_rows, relpages=45, inh=inh@entry=0 '\000', elevel=elevel@entry=13, vacstmt=error reading variable: Unhandled dwarf expression opcode 0xfa, vacstmt=error reading variable: Unhandled dwarf expression opcode 0xfa) at analyze.c:634 #12 0x00557fef in analyze_rel (relid=relid@entry=16384, vacstmt=vacstmt@entry=0x1af5678, bstrategy=optimized out) at analyze.c:267 ---Type return to continue, or q return to quit--- #13 0x005aa224 in vacuum (vacstmt=vacstmt@entry=0x1af5678, relid=relid@entry=0, do_toast=do_toast@entry=1 '\001', bstrategy=optimized out, bstrategy@entry=0x0, for_wraparound=for_wraparound@entry=0 '\000', isTopLevel=isTopLevel@entry=1 '\001') at vacuum.c:249 #14 0x0069f417 in standard_ProcessUtility (parsetree=0x1af5678, queryString=optimized out, context=optimized out, params=0x0, dest=optimized out, completionTag=optimized out) at utility.c:682 #15 0x0069c587 in PortalRunUtility (portal=0x1b33198, utilityStmt=0x1af5678, isTopLevel=1 '\001', dest=0x1af5a00, completionTag=0x7fffc836f920 ) at pquery.c:1187 #16 0x0069d299 in PortalRunMulti (portal=portal@entry=0x1b33198, isTopLevel=isTopLevel@entry=1 '\001', dest=dest@entry=0x1af5a00, altdest=altdest@entry=0x1af5a00, completionTag=completionTag@entry=0x7fffc836f920 ) at pquery.c:1318 #17 0x0069df32 in PortalRun (portal=portal@entry=0x1b33198, count=count@entry=9223372036854775807, isTopLevel=isTopLevel@entry=1 '\001', dest=dest@entry=0x1af5a00, altdest=altdest@entry=0x1af5a00, completionTag=completionTag@entry=0x7fffc836f920 ) at pquery.c:816 #18 0x0069afdb in exec_simple_query (query_string=0x1af4c18 analyze t1;) at postgres.c:1048 #19 PostgresMain (argc=optimized out, argv=argv@entry=0x1ab0a70, dbname=0x1ab08f0 postgres, username=optimized out) at postgres.c:3992 #20 0x0046559e in BackendRun (port=0x1ab2770) at postmaster.c:4083 #21 BackendStartup (port=0x1ab2770) at postmaster.c:3772 #22 ServerLoop () at postmaster.c:1583 #23 0x0065230e in PostmasterMain (argc=argc@entry=3, argv=argv@entry=0x1a8df00) at postmaster.c:1239 #24 0x00465ec5 in main (argc=3,
Re: [HACKERS] Patch for fail-back without fresh backup
syncrep.c: In function ‘SyncRepReleaseWaiters’: syncrep.c:421:6: warning: variable ‘numdataflush’ set but not used [-Wunused-but-set-variable] Sorry I forgot fix it. I have attached the patch which I modified. Attached patch combines documentation patch and source-code patch. -- Regards, Samrat Revgade synchronous_transfer_v7.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] PostgreSQL 9.3 beta breaks some extensions make install
Apt.pgdg got the patch present in postgresql head applyed. Erm, isn't apt.postgresql.org supposed to ship the *official* PostgreSQL versions? Given that this issue affects all distros, I don't see why Ubuntu/Debian need to be patched separately. Well, the patches are applyed on the debian packages (not only in apt.pgdg.org). The packages provided by apt.postgresql.org are based on the 'official packages' from debian. (if you allow me this circle) Does anyone else think this is problematic? By slipping patches into distro-specific packages, you're confusing users (like me) and bypassing the PostgreSQL QA process. The QA is about distribution of packages here. Debian applies 14 patches on 9.3 builds, not only the things about vpath we're talking about. PS: Where are the sources used to build packages on apt.postgresql.org? 9.3: http://anonscm.debian.org/loggerhead/pkg-postgresql/postgresql-9.3/trunk/changes -- Cédric Villemain +33 (0)6 20 30 22 52 http://2ndQuadrant.fr/ PostgreSQL: Support 24x7 - Développement, Expertise et Formation signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Minmax indexes
On 17 September 2013 07:20, Jaime Casanova ja...@2ndquadrant.com wrote: On Mon, Sep 16, 2013 at 3:47 AM, Thom Brown t...@linux.com wrote: On 15 September 2013 01:14, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Hi, Here's a reviewable version of what I've dubbed Minmax indexes. Thanks for the patch, but I seem to have immediately hit a snag: pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax (aid); PANIC: invalid xlog record length 0 fwiw, this seems to be triggered by ANALYZE. At least i can trigger it by executing ANALYZE on the table (attached is a stacktrace of a backend exhibiting the failure) Another thing is this messages i got when compiling: mmxlog.c: In function ‘minmax_xlog_revmap_set’: mmxlog.c:161:14: warning: unused variable ‘blkno’ [-Wunused-variable] bufpage.c: In function ‘PageIndexDeleteNoCompact’: bufpage.c:1066:18: warning: ‘lastused’ may be used uninitialized in this function [-Wmaybe-uninitialized] I'm able to run ANALYSE manually without it dying: pgbench=# analyse pgbench_accounts; ANALYZE pgbench=# analyse pgbench_accounts; ANALYZE pgbench=# create index minmaxtest on pgbench_accounts using minmax (aid); PANIC: invalid xlog record length 0 -- Thom
Re: [HACKERS] PL/pgSQL, RAISE and error context
Hi Marko, I have reviewed this patch. 1. Patch applies well. 2. make and make install is fine 3. make check is fine too. But as Peter pointed out plperl regression tests are failing. I just did grep on .sql files and found following files which has RAISE statement into it. These files too need expected output changes. Please run these testcases to get diffs. ./src/pl/plperl/sql/plperl_elog.sql ./src/pl/plpython/sql/plpython_error.sql ./src/pl/plpython/sql/plpython_setof.sql ./src/pl/plpython/sql/plpython_quote.sql ./contrib/sepgsql/sql/label.sql ./contrib/sepgsql/sql/ddl.sql Code changes looks fine to me. Thanks -- Jeevan B Chalke Principal Software Engineer, Product Development EnterpriseDB Corporation
Re: [HACKERS] insert throw error when year field len 4 for timestamptz datatype
On Mon, Sep 16, 2013 at 7:22 PM, Haribabu kommi haribabu.ko...@huawei.comwrote: *On *14 August 2013 Rushabh Lathia wrote:** ** ** postgres=# create table test ( a timestamptz); CREATE TABLE ** ** -- Date with year 1000 postgres=# insert into test values ( 'Sat Mar 11 23:58:48 1000 IST');*** * INSERT 0 1 ** ** -- Now try with year 1 it will return error postgres=# insert into test values ( 'Sat Mar 11 23:58:48 1 IST');** ** ERROR: invalid input syntax for type timestamp with time zone: Sat Mar 11 23:58:48 1 IST LINE 1: insert into test values ( 'Sat Mar 11 23:58:48 1 IST'); ** ** here error coming from timestamptz_in() - datefields_to_timestamp() -** ** DecodeDateTime() stack. ** ** Looking more at the DecodeDateTime() function, here error coming while trying to Decode year field which is 1 in the our test. For year field ftype is DTK_NUMBER, and under DTK_NUMBER for this case if drop in to following condition: ** ** else if (flen 4) { dterr = DecodeNumberField(flen, field[i], fmask, tmask, tm, fsec, is2digits); if (dterr 0) return dterr; } ** ** because flen in out case flen is 5 (1). ** ** As per the comment above DecodeNumberField(), it interpret numeric string as a concatenated date or time field. So ideally we should be into DecodeNumberField function only with (fmask DTK_DATE_M) == 0 or (fmask DTK_TIME_M) == 0, right ?? ** ** So, I tried the same and after that test working fine. ** ** PFA patch and share your input/suggestions. ** ** Patch applies cleanly to HEAD. As this patch tries to improve in inserting the date of the year value to be more than 4 in length. But it didn’t solve all the ways to insert the year field more than 4 in length. Please check the following test. ** ** ** ** postgres=# insert into test values ('10001010 10:10:10 IST'); INSERT 0 1 postgres=# insert into test values ('100011010 10:10:10 IST'); ERROR: invalid input syntax for type timestamp with time zone: 100011010 10:10:10 IST at character 26 STATEMENT: insert into test values ('100011010 10:10:10 IST'); ERROR: invalid input syntax for type timestamp with time zone: 100011010 10:10:10 IST LINE 1: insert into test values ('100011010 10:10:10 IST'); ^ ** ** I feel it is better to provide the functionality of inserting year field more than 4 in length in all flows. +1. Nice catch. Here is the latest version of patch which handles the functionality in all flows. Could you test it and share you comments. Thanks, Rushabh Lathia www.EnterpriseDB.com timestamptz_fix_with_testcase_v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] record identical operator
On 09/16/2013 04:01 PM, Kevin Grittner wrote: Hannu Krosing ha...@2ndquadrant.com wrote: Lots of people were bitten when (undocumented) hash functions were changed thus breaking hash-based partitioning. Nobody can be affected by the new operators in this patch unless they choose to use them to compare two records. They don't work for any other type and they don't come into play unless you specifically request them. Maybe the binary equality operator should be named for really deeply equal to distinguish it from === which would be merely NOT DISTINCT FROM we could even go one step further and define = to mean the same. ? This could fit better in conceptual sequence of operator 'strength' -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Typo fix in spgtextproc.c
I ran into a typo. Attached is a patch. Thanks, Best regards, Etsuro Fujita typofix-20130917.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Docs fix in advanced.sgml
I think the document in advanced.sgml should be corrected, though I might misunderstand the rules of usage. Attached is a patch. Thanks, Best regards, Etsuro Fujita docsfix-20130917.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Minor inheritance/check bug: Inconsistent behavior
Hi Amit. I gone through the mail thread discussion regarding this issue and reviewed you patch. -- Patch get applied cleanly on Master branch -- Make and Make Install fine -- make check also running cleanly In the patch code changes looks good to me. This patch having two part: 1) Allowed TableOid system column in the CHECK constraint 2) Throw an error if other then TableOid system column in CHECK constraint. I noticed that you added test coverage for 1) but the test coverage for 2) is missing.. I added the test coverage for 2) in the attached patch. Marking this as Ready for committer. Regards, Rushabh On Sun, Sep 15, 2013 at 2:31 PM, Amit Kapila amit.kapil...@gmail.comwrote: Bruce Momjian wrote: On Sun, Jun 30, 2013 at 06:57:10AM +, Amit kapila wrote: I have done the initial analysis and prepared a patch, don't know if anything more I can do until someone can give any suggestions to further proceed on this bug. So, I guess we never figured this out. I can submit this bug-fix for next commitfest if there is no objection for doing so. What is your opinion? Yes, good idea. I had rebased the patch against head and added the test case to validate it. I will upload this patch to commit fest. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Rushabh Lathia www.EnterpriseDB.com sys_col_constr_v2.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PATCH] Add use of asprintf()
Hi, I did put some time review the patch, please see my findings below i.e. 1. It seems that you have used strdup() on multiple places in the patch, e.g. in the below code snippet is it going to lead crash if newp-ident is NULL because of strdup() failure ? static EPlan * find_plan(char *ident, EPlan **eplan, int *nplans) { ... newp-ident = strdup(ident); ... } 2. Can we rely on return value of asprintf() instead of recompute length as strlen(cmdbuf) ? if (asprintf(cmdbuf, COMMENT ON LARGE OBJECT %u IS ', loid) 0) return fail_lo_xact(\\lo_import, own_transaction); bufptr = cmdbuf + strlen(cmdbuf); 3. There seems naming convention for functions like pfree (that seems similar to free()), pstrdup etc; but psprintf seems different than sprintf. Can't we use pg_asprintf instead in the patch, as it seems that both (psprintf and pg_asprintf) provide almost same functionality ? 4. It seems that ret 0 need to be changed to rc 0 in the following code i.e. int pg_asprintf(char **ret, const char *format, ...) { va_list ap; int rc; va_start(ap, format); rc = vasprintf(ret, format, ap); va_end(ap); if (ret 0) { fprintf(stderr, _(out of memory\n)); exit(EXIT_FAILURE); } return rc; } 5. It seems that in the previously implementation, error handling was not there, is it appropriate here that if there is issue in duplicating environment, quit the application ? i.e. /* * Handy subroutine for setting an environment variable var to val */ static void doputenv(const char *var, const char *val) { char*s; pg_asprintf(s, %s=%s, var, val); putenv(s); } Please do let me know if I missed something or more info is required. Thank you. Regards, Muhammad Asif Naeem On Tue, Sep 17, 2013 at 1:31 AM, Alvaro Herrera alvhe...@2ndquadrant.comwrote: Peter Eisentraut wrote: The attached patch should speak for itself. Yeah, it's a very nice cleanup. I have supplied a few variants: - asprintf() is the standard function, supplied by libpgport if necessary. - pg_asprintf() is asprintf() with automatic error handling (like pg_malloc(), etc.) - psprintf() is the same idea but with palloc. Looks good to me, except that pg_asprintf seems to be checking ret instead of rc. Is there a reason for the API discrepancy of pg_asprintf vs. psprintf? I don't see that we use the integer return value anywhere. Callers interested in the return value can use asprintf directly (and you have already inserted callers that do nonstandard things using direct asprintf). -- Álvaro Herrerahttp://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] Add use of asprintf()
On Tue, Sep 17, 2013 at 3:13 PM, Asif Naeem anaeem...@gmail.com wrote: Hi, I did put some time review the patch, please see my findings below i.e. 1. It seems that you have used strdup() on multiple places in the patch, e.g. in the below code snippet is it going to lead crash if newp-ident is NULL because of strdup() failure ? static EPlan * find_plan(char *ident, EPlan **eplan, int *nplans) { ... newp-ident = strdup(ident); ... } 2. Can we rely on return value of asprintf() instead of recompute length as strlen(cmdbuf) ? if (asprintf(cmdbuf, COMMENT ON LARGE OBJECT %u IS ', loid) 0) return fail_lo_xact(\\lo_import, own_transaction); bufptr = cmdbuf + strlen(cmdbuf); 3. There seems naming convention for functions like pfree (that seems similar to free()), pstrdup etc; but psprintf seems different than sprintf. Can't we use pg_asprintf instead in the patch, as it seems that both (psprintf and pg_asprintf) provide almost same functionality ? 4. It seems that ret 0 need to be changed to rc 0 in the following code i.e. int pg_asprintf(char **ret, const char *format, ...) { va_list ap; int rc; va_start(ap, format); rc = vasprintf(ret, format, ap); va_end(ap); if (ret 0) { fprintf(stderr, _(out of memory\n)); exit(EXIT_FAILURE); } return rc; } It seems this point is already mentioned by Alvaro. Thanks. 5. It seems that in the previously implementation, error handling was not there, is it appropriate here that if there is issue in duplicating environment, quit the application ? i.e. /* * Handy subroutine for setting an environment variable var to val */ static void doputenv(const char *var, const char *val) { char*s; pg_asprintf(s, %s=%s, var, val); putenv(s); } Please do let me know if I missed something or more info is required. Thank you. Regards, Muhammad Asif Naeem On Tue, Sep 17, 2013 at 1:31 AM, Alvaro Herrera alvhe...@2ndquadrant.comwrote: Peter Eisentraut wrote: The attached patch should speak for itself. Yeah, it's a very nice cleanup. I have supplied a few variants: - asprintf() is the standard function, supplied by libpgport if necessary. - pg_asprintf() is asprintf() with automatic error handling (like pg_malloc(), etc.) - psprintf() is the same idea but with palloc. Looks good to me, except that pg_asprintf seems to be checking ret instead of rc. Is there a reason for the API discrepancy of pg_asprintf vs. psprintf? I don't see that we use the integer return value anywhere. Callers interested in the return value can use asprintf directly (and you have already inserted callers that do nonstandard things using direct asprintf). -- Álvaro Herrerahttp://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] pg_stat_statements: calls under-estimation propagation
On Sun, Sep 15, 2013 at 3:54 PM, samthakur74 samthaku...@gmail.com wrote: You have added this email to the commit fest, but it contains no patch. Please add the email with the actual patch. I hope its attached now! You seem to have forgotten to include the pg_stat_statements--1.2.sql and pg_stat_statements--1.1--1.2.sql in the patch. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] record identical operator
Andres Freund and...@2ndquadrant.com wrote: On 2013-09-16 15:26:08 -0700, Kevin Grittner wrote: I can understand claiming that the risk is acceptable but arguing it's not there seems extremly strange to me. It's not a risk. It's why the operator exists. Pft. It's fine if the materialized view code uses it. I don't see the danger there. It's about users discovering it. If they notice it, they will use it because its a crapload faster than normal row comparisons. To have clean semantics, I think the operator should mean that the stored format of the row is the same. Regarding the array null bitmap example, I think it would be truly weird if the operator said that the stored format was the same, but this happened: test=# select pg_column_size(ARRAY[1,2,3]); pg_column_size 36 (1 row) test=# select pg_column_size((ARRAY[1,2,3,NULL])::int4[3]); pg_column_size 44 (1 row) They have the same stored format, but a different number of bytes?!? And deals with NULLs in a simpler way. That might be addressed by leaving room to implement IS NOT DISTINCT FROM as an operator. See below. Perhaps the name of the operator (===) or the fact that I've been using the shorthand description of identical instead of writing out record image equals in these emails has confused you. If you really think that confusing you is the correct description for concerns about users not understanding limitations (which you didn't seem to know about), go ahead. Way to go to solicit feedback. I see how that could be taken in a pejorative way; that was not intended. I apologize. I was rushing a bit on that email to make an appointment shortly afterward. The point was to suggest that bad communication on my part about the intent and concept of the operator may have contributed to you saying that there was a risk of it working as intended. There is perhaps a risk that someone will think that it does something other than what is intended, but when you say that there is a risk that it will behave exactly as intended, it does suggest that we're talking past each other. I can stop using the shorter description and have absolutely no attachment to the operator name, if that helps. You're unfortunately going to need operators if you want mergejoins to work, right? Yes, operators are needed for that. If not I'd have said name it matview_needs_update() or something like that... But yes, === certainly seems like a bad idea. I've come to agree with that. The appointment was to meet with a local PostgreSQL user who I've talked to before. He reminded me that his pet peeve was that he couldn't use IS [ NOT ] DISTINCT FROM with the ALL | ANY construct because the IS [ NOT ] DISTINCT FROM predicate isn't an operator. Hannu Krosing ha...@2ndquadrant.com wrote: Maybe the binary equality operator should be named for really deeply equal to distinguish it from === which would be merely NOT DISTINCT FROM we could even go one step further and define = to mean the same. ? This could fit better in conceptual sequence of operator 'strength' I'm inclined to go with this. It would leave the door open to implementing IS NOT DISTINCT FROM on a type-by-type basis. My one concern is that it doesn't make room for a has no user-visible differences from operator; but I'm not sure whether that is something that there really is any use for. But if we want to reserve name space for it just in case someone has a use for it, that would be between IS NOT DISTINCT FROM and has the same storage representation as, so that would leave: = equals (but doesn't necessarily look the same as) === IS NOT DISTINCT FROM as an operator reserved for has no user-visible differences from = stored image is the same Of course, that begs the question of whether == is already taken. If not, we could knock one '=' off of everything above except for equals. What existing uses are known for == ? -- 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] UTF8 national character data type support WIP patch and list of open issues.
-Original Message- From: pgsql-hackers-ow...@postgresql.org [mailto:pgsql-hackers- ow...@postgresql.org] On Behalf Of MauMau Hello, I think it would be nice for PostgreSQL to support national character types largely because it should ease migration from other DBMSs. [Reasons why we need NCHAR] -- 1. Invite users of other DBMSs to PostgreSQL. Oracle, SQL Server, MySQL, etc. all have NCHAR support. PostgreSQL is probably the only database out of major ones that does not support NCHAR. Sadly, I've read a report from some Japanese government agency that the number of MySQL users exceeded that of PostgreSQL here in Japan in 2010 or 2011. I wouldn't say that is due to NCHAR support, but it might be one reason. I want PostgreSQL to be more popular and regain those users. 2. Enhance the open image of PostgreSQL by implementing more features of SQL standard. NCHAR may be a wrong and unnecessary feature of SQL standard now that we have Unicode support, but it is defined in the standard and widely implemented. 3. I have heard that some potential customers didn't adopt PostgreSQL due to lack of NCHAR support. However, I don't know the exact reason why they need NCHAR. The use case we have is for customer(s) who are modernizing their databases on mainframes. These applications are typically written in COBOL which does have extensive support for National Characters. Supporting National Characters as in-built data types in PostgreSQL is, not to exaggerate, an important criteria in their decision to use PostgreSQL or not. (So is Embedded COBOL. But that is a separate issue.) 4. I guess some users really want to continue to use ShiftJIS or EUC_JP for database encoding, and use NCHAR for a limited set of columns to store international text in Unicode: - to avoid code conversion between the server and the client for performance - because ShiftJIS and EUC_JP require less amount of storage (2 bytes for most Kanji) than UTF-8 (3 bytes) This use case is described in chapter 6 of Oracle Database Globalization Support Guide. -- I think we need to do the following: [Minimum requirements] -- 1. Accept NCHAR/NVARCHAR as data type name and N'...' syntactically. This is already implemented. PostgreSQL treats NCHAR/NVARCHAR as synonyms for CHAR/VARCHAR, and ignores N prefix. But this is not documented. 2. Declare support for national character support in the manual. 1 is not sufficient because users don't want to depend on undocumented behavior. This is exactly what the TODO item national character support in PostgreSQL TODO wiki is about. 3. Implement NCHAR/NVARCHAR as distinct data types, not as synonyms so that: - psql \d can display the user-specified data types. - pg_dump/pg_dumpall can output NCHAR/NVARCHAR columns as-is, not as CHAR/VARCHAR. - To implement additional features for NCHAR/NVARCHAR in the future, as described below. -- Agreed. This is our minimum requirement too. Rgds, Arul Shaji [Optional requirements] -- 1. Implement client driver support, such as: - NCHAR host variable type (e.g. NCHAR var_name[12];) in ECPG, as specified in the SQL standard. - national character methods (e.g. setNString, getNString, setNCharacterStream) as specified in JDBC 4.0. I think at first we can treat these national-character-specific features as the same as CHAR/VARCHAR. 2. NCHAR/NVARCHAR columns can be used in non-UTF-8 databases and always contain Unicode data. I think it is sufficient at first that NCHAR/NVARCHAR columns can only be used in UTF-8 databases and they store UTF-8 strings. This allows us to reuse the input/output/send/recv functions and other infrastructure of CHAR/VARCHAR. This is a reasonable compromise to avoid duplication and minimize the first implementation of NCHAR support. 3. Store strings in UTF-16 encoding in NCHAR/NVARCHAR columns. Fixed-width encoding may allow faster string manipulation as described in Oracle's manual. But I'm not sure about this, because UTF-16 is not a real fixed-width encoding due to supplementary characters. This would definitely be a welcome addition. -- I don't think it is good to implement NCHAR/NVARCHAR types as extensions like contrib/citext, because NCHAR/NVARCHAR are basic types and need client-side support. That is, client drivers need to be aware of the fixed NCHAR/NVARCHAR OID values. How do you think we should implement NCHAR support? Regards MauMau -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription:
Re: [HACKERS] record identical operator
On Tue, Sep 17, 2013 at 8:23 AM, Kevin Grittner kgri...@ymail.com wrote: Of course, that begs the question of whether == is already taken. If not, we could knock one '=' off of everything above except for equals. What existing uses are known for == ? == is already taken as a common typo in plpgsql scripts. I strongly prefer if this remained an error. IF foo == bar THEN ...
Re: [HACKERS] Patch for fail-back without fresh backup
On Tue, Sep 17, 2013 at 3:45 PM, Samrat Revagade revagade.sam...@gmail.com wrote: syncrep.c: In function ‘SyncRepReleaseWaiters’: syncrep.c:421:6: warning: variable ‘numdataflush’ set but not used [-Wunused-but-set-variable] Sorry I forgot fix it. I have attached the patch which I modified. Attached patch combines documentation patch and source-code patch. I set up synchronous replication with synchronous_transfer = all, and then I ran pgbench -i and executed CHECKPOINT in the master. After that, when I executed CHECKPOINT in the standby, it got stuck infinitely. I guess this was cased by synchronous_transfer feature. How does synchronous_transfer work with cascade replication? If it's set to all in the sender-side standby, it can resolve the data page inconsistency between two standbys? Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] insert throw error when year field len 4 for timestamptz datatype
On Tue, 17 September 2013 14:33 Rushabh Lathia wrote: On Mon, Sep 16, 2013 at 7:22 PM, Haribabu kommi haribabu.ko...@huawei.commailto:haribabu.ko...@huawei.com wrote: On 14 August 2013 Rushabh Lathia wrote: postgres=# create table test ( a timestamptz); CREATE TABLE -- Date with year 1000 postgres=# insert into test values ( 'Sat Mar 11 23:58:48 1000 IST'); INSERT 0 1 -- Now try with year 1 it will return error postgres=# insert into test values ( 'Sat Mar 11 23:58:48 1 IST'); ERROR: invalid input syntax for type timestamp with time zone: Sat Mar 11 23:58:48 1 IST LINE 1: insert into test values ( 'Sat Mar 11 23:58:48 1 IST'); Patch applies cleanly to HEAD. As this patch tries to improve in inserting the date of the year value to be more than 4 in length. But it didn't solve all the ways to insert the year field more than 4 in length. Please check the following test. postgres=# insert into test values ('10001010 10:10:10 IST'); INSERT 0 1 postgres=# insert into test values ('100011010 10:10:10 IST'); ERROR: invalid input syntax for type timestamp with time zone: 100011010 10:10:10 IST at character 26 STATEMENT: insert into test values ('100011010 10:10:10 IST'); ERROR: invalid input syntax for type timestamp with time zone: 100011010 10:10:10 IST LINE 1: insert into test values ('100011010 10:10:10 IST'); ^ I feel it is better to provide the functionality of inserting year field more than 4 in length in all flows. +1. Nice catch. Here is the latest version of patch which handles the functionality in all flows. Could you test it and share you comments. I am getting some other failures with the updated patch also, please check the following tests. select date 'January 8, 19990'; select timestamptz 'January 8, 199910 01:01:01 IST'; INSERT INTO TIMESTAMPTZ_TST VALUES(4, '10001 SAT 8 MAR 10:10:10 IST'); you can get the test scripts from regress test files of date.sql, timetz.sql, timestamp.sql and timestamptz.sql and modify according to the patch for verification. I feel changing the year value to accept the length (4) is not simple. So many places the year length crossing more than length 4 is not considered. Search in the code with and correct all related paths. Regards, Hari babu.
Re: [HACKERS] Fix picksplit with nan values
On Mon, Sep 16, 2013 at 4:13 PM, Andrew Gierth and...@tao11.riddles.org.ukwrote: Alexander == Alexander Korotkov aekorot...@gmail.com writes: Alexander 2) NaN coordinates should be processed in GiST index scan Alexander like in sequential scan. postgres=# select * from pts order by a - '(0,0)' limit 10; a -- (1,1) (7,nan) (9,nan) (11,nan) (4,nan) (nan,6) (2,1) (1,2) (2,2) (3,1) (10 rows) postgres=# set enable_indexscan=false; SET postgres=# select * from pts order by a - '(0,0)' limit 10; a --- (1,1) (2,1) (1,2) (2,2) (3,1) (1,3) (3,2) (2,3) (4,1) (1,4) (10 rows) this data set was created by: insert into pts select point(i,j) from (select generate_series(1,100)::float8 union all select 'nan') s1(i), (select generate_series(1,100)::float8 union all select 'nan') s2(j) order by random(); Thanks, Andrew! Good spot. I didn't examine order by operators for work with NaNs. I think this time problem is in GiST itself rather than in opclass. I'm going to fix it in a separate patch. -- With best regards, Alexander Korotkov.
Re: [HACKERS] record identical operator
On 09/17/2013 02:46 PM, Rod Taylor wrote: On Tue, Sep 17, 2013 at 8:23 AM, Kevin Grittner kgri...@ymail.com mailto:kgri...@ymail.com wrote: Of course, that begs the question of whether == is already taken. If not, we could knock one '=' off of everything above except for equals. What existing uses are known for == ? == is already taken as a common typo in plpgsql scripts. I strongly prefer if this remained an error. IF foo == bar THEN ... That was also my reason for not suggesting == . It is too widely used in other systems for simple equality check. -- Hannu Krosing PostgreSQL Consultant Performance, Scalability and High Availability 2ndQuadrant Nordic OÜ
Re: [HACKERS] record identical operator
On 09/17/2013 12:52 AM, Andres Freund wrote: On 2013-09-16 15:26:08 -0700, Kevin Grittner wrote: I can understand claiming that the risk is acceptable but arguing it's not there seems extremly strange to me. It's not a risk. It's why the operator exists. Pft. It's fine if the materialized view code uses it. I don't see the danger there. It's about users discovering it. If they notice it, they will use it because its a crapload faster than normal row comparisons. And deals with NULLs in a simpler way. This is why I suggested naming the operator with four '='s as this should make the user ponder why there are so many equal signs. And there are other languages where more equal signs mean stronger equality. Basically it is equality with possible false negatives. If a user wants to use it for speeding up simple equality, it should be used as WHERE t.* sample AND t.* = sample Perhaps the name of the operator (===) or the fact that I've been using the shorthand description of identical instead of writing out record image equals in these emails has confused you. If you really think that confusing you is the correct description for concerns about users not understanding limitations (which you didn't seem to know about), IIRC he did start with a visible limitation of = being too weak equality for some fields go ahead. Way to go to solicit feedback. I can stop using the shorter description and have absolutely no attachment to the operator name, if that helps. You're unfortunately going to need operators if you want mergejoins to work, right? If not I'd have said name it matview_needs_update() or something like that... But yes, === certainly seems like a bad idea. but having === as an operator form of IS NOT DISTINCT FROM seems like a good idea to me. We will get an operator which most people expect, with sufficiently strange name to make people look up the docs and we will not be violating SQL spec. And it prepares their heads for stronger semantics of :) Greetings, Andres Freund -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Minmax indexes
On Tue, Sep 17, 2013 at 3:30 AM, Thom Brown t...@linux.com wrote: On 17 September 2013 07:20, Jaime Casanova ja...@2ndquadrant.com wrote: On Mon, Sep 16, 2013 at 3:47 AM, Thom Brown t...@linux.com wrote: On 15 September 2013 01:14, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Hi, Here's a reviewable version of what I've dubbed Minmax indexes. Thanks for the patch, but I seem to have immediately hit a snag: pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax (aid); PANIC: invalid xlog record length 0 fwiw, this seems to be triggered by ANALYZE. At least i can trigger it by executing ANALYZE on the table (attached is a stacktrace of a backend exhibiting the failure) I'm able to run ANALYSE manually without it dying: try inserting some data before the ANALYZE, that will force a resumarization which is mentioned in the stack trace of the failure -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157 -- Sent 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 statistics for WAL buffer dirty writes
On Wed, Sep 11, 2013 at 12:43 PM, Satoshi Nagayasu sn...@uptime.jp wrote: (2013/09/10 22:48), Peter Eisentraut wrote: On 9/10/13 3:37 AM, Satoshi Nagayasu wrote: Thanks for checking. Revised one attached. Please fix compiler warning: walwriter.c: In function ‘WalWriterMain’: walwriter.c:293:3: warning: implicit declaration of function ‘pgstat_send_walwriter’ [-Wimplicit-function-declaration] Thanks. Fixed. The patch looks good to me. I have some comments: The description of pg_stat_reset_shared() should mention pg_stat_walwriter in the document. We should implment something like pg_stat_reset_shared('all') so that we can easily reset all cluster-wide statistics counters to zero? Some background workers may write WAL because WAL buffer is full. So you seem to need to change those processes so that they also can increase the xlog_dirty_writes counter. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Freezing without write I/O
On 9/16/13 9:59 AM, Heikki Linnakangas wrote: Here's a rebased version of the patch, including the above-mentioned fixes. Nothing else new. It still fails to apply. You might need to rebase it again. -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Minmax indexes
On 17 September 2013 14:37, Jaime Casanova ja...@2ndquadrant.com wrote: On Tue, Sep 17, 2013 at 3:30 AM, Thom Brown t...@linux.com wrote: On 17 September 2013 07:20, Jaime Casanova ja...@2ndquadrant.com wrote: On Mon, Sep 16, 2013 at 3:47 AM, Thom Brown t...@linux.com wrote: On 15 September 2013 01:14, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Hi, Here's a reviewable version of what I've dubbed Minmax indexes. Thanks for the patch, but I seem to have immediately hit a snag: pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax (aid); PANIC: invalid xlog record length 0 fwiw, this seems to be triggered by ANALYZE. At least i can trigger it by executing ANALYZE on the table (attached is a stacktrace of a backend exhibiting the failure) I'm able to run ANALYSE manually without it dying: try inserting some data before the ANALYZE, that will force a resumarization which is mentioned in the stack trace of the failure I've tried inserting 1 row then ANALYSE and 10,000 rows then ANALYSE, and in both cases there's no error. But then trying to create the index again results in my original error. -- Thom
Re: [HACKERS] Freezing without write I/O
On 2013-09-17 09:41:47 -0400, Peter Eisentraut wrote: On 9/16/13 9:59 AM, Heikki Linnakangas wrote: Here's a rebased version of the patch, including the above-mentioned fixes. Nothing else new. It still fails to apply. You might need to rebase it again. FWIW, I don't think it's too important that this specific patch applies all the time. From the look I had and the discussions onlist, what it needs so far is mostly architecture review and such. 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] logical changeset generation v6
On 9/15/13 11:30 AM, Andres Freund wrote: On 2013-09-15 11:20:20 -0400, Peter Eisentraut wrote: On Sat, 2013-09-14 at 22:49 +0200, Andres Freund wrote: Attached you can find the newest version of the logical changeset generation patchset. You probably have bigger things to worry about, but please check the results of cpluspluscheck, because some of the header files don't include header files they depend on. Hm. I tried to get that right, but it's been a while since I last checked. I don't regularly use cpluspluscheck because it doesn't work in VPATH builds... We really need to fix that. I'll push a fix for that to the git tree, don't think that's worth a resend in itself. This patch set now fails to apply because of the commit Rename various freeze multixact variables. -- Sent 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_stat_statements: calls under-estimation propagation
You seem to have forgotten to include the pg_stat_statements--1.2.sql and pg_stat_statements--1.1--1.2.sql in the patch. Sorry again. Please find updated patch attached. http://www.postgresql.org/mailpref/pgsql-hackers NAMLhttp://postgresql.1045698.n5.nabble.com/template/NamlServlet.jtp?macro=macro_viewerid=instant_html%21nabble%3Aemail.namlbase=nabble.naml.namespaces.BasicNamespace-nabble.view.web.template.NabbleNamespace-nabble.view.web.template.NodeNamespacebreadcrumbs=notify_subscribers%21nabble%3Aemail.naml-instant_emails%21nabble%3Aemail.naml-send_instant_email%21nabble%3Aemail.naml On Tue, Sep 17, 2013 at 5:47 PM, Fujii Masao-2 [via PostgreSQL] ml-node+s1045698n5771213...@n5.nabble.com wrote: On Sun, Sep 15, 2013 at 3:54 PM, samthakur74 [hidden email]http://user/SendEmail.jtp?type=nodenode=5771213i=0 wrote: You have added this email to the commit fest, but it contains no patch. Please add the email with the actual patch. I hope its attached now! You seem to have forgotten to include the pg_stat_statements--1.2.sql and pg_stat_statements--1.1--1.2.sql in the patch. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list ([hidden email]http://user/SendEmail.jtp?type=nodenode=5771213i=1) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers -- If you reply to this email, your message will be added to the discussion below: http://postgresql.1045698.n5.nabble.com/pg-stat-statements-calls-under-estimation-propagation-tp5738128p5771213.html To unsubscribe from pg_stat_statements: calls under-estimation propagation, click herehttp://postgresql.1045698.n5.nabble.com/template/NamlServlet.jtp?macro=unsubscribe_by_codenode=5738128code=c2FtdGhha3VyNzRAZ21haWwuY29tfDU3MzgxMjh8ODM4MzYxMTcy . NAMLhttp://postgresql.1045698.n5.nabble.com/template/NamlServlet.jtp?macro=macro_viewerid=instant_html%21nabble%3Aemail.namlbase=nabble.naml.namespaces.BasicNamespace-nabble.view.web.template.NabbleNamespace-nabble.view.web.template.NodeNamespacebreadcrumbs=notify_subscribers%21nabble%3Aemail.naml-instant_emails%21nabble%3Aemail.naml-send_instant_email%21nabble%3Aemail.naml pg_stat_statements-identification-v4.patch.gz (9K) http://postgresql.1045698.n5.nabble.com/attachment/5771248/0/pg_stat_statements-identification-v4.patch.gz -- View this message in context: http://postgresql.1045698.n5.nabble.com/pg-stat-statements-calls-under-estimation-propagation-tp5738128p5771248.html Sent from the PostgreSQL - hackers mailing list archive at Nabble.com.
Re: [HACKERS] record identical operator
Kevin Grittner kgri...@ymail.com writes: = equals (but doesn't necessarily look the same as) === IS NOT DISTINCT FROM as an operator reserved for has no user-visible differences from = stored image is the same I understand the need for more than one equality operator and my programming language of choice exposes eq, eql, equal and equalp for solving a very similar situation. Oh, and also offers = and string= and string-equal, among others. My vote goes against using a different number of the same character to name the operators though, as that's not visually helpful enough IMO. In yet another language whose grammar took inspiration with Prolog, dealing with binary is made with explicitely using the bit syntax expression where any datum is boxed into and . http://erlang.org/doc/reference_manual/expressions.html#id78513 It might not be practical to do the same in SQL and have either the following syntax example or even a unary bit representation operator to do the same: SELECT row(expression, here) = column_name … That would probably prevent using indexes? It might be inspiration to name the bit-comparison operator something like = though, or == or something else. If only we could force the usage of unicode for the SQL input itself… I'm sure there's a unicode glyph perfect for what we want here… About IS NOT DISTINCT FROM, I would propose something where NOT and DISTINCT are kept as concepts. Not usually is expressed with ! and we already have a bunch of same as or matches operators with ~, so: ~ IS DISTINCT FROM !~ IS NOT DISTINCT FROM Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Re: Proposal/design feedback needed: WITHIN GROUP (sql standard ordered set aggregate functions)
On Sat, Sep 14, 2013 at 7:23 AM, Andrew Gierth and...@tao11.riddles.org.uk wrote: Peter Please fix compiler warnings: Someone should do the same in WaitForBackgroundWorkerStartup so that building with -Werror works. I don't get a warning there. Can you be more specific about the problem? -- 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] unaccent module - two params function should be immutable
On Sat, Sep 14, 2013 at 9:42 AM, Pavel Stehule pavel.steh...@gmail.com wrote: I have developed the attached patch based on your suggestion. I did not see anything in the code that would make it STABLE, except a lookup of a dictionary library: dictOid = get_ts_dict_oid(stringToQualifiedNameList(unaccent), false); yes, it risk, but similar is with timezones, and other external data. That's a catalog lookup - not a reliance on external data. -- 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] output/security_label.source referring to abs_builddir instead of libdir
In make check, there are 4 shared libraries that are loaded for test cases from the src/test/regress folder. Whereas, output of other libraries contain @libdir@ tag, the output file for security_label (dumm_seclabel) contains a tag for @abs_builddir@ in the output/security_label.source file. I believe this an issue, and rather than referring @abs_builddir@, it should instead contain the tag @libdir@. Also note that while I could provide a path for dynamic libraries via dlpath option of pg_regress, this is not honored by security_label test case. PFA patch for this fix. Regards. -- * * *Hamid Quddus Akhtar* Configuration Management Team Ph: +92.333.544.9950 Skype ID: EngineeredVirus www.enterprisedb.co http://www.enterprisedb.com/mhttp://www.enterprisedb.com/ * Follow us on Twitter* @EnterpriseDB Visit EnterpriseDB for tutorials, webinars, whitepapershttp://www.enterprisedb.com/resources-community and more http://www.enterprisedb.com/resources-community security_label.source.output.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] json docs fixup
On Sat, Sep 14, 2013 at 3:23 PM, Andrew Dunstan and...@dunslane.net wrote: While writing slides for pgopen next week, I noticed that the JSON docs on json_populate_record and json_populate_recordset contain this sentence: A column may only be specified once. IIRC we removed that restriction during development, so unless there is a squawk I am going to simply remove that sentence where it occurs. Or should we say something like: If a column is specified more than once, the last value is used. If the latter statement is accurate, I favor including it, versus saying nothing on the topic. -- 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] [RFC] Extend namespace of valid guc names
On Sat, Sep 14, 2013 at 5:21 PM, Andres Freund and...@2ndquadrant.com wrote: a) allow repeatedly qualified names like a.b.c The attached patch does a) What is the use case for this change? -- 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] [RFC] Extend namespace of valid guc names
On 2013-09-17 10:23:30 -0400, Robert Haas wrote: On Sat, Sep 14, 2013 at 5:21 PM, Andres Freund and...@2ndquadrant.com wrote: a) allow repeatedly qualified names like a.b.c The attached patch does a) What is the use case for this change? Check http://archives.postgresql.org/message-id/20130225213400.GF3849%40awork2.anarazel.de or the commit message of the patch. 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] pg_system_identifier()
On 2013-09-17 10:57:46 -0400, Robert Haas wrote: On Mon, Sep 16, 2013 at 1:25 AM, Satoshi Nagayasu sn...@uptime.jp wrote: How about adding new system view with new function which returns a single pg_controldata value in text type, and using a cast for each column in the view definition? CREATE VIEW pg_catalog.pg_controldata AS SELECT pg_controldata('control_version')::integer AS control_version, pg_controldata('catalog_version')::integer AS catalog_version, pg_controldata('system_identifier')::bigint AS system_identifier, ... pg_controldata('next_xlog_file')::char(25) AS next_xlog_file, ... pg_controldata('encoding')::text AS encoding; Given that the view can work like a SRF, and it allows us to retrieve all the values of pg_controldata with appropriate types in single record from the view: I like this idea. I think having an easy way to get the values with the right types will be a plus. But adding a separate function for each field seems excessive, so I think this is a good compromise. Why not add a single function returning a composite type then? That'd at least have a chance of returning consistent values for the individual values that change during runtime. It would also produce proper errors when you load a view using columns that don't exist anymore instead of just at runtime. 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] proposal: simple date constructor from numeric values
Pavel Stehule escribió: fixed - see attached patch There's a typo tange in some error messages, which has found its way to the regression tests. -- Álvaro Herrerahttp://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
[HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Re: [HACKERS] Is it necessary to rewrite table while increasing the scale of datatype numeric?
On Sun, Sep 15, 2013 at 8:05 PM, Jeff Janes jeff.ja...@gmail.com wrote: But note that the current behavior is worse in this regard. If you specify a scale of 4 at the column level, than it is not possible to distinguish between 5.000 and 5. on a per-value basis within that column. If the scale at the column level was taken as the maximum scale, not the only allowed one, then that distinction could be recorded. That behavior seems more sensible to me (metrologically speaking, regardless of alter table performance aspects), but I don't see how to get there from here with acceptable compatibility breakage. I think I'd probably agree with that in a green field, but as you say, I can't see accepting the backward compatibility break at this point. After all, you can get variable-precision in a single column by declaring it as unqualified numeric. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_system_identifier()
On Mon, Sep 16, 2013 at 1:25 AM, Satoshi Nagayasu sn...@uptime.jp wrote: How about adding new system view with new function which returns a single pg_controldata value in text type, and using a cast for each column in the view definition? CREATE VIEW pg_catalog.pg_controldata AS SELECT pg_controldata('control_version')::integer AS control_version, pg_controldata('catalog_version')::integer AS catalog_version, pg_controldata('system_identifier')::bigint AS system_identifier, ... pg_controldata('next_xlog_file')::char(25) AS next_xlog_file, ... pg_controldata('encoding')::text AS encoding; Given that the view can work like a SRF, and it allows us to retrieve all the values of pg_controldata with appropriate types in single record from the view: I like this idea. I think having an easy way to get the values with the right types will be a plus. But adding a separate function for each field seems excessive, so I think this is a good compromise. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_stat_statements: calls under-estimation propagation
On Tue, Sep 17, 2013 at 10:48 PM, samthakur74 samthaku...@gmail.com wrote: You seem to have forgotten to include the pg_stat_statements--1.2.sql and pg_stat_statements--1.1--1.2.sql in the patch. Sorry again. Please find updated patch attached. pg_stat_statements--1.2.sql is missing. Could you confirm that the patch includes all the changes? Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_system_identifier()
On Tue, Sep 17, 2013 at 10:59 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-09-17 10:57:46 -0400, Robert Haas wrote: On Mon, Sep 16, 2013 at 1:25 AM, Satoshi Nagayasu sn...@uptime.jp wrote: How about adding new system view with new function which returns a single pg_controldata value in text type, and using a cast for each column in the view definition? CREATE VIEW pg_catalog.pg_controldata AS SELECT pg_controldata('control_version')::integer AS control_version, pg_controldata('catalog_version')::integer AS catalog_version, pg_controldata('system_identifier')::bigint AS system_identifier, ... pg_controldata('next_xlog_file')::char(25) AS next_xlog_file, ... pg_controldata('encoding')::text AS encoding; Given that the view can work like a SRF, and it allows us to retrieve all the values of pg_controldata with appropriate types in single record from the view: I like this idea. I think having an easy way to get the values with the right types will be a plus. But adding a separate function for each field seems excessive, so I think this is a good compromise. Why not add a single function returning a composite type then? That'd at least have a chance of returning consistent values for the individual values that change during runtime. It would also produce proper errors when you load a view using columns that don't exist anymore instead of just at runtime. Hmm. Yeah, that might be better. -- 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] Minmax indexes
On Tue, Sep 17, 2013 at 8:43 AM, Thom Brown t...@linux.com wrote: On 17 September 2013 14:37, Jaime Casanova ja...@2ndquadrant.com wrote: On Tue, Sep 17, 2013 at 3:30 AM, Thom Brown t...@linux.com wrote: On 17 September 2013 07:20, Jaime Casanova ja...@2ndquadrant.com wrote: On Mon, Sep 16, 2013 at 3:47 AM, Thom Brown t...@linux.com wrote: On 15 September 2013 01:14, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Hi, Here's a reviewable version of what I've dubbed Minmax indexes. Thanks for the patch, but I seem to have immediately hit a snag: pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax (aid); PANIC: invalid xlog record length 0 fwiw, this seems to be triggered by ANALYZE. At least i can trigger it by executing ANALYZE on the table (attached is a stacktrace of a backend exhibiting the failure) I'm able to run ANALYSE manually without it dying: try inserting some data before the ANALYZE, that will force a resumarization which is mentioned in the stack trace of the failure I've tried inserting 1 row then ANALYSE and 10,000 rows then ANALYSE, and in both cases there's no error. But then trying to create the index again results in my original error. Ok So, please confirm if this is the pattern you are following: CREATE TABLE t1(i int); INSERT INTO t1 SELECT generate_series(1, 1); CREATE INDEX idx1 ON t1 USING minmax (i); if that, then the attached stack trace (index_failure_thom.txt) should correspond to the failure you are looking. My test was slightly different: CREATE TABLE t1(i int); CREATE INDEX idx1 ON t1 USING minmax (i); INSERT INTO t1 SELECT generate_series(1, 1); ANALYZE t1; and the failure happened in a different time, in resumarization (attached index_failure_jcm.txt) but in the end, both failures seems to happen for the same reason: a record of length 0... at XLogInsert time #4 XLogInsert at xlog.c:966 #5 mmSetHeapBlockItemptr at mmrevmap.c:169 #6 mm_doinsert at minmax.c:1410 actually, if you create a temp table both tests works fine -- Jaime Casanova www.2ndQuadrant.com Professional PostgreSQL: Soporte 24x7 y capacitación Phone: +593 4 5107566 Cell: +593 987171157 Program received signal SIGABRT, Aborted. 0x7f4428819475 in *__GI_raise (sig=optimized out) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 64 ../nptl/sysdeps/unix/sysv/linux/raise.c: No existe el fichero o el directorio. (gdb) bt #0 0x7f4428819475 in *__GI_raise (sig=optimized out) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 #1 0x7f442881c6f0 in *__GI_abort () at abort.c:92 #2 0x0076d6ac in errfinish (dummy=dummy@entry=0) at elog.c:546 #3 0x0076fdca in elog_finish (elevel=elevel@entry=22, fmt=fmt@entry=0x7d2aa7 invalid xlog record length %u) at elog.c:1304 #4 0x004e1eba in XLogInsert (rmid=rmid@entry=17 '\021', info=info@entry=48 '0', rdata=rdata@entry=0x7fffc836ea10) at xlog.c:966 #5 0x004a9bb9 in mmSetHeapBlockItemptr (rmAccess=rmAccess@entry=0x1b3af38, heapBlk=heapBlk@entry=0, blkno=blkno@entry=6, offno=offno@entry=1) at mmrevmap.c:169 #6 0x004a73e8 in mm_doinsert (idxrel=0x7f4429054c88, rmAccess=0x1b3af38, buffer=buffer@entry=0x7f441f7e718c, heapblkno=0, tup=tup@entry=0x7f441f84cff8, itemsz=16) at minmax.c:1410 #7 0x004a9464 in rerun_summarization (numnonsummarized=22, nonsummarized=0x1b3a408, rmAccess=0x1b3af38, heapRel=0x7f4429052e68, idxRel=0x7f4429054c88) at minmax.c:1205 #8 mmvacuumcleanup (fcinfo=optimized out) at minmax.c:1268 #9 0x0077388f in FunctionCall2Coll (flinfo=flinfo@entry=0x7fffc836f0a0, collation=collation@entry=0, arg1=arg1@entry=140736552432368, arg2=arg2@entry=0) at fmgr.c:1326 #10 0x004a6c2d in index_vacuum_cleanup (info=info@entry=0x7fffc836f2f0, stats=stats@entry=0x0) at indexam.c:715 #11 0x005570d1 in do_analyze_rel (onerel=onerel@entry=0x7f4429052e68, acquirefunc=0x556020 acquire_sample_rows, relpages=45, inh=inh@entry=0 '\000', elevel=elevel@entry=13, vacstmt=error reading variable: Unhandled dwarf expression opcode 0xfa, vacstmt=error reading variable: Unhandled dwarf expression opcode 0xfa) at analyze.c:634 #12 0x00557fef in analyze_rel (relid=relid@entry=16384, vacstmt=vacstmt@entry=0x1af5678, bstrategy=optimized out) at analyze.c:267 ---Type return to continue, or q return to quit--- #13 0x005aa224 in vacuum (vacstmt=vacstmt@entry=0x1af5678, relid=relid@entry=0, do_toast=do_toast@entry=1 '\001', bstrategy=optimized out, bstrategy@entry=0x0, for_wraparound=for_wraparound@entry=0 '\000', isTopLevel=isTopLevel@entry=1 '\001') at vacuum.c:249 #14 0x0069f417 in standard_ProcessUtility (parsetree=0x1af5678, queryString=optimized out, context=optimized out, params=0x0, dest=optimized out, completionTag=optimized out) at utility.c:682 #15 0x0069c587 in PortalRunUtility
Re: [HACKERS] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE
On Sat, Sep 14, 2013 at 6:27 PM, Peter Geoghegan p...@heroku.com wrote: Note that today there is no guarantee that the original waiter for a duplicate-inserting xact to complete will be the first one to get a second chance, so I think it's hard to question this on correctness grounds. Even if they are released in FIFO order, there is no reason to assume that the first waiter will win the race with a second. Most obviously, the second waiter may not even ever get the chance to block on the same xid at all (so it's not really a waiter at all) and still be able to insert, if the blocking-xact aborts after the second waiter starts its descent but before it checks uniqueness. All this, even though the second waiter arrived maybe minutes after the first. ProcLockWakeup() only wakes as many waiters from the head of the queue as can all be granted the lock without any conflicts. So I don't think there is a race condition in that path. So far it's been a slippery slope type argument that can be equally well used to argue against some facet of almost any substantial patch ever proposed. I don't completely agree with that characterization, but you do have a point. Obviously, if the differences in the area of interruptibility, starvation, deadlock risk, etc. can be made small enough relative to the status quo can be made small enough, then those aren't reasons to reject the approach. But I'm skeptical that you're going to be able to accomplish that, especially without adversely affecting maintainability. I think the way that you're proposing to use lwlocks here is sufficiently different from what the rest of the system does that it's going to be hard to avoid system-wide affects that can't easily be caught during code review; and like Andres, I don't share your skepticism about alternative approaches. For that matter, if the table has more than MAX_SIMUL_LWLOCKS indexes, you'll error out. In fact, if you get the number of indexes exactly right, you'll exceed MAX_SIMUL_LWLOCKS in visibilitymap_clear() and panic the whole system. Oh, come on. We can obviously engineer a solution to that problem. I don't think I've ever seen a table with close to 100 *unique* indexes. 4 or 5 is a very high number. If we just raised on error if someone tried to do this with more than 10 unique indexes, I would guess that'd we'd get exactly zero complaints about it. That's not a solution; that's a hack. Undetected deadlock is really not much worse than detected deadlock here. Either way, it's a bug. And it's something that any kind of implementation will need to account for. It's not okay to *unpredictably* deadlock, in a way that the user has no control over. Today, someone can do an analysis of their application and eliminate deadlocks if they need to. That might not be terribly practical much of the time, but it can be done. It certainly is practical to do it in a localized way. I wouldn't like to compromise that. I agree that unpredictable deadlocks are bad. I think the fundamental problem with UPSERT, MERGE, and this proposal is what happens when the conflicting tuple is present but not visible to your scan, either because it hasn't committed yet or because it has committed but is not visible to your snapshot. I'm not clear on how you handle that in your approach. If you look at the code, you'll see that I've made very modest modifications to LWLockRelease only. I would be extremely surprised if the overhead was not only in the noise, but was completely impossible to detect through any conventional benchmark. These are the same kind of very modest changes made for LWLockAcquireOrWait(), and you said nothing about that at the time. Despite the fact that you now appear to think that that whole effort was largely a waste of time. Well, I did have some concerns about the performance impact of that patch: http://www.postgresql.org/message-id/CA+TgmoaPyQKEaoFz8HkDGvRDbOmRpkGo69zjODB5=7jh3hb...@mail.gmail.com I also discovered, after it was committed, that it didn't help in the way I expected: http://www.postgresql.org/message-id/CA+TgmoY8P3sD=ouvig+xzjmzk5-phunv39rtfyzuqxu8hjt...@mail.gmail.com It's true that I didn't raise those concerns contemporaneously with the commit, but I didn't understand the situation well enough at that time to realize how narrow the benefit was. I've wished, on a number of occasions, to be able to add more lwlock primitives. The problem with that is that if everybody does it, we'll pretty soon end up with a mess. I attempted to address that with this proposal: http://www.postgresql.org/message-id/ca+tgmob4ye_k5dpo0t07pnf1sokpybo+wj4m4fryos7z4_y...@mail.gmail.com ...but nobody (including me) was very sure that was the right way forward, and it never went anywhere. However, I think the basic issue remains. I was sad to discover last week that Heikki handled this problem for the WAL scalability patch by basically copy-and-pasting much of the lwlock
Re: [HACKERS] [PERFORM] encouraging index-only scans
On 9/7/13 12:34 AM, Andres Freund wrote: What I was thinking of was to keep track of the oldest xids on pages that cannot be marked all visible. I haven't thought about the statistics part much, but what if we binned the space between [RecentGlobalXmin, -nextXid) into 10 bins and counted the number of pages falling into each bin. Then after the vacuum finished we could compute how far RecentGlobalXmin would have to progress to make another vacuum worthwile by counting the number of pages from the lowest bin upwards and use the bin's upper limit as the triggering xid. Now, we'd definitely need to amend that scheme by something that handles pages that are newly written to, but it seems something like that wouldn't be too hard to implement and would make autovacuum more useful. If we're binning by XID though you're still dependent on scanning to build that range. Anything that creates dead tuples will also be be problematic, because it's going to unset VM bits on you, and you won't know if it's due to INSERTS or dead tuples. What if we maintained XID stats for ranges of pages in a separate fork? Call it the XidStats fork. Presumably the interesting pieces would be min(xmin) and max(xmax) for pages that aren't all visible. If we did that at a granularity of, say, 1MB worth of pages[1] we're talking 8 bytes per MB, or 1 XidStats page per GB of heap. (Worst case alignment bumps that up to 2 XidStats pages per GB of heap.) Having both min(xmin) and max(xmax) for a range of pages would allow for very granular operation of vacuum. Instead of hitting every heap page that's not all-visible, it would only hit those that are not visible and where min(xmin) or max(xmax) were less than RecentGlobalXmin. One concern is maintaining this data. A key point is that we don't have to update it every time it changes; if the min/max are only off by a few hundred XIDs there's no point to updating the XidStats page. We'd obviously need the XidStats page to be read in, but even a 100GB heap would be either 100 or 200 XidStats pages. [1]: There's a trade-off between how much space we 'waste' on XidStats pages and how many heap pages we potentially have to scan in the range. We'd want to see what this looked like in a real system. The thing that helps here is that regardless of what the stats for a particular heap range are, you're not going to scan any pages in that range that are already all-visible. -- Jim C. Nasby, Data Architect j...@nasby.net 512.569.9461 (cell) http://jim.nasby.net -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] relscan_details.h
On Mon, Sep 16, 2013 at 11:02:28PM -0300, Alvaro Herrera wrote: relscan.h is a bit of an unfortunate header because it requires a lot of other headers to compile, and is also required by execnodes.h. This Not in my copy of the source tree. execnodes.h uses the corresponding typedefs that appear in genam.h and heapam.h. Indeed, find . -name '*.Po' | xargs grep -l relscan.h | wc -l only locates 26 files that include relscan.h directly or indirectly. quick patch removes the struct definitions from that file, moving them into a new relscan_details.h file; the reworked relscan.h does not need any other header, freeing execnodes.h from needlessly including lots of unnecessary stuff all over the place. Only the files that really need the struct definition include the new file, and as seen in the patch, they are quite few. execnodes.h is included by 147 backend source files; relscan_details.h only 27. So the exposure area is reduced considerably. This patch also modifies a few other backend source files to add one or both of genam.h and heapam.h, which were previously included through execnodes.h. This is probably missing tweaks for contrib/. The following modules would need to include relscan_details.h: sepgsql dblink pgstattuple pgrowlocks. I expect the effect on third-party modules to be much less than by the htup_details.h change. -1 on header restructuring in the absence of noteworthy compile-time benchmark improvements. Besides the obvious benchmark of full-build time, one could exercise the benefit of fewer header dependencies by modelling the series of compile times from running git pull; make at each commit for the past several months. The latter benchmark is perhaps more favorable. nm -- Noah Misch EnterpriseDB http://www.enterprisedb.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] Freezing without write I/O
Heikki Linnakangas escribió: Here's a rebased version of the patch, including the above-mentioned fixes. Nothing else new. Nice work. I apologize for the conflicts I created yesterday. I would suggest renaming varsup_internal.h to varsup_xlog.h. You added a FIXME comment to HeapTupleHeaderIsOnlyLocked. I think the answer to the question is yes, because checking for locked might incur examining the Xmax of tuples, which cannot be done if the page is mature; perhaps the check for maturity needs to happen only after the infomask has been checked. The new stuff in GetNewTransactionId() is said to be very expensive (it might involve grabbing the ProcArrayLock and scanning the whole procarray, for example.) There's a comment about having this be done only in other processes, and I think that's a good idea, otherwise we risk adding a lot of latency, randomly, to client-connected processes which might be latency sensitive. I think autovacuum is not a good choice however (it might not even be running). How about using the bgwriter or walwriter for this? As far as I can tell, there's no need for this process to actually be able to run transactions or scan catalogs; the ability to lock and scan ProcArray appears sufficient. Using a RTC (instead of the Xid counter % 128) to determine when to run this doesn't look like a problem to me. Something that sleeps for too long might be, so we would need to ensure that it happens timely. Not sure what's an appropriate time for this, however. Another option is have backends check the Xid counter, and every 128 ticks set a flag in shmem so that the background process knows to execute the switch. heap_freeze_tuple() receives one Xid and one MultiXactId; they can be passed as Invalid to mean forced freezing. Do we really need to offer the possibility of freezing xids but not multis, and multis but not xids? From a conceptual PoV, it seems to make more sense to me to pass a boolean force meaning freeze everything, and ignore the numerical values. The greatest risk in this patch is the possibility that some heap_freeze_page() call is forgotten. I think you covered all cases in heapam.c. -- Álvaro Herrerahttp://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] Re: Proposal/design feedback needed: WITHIN GROUP (sql standard ordered set aggregate functions)
Robert == Robert Haas robertmh...@gmail.com writes: Someone should do the same in WaitForBackgroundWorkerStartup so that building with -Werror works. Robert I don't get a warning there. Can you be more specific about Robert the problem? bgworker.c: In function 'WaitForBackgroundWorkerStartup': bgworker.c:866: warning: 'pid' may be used uninitialized in this function gcc 4.2.2 / freebsd 8.2 -- 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] relscan_details.h
Noah Misch wrote: On Mon, Sep 16, 2013 at 11:02:28PM -0300, Alvaro Herrera wrote: relscan.h is a bit of an unfortunate header because it requires a lot of other headers to compile, and is also required by execnodes.h. This Not in my copy of the source tree. execnodes.h uses the corresponding typedefs that appear in genam.h and heapam.h. Indeed, find . -name '*.Po' | xargs grep -l relscan.h | wc -l only locates 26 files that include relscan.h directly or indirectly. Ah, I see now that I misspoke. This changeset has been dormant on my repo for a while, so I confused the detail of what it is about. The real benefit of this change is to eliminate indirect inclusion of genam.h and heapam.h. The number of indirect inclusions of each of them is: HEADwith patch heapam.h219 118 genam.h 226 121 -1 on header restructuring in the absence of noteworthy compile-time benchmark improvements. Besides the obvious benchmark of full-build time, one could exercise the benefit of fewer header dependencies by modelling the series of compile times from running git pull; make at each commit for the past several months. The latter benchmark is perhaps more favorable. I will have a go at measuring things this way. -- Álvaro Herrerahttp://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: add MAP_HUGETLB to mmap() where supported (WIP)
On Mon, Sep 16, 2013 at 9:13 AM, Heikki Linnakangas hlinnakan...@vmware.com wrote: Robert, do you remember why you put the pagesize = sysconf(_SC_PAGE_SIZE); call in the new mmap() shared memory allocator? Hmm, no. Unfortunately, I don't. We could try ripping it out and see if the buildfarm breaks. If it is needed, then the dynamic shared memory patch I posted probably needs it as well. -- 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] relscan_details.h
On Tue, Sep 17, 2013 at 2:30 PM, Alvaro Herrera alvhe...@2ndquadrant.com wrote: -1 on header restructuring in the absence of noteworthy compile-time benchmark improvements. Besides the obvious benchmark of full-build time, one could exercise the benefit of fewer header dependencies by modelling the series of compile times from running git pull; make at each commit for the past several months. The latter benchmark is perhaps more favorable. I will have a go at measuring things this way. Personally, I'm not particularly in favor of these kinds of changes. The changes we made last time broke a lot of extensions - including some proprietary EDB ones that I had to go fix. I think a lot of people spent a lot of time fixing broken builds, at EDB and elsewhere, as well as rebasing patches. And the only benefit we have to balance that out is that incremental recompiles are faster, and I'm not really sure how important that actually is. On my system, configure takes 25 seconds and make -j3 takes 65 seconds; so, even a full recompile is pretty darn fast, and an incremental recompile is usually really fast. Now granted this is a relatively new system, but still. I don't want to be too dogmatic in opposing this; I accept that we should, from time to time, refactor things. If we don't, superflouous dependencies will probably proliferate over time. But personally, I'd rather do these changes in bulk every third release or so and keep them to a minimum in between times, so that we're not forcing people to constantly decorate their extensions with new #if directives. -- 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] plpgsql.print_strict_params
Hi, Attached is a patch with the following changes: On 16/09/2013 10:57, I wrote: On 9/16/13 8:04 AM, Ian Lawrence Barwick wrote: However the sample function provided in the documentation throws a runtime error due to a missing FROM-clause entry. Ugh. I'll look into fixing that. Fixed. This lineis in exec_get_query_params() and exec_get_dynquery_params(): return (no parameters); Presumably the message will escape translation and this line should be better written as: return _((no parameters)); Nice catch. Will look into this. Another option would be to simply omit the DETAIL line if there were no parameters. At this very moment I'm thinking that might be a bit nicer. Decided to remove the DETAIL line in these cases. Also, if the offending query parameter contains a single quote, the output is not escaped: postgres=# select get_userid(E'foo'''); ERROR: query returned more than one row DETAIL: p1 = 'foo'' CONTEXT: PL/pgSQL function get_userid(text) line 9 at SQL statement Not sure if that's a real problem though. Hmm.. I should probably look at what happens when the parameters to a prepared statement are currently logged and imitate that. Yup, they're escaped. Did just that. Also copied the parameters: prefix on the DETAIL line from there. The functions added in pl_exec.c - exec_get_query_params() and exec_get_dynquery_params() do strike me as potentially misnamed, as they don't actually execute anything but are more utility functions for generating formatted output. Maybe format_query_params() etc. would be better? Agreed, they could use some renaming. I went with format_expr_params and format_preparedparamsdata. * Are the comments sufficient and accurate? exec_get_query_params() and exec_get_dynquery_params() could do with some brief comments explaining their purpose. Agreed. Still agreeing with both of us, added a comment each. I also added the new keywords to the unreserved_keywords production, as per the instructions near the beginning of pl_gram.y. Regards, Marko Tiikkaja *** a/doc/src/sgml/plpgsql.sgml --- b/doc/src/sgml/plpgsql.sgml *** *** 1077,1082 END; --- 1077,1106 /para para + If literalprint_strict_params/ is enabled for the function, + you will get information about the parameters passed to the + query in the literalDETAIL/ part of the error message produced + when the requirements of STRICT are not met. You can change this + setting on a system-wide basis by setting + varnameplpgsql.print_strict_params/, though only subsequent + function compilations will be affected. You can also enable it + on a per-function basis by using a compiler option: + programlisting + CREATE FUNCTION get_userid(username text) RETURNS int + AS $$ + #print_strict_params on + DECLARE + userid int; + BEGIN + SELECT users.userid INTO STRICT userid + FROM users WHERE users.username = get_userid.username; + RETURN userid; + END + $$ LANGUAGE plpgsql; + /programlisting + /para + + para For commandINSERT//commandUPDATE//commandDELETE/ with literalRETURNING/, applicationPL/pgSQL/application reports an error for more than one returned row, even when *** a/src/pl/plpgsql/src/pl_comp.c --- b/src/pl/plpgsql/src/pl_comp.c *** *** 351,356 do_compile(FunctionCallInfo fcinfo, --- 351,357 function-fn_cxt = func_cxt; function-out_param_varno = -1; /* set up for no OUT param */ function-resolve_option = plpgsql_variable_conflict; + function-print_strict_params = plpgsql_print_strict_params; if (is_dml_trigger) function-fn_is_trigger = PLPGSQL_DML_TRIGGER; *** *** 847,852 plpgsql_compile_inline(char *proc_source) --- 848,854 function-fn_cxt = func_cxt; function-out_param_varno = -1; /* set up for no OUT param */ function-resolve_option = plpgsql_variable_conflict; + function-print_strict_params = plpgsql_print_strict_params; plpgsql_ns_init(); plpgsql_ns_push(func_name); *** a/src/pl/plpgsql/src/pl_exec.c --- b/src/pl/plpgsql/src/pl_exec.c *** *** 221,226 static Portal exec_dynquery_with_params(PLpgSQL_execstate *estate, --- 221,231 PLpgSQL_expr *dynquery, List *params, const char *portalname, int cursorOptions); + static char *format_expr_params(PLpgSQL_execstate *estate, + const PLpgSQL_expr *expr); + static char *format_preparedparamsdata(PLpgSQL_execstate *estate, + const PreparedParamsData *ppd); + /* -- * plpgsql_exec_function Called by the call handler for *** *** 3391,3408
Re: [HACKERS] [RFC] Extend namespace of valid guc names
On Tue, Sep 17, 2013 at 10:27 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-09-17 10:23:30 -0400, Robert Haas wrote: On Sat, Sep 14, 2013 at 5:21 PM, Andres Freund and...@2ndquadrant.com wrote: a) allow repeatedly qualified names like a.b.c The attached patch does a) What is the use case for this change? Check http://archives.postgresql.org/message-id/20130225213400.GF3849%40awork2.anarazel.de or the commit message of the patch. That's more or less what I figured. One thing I'm uncomfortable with is that, while this is useful for extensions, what do we do when we have a similar requirement for core? The proposed GUC name is of the format extension.user_defined_object_name.property_name; but omitting the extension name would lead to user_defined_object_name.property_name, which doesn't feel good as a method for naming core GUCs. The user defined object name might just so happen to be an extension name. More generally, it seems like this is trying to take structured data and fit into the GUC mechanism, and I'm not sure that's going to be a real good fit. I mean, pg_hba.conf could be handled this way. You could have hba.1.type = local, hba.1.database = all, hba.1.user = all, hba.2.type = host, etc. But I doubt that any of us would consider that an improvement over the actual approach of having a separate file with that information. If it's not a good fit for pg_hba.conf, why is it a good fit for anything else we want to do? -- 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] Minmax indexes
On 17 September 2013 22:03, Alvaro Herrera alvhe...@2ndquadrant.com wrote: Thom Brown wrote: Thanks for testing. Thanks for the patch, but I seem to have immediately hit a snag: pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax (aid); PANIC: invalid xlog record length 0 Silly mistake I had already made in another patch. Here's an incremental patch which fixes this bug. Apply this on top of previous minmax-1.patch. Thanks. Hit another issue with exactly the same procedure: pgbench=# create index minmaxtest on pgbench_accounts using minmax (aid); ERROR: lock 176475 is not held -- Thom
Re: [HACKERS] relscan_details.h
Robert Haas escribió: Personally, I'm not particularly in favor of these kinds of changes. The changes we made last time broke a lot of extensions - including some proprietary EDB ones that I had to go fix. I think a lot of people spent a lot of time fixing broken builds, at EDB and elsewhere, as well as rebasing patches. And the only benefit we have to balance that out is that incremental recompiles are faster, and I'm not really sure how important that actually is. On my system, configure takes 25 seconds and make -j3 takes 65 seconds; so, even a full recompile is pretty darn fast, and an incremental recompile is usually really fast. Now granted this is a relatively new system, but still. Fortunately the machines I work on now are also reasonably fast. There was a time when my desktop was so slow that it paid off to tweak certain file timestamps to avoid spurious recompiles. Now I don't have to worry. But it still annoys me that I have enough time to context-switch to, say, the email client or web browser, from where I don't switch back so quickly; which means I waste five or ten minutes for a task that should have taken 20 seconds. Now, htup_details.h was a bit different than the case at hand because there's evidently lots of code that want to deal with the guts of tuples, but for scans you mainly want to start one, iterate and finish, but don't care much about the innards. So the cleanup work required is going to be orders of magnitude smaller. OTOH, back then we had the alternative of doing the split in such a way that third-party code wouldn't have been affected that heavily, but we failed to take that into consideration. I trust we have all learned that lesson and will keep it in mind for next time. I don't want to be too dogmatic in opposing this; I accept that we should, from time to time, refactor things. If we don't, superflouous dependencies will probably proliferate over time. But personally, I'd rather do these changes in bulk every third release or so and keep them to a minimum in between times, so that we're not forcing people to constantly decorate their extensions with new #if directives. We can batch things, sure, but I don't think doing it only once every three years is the right tradeoff. There's not all that much of this refactoring, after all. -- Álvaro Herrerahttp://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] Minmax indexes
Thom Brown wrote: Thanks for testing. Thanks for the patch, but I seem to have immediately hit a snag: pgbench=# CREATE INDEX minmaxtest ON pgbench_accounts USING minmax (aid); PANIC: invalid xlog record length 0 Silly mistake I had already made in another patch. Here's an incremental patch which fixes this bug. Apply this on top of previous minmax-1.patch. I also renumbered the duplicate OID pointed out by Peter, and fixed the two compiler warnings reported by Jaime. Note you'll need to re-initdb in order to get the right catalog entries. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services diff --git a/src/backend/access/minmax/mmrevmap.c b/src/backend/access/minmax/mmrevmap.c index 3e19f90..76cddde 100644 --- a/src/backend/access/minmax/mmrevmap.c +++ b/src/backend/access/minmax/mmrevmap.c @@ -147,12 +147,10 @@ mmSetHeapBlockItemptr(mmRevmapAccess *rmAccess, BlockNumber heapBlk, { xl_minmax_rm_set xlrec; XLogRecPtr recptr; - XLogRecData rdata; + XLogRecData rdata[2]; uint8 info; info = XLOG_MINMAX_REVMAP_SET; - if (extend) - info |= XLOG_MINMAX_INIT_PAGE; xlrec.node = rmAccess-idxrel-rd_node; xlrec.mapBlock = mapBlk; @@ -160,13 +158,26 @@ mmSetHeapBlockItemptr(mmRevmapAccess *rmAccess, BlockNumber heapBlk, xlrec.heapBlock = heapBlk; ItemPointerSet((xlrec.newval), blkno, offno); - rdata.data = (char *) xlrec; - rdata.len = SizeOfMinmaxRevmapSet; - rdata.buffer = rmAccess-currBuf; - rdata.buffer_std = false; - rdata.next = NULL; + rdata[0].data = (char *) xlrec; + rdata[0].len = SizeOfMinmaxRevmapSet; + rdata[0].buffer = InvalidBuffer; + rdata[0].buffer_std = false; + rdata[0].next = (rdata[1]); + + rdata[1].data = NULL; + rdata[1].len = 0; + rdata[1].buffer = rmAccess-currBuf; + rdata[1].buffer_std = false; + rdata[1].next = NULL; + + if (extend) + { + info |= XLOG_MINMAX_INIT_PAGE; + /* If the page is new, there's no need for a full page image */ + rdata[0].next = NULL; + } - recptr = XLogInsert(RM_MINMAX_ID, info, rdata); + recptr = XLogInsert(RM_MINMAX_ID, info, rdata); PageSetLSN(BufferGetPage(rmAccess-currBuf), recptr); } diff --git a/src/backend/access/minmax/mmxlog.c b/src/backend/access/minmax/mmxlog.c index ee095a2..758fc5f 100644 --- a/src/backend/access/minmax/mmxlog.c +++ b/src/backend/access/minmax/mmxlog.c @@ -158,7 +158,6 @@ minmax_xlog_revmap_set(XLogRecPtr lsn, XLogRecord *record) { xl_minmax_rm_set *xlrec = (xl_minmax_rm_set *) XLogRecGetData(record); bool init; - BlockNumber blkno; Buffer buffer; Page page; diff --git a/src/backend/storage/page/bufpage.c b/src/backend/storage/page/bufpage.c index 1e7cbac..d1a3ca7 100644 --- a/src/backend/storage/page/bufpage.c +++ b/src/backend/storage/page/bufpage.c @@ -1040,6 +1040,7 @@ PageIndexDeleteNoCompact(Page page, OffsetNumber *itemnos, int nitems) * page. */ memcpy(pageCopy, page, BLCKSZ); + lastused = FirstOffsetNumber; upper = pd_special; PageClearHasFreeLinePointers(page); for (i = 0, itemidptr = itemidbase; i nline; i++, itemidptr++) diff --git a/src/include/catalog/pg_amop.h b/src/include/catalog/pg_amop.h index 8109949..192e295 100644 --- a/src/include/catalog/pg_amop.h +++ b/src/include/catalog/pg_amop.h @@ -784,28 +784,28 @@ DATA(insert ( 3474 3831 3831 18 s 3882 4000 0 )); /* * MinMax int4_ops */ -DATA(insert ( 3177 23 23 1 s 97 403 0 )); -DATA(insert ( 3177 23 23 2 s 523 403 0 )); -DATA(insert ( 3177 23 23 3 s 96 403 0 )); -DATA(insert ( 3177 23 23 4 s 525 403 0 )); -DATA(insert ( 3177 23 23 5 s 521 403 0 )); +DATA(insert ( 3192 23 23 1 s 97 3847 0 )); +DATA(insert ( 3192 23 23 2 s 523 3847 0 )); +DATA(insert ( 3192 23 23 3 s 96 3847 0 )); +DATA(insert ( 3192 23 23 4 s 525 3847 0 )); +DATA(insert ( 3192 23 23 5 s 521 3847 0 )); /* * MinMax numeric_ops */ -DATA(insert ( 3192 1700 1700 1 s 1754 403 0 )); -DATA(insert ( 3192 1700 1700 2 s 1755 403 0 )); -DATA(insert ( 3192 1700 1700 3 s 1752 403 0 )); -DATA(insert ( 3192 1700 1700 4 s 1757 403 0 )); -DATA(insert ( 3192 1700 1700 5 s 1756 403 0 )); +DATA(insert ( 3193 1700 1700 1 s 1754 3847 0 )); +DATA(insert ( 3193 1700 1700 2 s 1755 3847 0 )); +DATA(insert ( 3193 1700 1700 3 s 1752 3847 0 )); +DATA(insert ( 3193 1700 1700 4 s 1757 3847 0 )); +DATA(insert ( 3193 1700 1700 5 s 1756 3847 0 )); /* * MinMax text_ops */ -DATA(insert ( 3193 25 25 1 s 664 403 0 )); -DATA(insert ( 3193 25 25 2 s 665 403 0 )); -DATA(insert ( 3193 25 25 3 s 98 403 0 )); -DATA(insert ( 3193 25 25 4 s 667 403 0 )); -DATA(insert ( 3193 25 25 5 s 666 403 0 )); +DATA(insert ( 3194 25 25 1 s 664 3847 0 )); +DATA(insert ( 3194 25 25 2 s 665 3847 0 )); +DATA(insert ( 3194 25 25 3 s 98 3847 0 )); +DATA(insert ( 3194 25 25 4 s 667 3847 0 )); +DATA(insert ( 3194 25 25 5 s 666 3847 0 )); #endif /* PG_AMOP_H */ diff --git a/src/include/catalog/pg_amproc.h
Re: [HACKERS] Minmax indexes
On Tue, September 17, 2013 23:03, Alvaro Herrera wrote: [minmax-1.patch. + minmax-2-incr.patch. (and initdb)] The patches apply and compile OK. I've not yet really tested; I just wanted to mention that make check gives the following differences: *** /home/aardvark/pg_stuff/pg_sandbox/pgsql.minmax/src/test/regress/expected/opr_sanity.out 2013-09-17 23:18:31.427356703 +0200 --- /home/aardvark/pg_stuff/pg_sandbox/pgsql.minmax/src/test/regress/results/opr_sanity.out 2013-09-17 23:20:48.208150824 +0200 *** *** 1076,1081 --- 1076,1086 2742 |2 | @@@ 2742 |3 | @ 2742 |4 | = +3847 |1 | +3847 |2 | = +3847 |3 | = +3847 |4 | = +3847 |5 | 4000 |1 | 4000 |1 | ~~ 4000 |2 | *** *** 1098,1104 4000 | 15 | 4000 | 16 | @ 4000 | 18 | = ! (62 rows) -- Check that all opclass search operators have selectivity estimators. -- This is not absolutely required, but it seems a reasonable thing --- 1103,1109 4000 | 15 | 4000 | 16 | @ 4000 | 18 | = ! (67 rows) -- Check that all opclass search operators have selectivity estimators. -- This is not absolutely required, but it seems a reasonable thing *** *** 1272,1280 WHERE am.amname 'btree' AND am.amname 'gist' AND am.amname 'gin' GROUP BY amname, amsupport, opcname, amprocfamily HAVING count(*) != amsupport OR amprocfamily IS NULL; ! amname | opcname | count ! +-+--- ! (0 rows) SELECT amname, opcname, count(*) FROM pg_am am JOIN pg_opclass op ON opcmethod = am.oid --- 1277,1288 WHERE am.amname 'btree' AND am.amname 'gist' AND am.amname 'gin' GROUP BY amname, amsupport, opcname, amprocfamily HAVING count(*) != amsupport OR amprocfamily IS NULL; ! amname | opcname | count ! +-+--- ! minmax | int4_ops| 1 ! minmax | text_ops| 1 ! minmax | numeric_ops | 1 ! (3 rows) SELECT amname, opcname, count(*) FROM pg_am am JOIN pg_opclass op ON opcmethod = am.oid == Erik Rijkers -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Support for REINDEX CONCURRENTLY
On Mon, Sep 16, 2013 at 10:38 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-08-29 10:39:09 -0400, Robert Haas wrote: I have been of the opinion for some time now that the shared-invalidation code is not a particularly good design for much of what we need. Waiting for an old snapshot is often a proxy for waiting long enough that we can be sure every other backend will process the shared-invalidation message before it next uses any of the cached data that will be invalidated by that message. However, it would be better to be able to send invalidation messages in some way that causes them to processed more eagerly by other backends, and that provides some more specific feedback on whether or not they have actually been processed. Then we could send the invalidation messages, wait just until everyone confirms that they have been seen, which should hopefully happen quickly, and then proceed. Actually, the shared inval code already has that knowledge, doesn't it? ISTM all we'd need is have a sequence number of SI entries which has to be queryable. Then one can simply wait till all backends have consumed up to that id which we keep track of the furthest back backend in shmem. In theory, yes, but in practice, there are a few difficulties. 1. We're not in a huge hurry to ensure that sinval notifications are delivered in a timely fashion. We know that sinval resets are bad, so if a backend is getting close to needing a sinval reset, we kick it in an attempt to get it to AcceptInvalidationMessages(). But if the sinval queue isn't filling up, there's no upper bound on the amount of time that can pass before a particular sinval is read. Therefore, the amount of time that passes before an idle backend is forced to drain the sinval queue can vary widely, from a fraction of a second to minutes, hours, or days. So it's kind of unappealing to think about making user-visible behavior dependent on how long it ends up taking. 2. Every time we add a new kind of sinval message, we increase the frequency of sinval resets, and those are bad. So any notifications that we choose to send this way had better be pretty low-volume. Considering the foregoing points, it's unclear to me whether we should try to improve sinval incrementally or replace it with something completely new. I'm sure that the above-mentioned problems are solvable, but I'm not sure how hairy it will be. On the other hand, designing something new could be pretty hairy, too. -- 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] Minmax indexes
Thom Brown wrote: Hit another issue with exactly the same procedure: pgbench=# create index minmaxtest on pgbench_accounts using minmax (aid); ERROR: lock 176475 is not held That's what I get for restructuring the way buffers are acquired to use the FSM, and then neglecting to test creation on decently-sized indexes. Fix attached. I just realized that xlog replay is also broken. -- Álvaro Herrerahttp://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training Services diff --git a/src/backend/access/minmax/minmax.c b/src/backend/access/minmax/minmax.c index 3b41100..47cb05e 100644 --- a/src/backend/access/minmax/minmax.c +++ b/src/backend/access/minmax/minmax.c @@ -1510,10 +1510,8 @@ mm_getinsertbuffer(Relation irel, Buffer *buffer, Size itemsz) } if (!BufferIsInvalid(*buffer)) - { - MarkBufferDirty(*buffer); - UnlockReleaseBuffer(*buffer); - } + ReleaseBuffer(*buffer); + *buffer = buf; } else -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Assertions in PL/PgSQL
On 2013-09-16 21:24, Pavel Stehule wrote: 2. a failed assert should to raise a exception, that should not be handled by any exception handler - similar to ERRCODE_QUERY_CANCELED - see exception_matches_conditions. I'm not sure what I think about that idea. I see decent arguments for it working either way. Care to unravel yours a bit more? Regards, Marko Tiikkaja -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Minmax indexes
Erik Rijkers wrote: On Tue, September 17, 2013 23:03, Alvaro Herrera wrote: [minmax-1.patch. + minmax-2-incr.patch. (and initdb)] The patches apply and compile OK. I've not yet really tested; I just wanted to mention that make check gives the following differences: Oops, I forgot to update the expected file. I had to comment on this when submitting minmax-2-incr.patch and forgot. First, those extra five operators are supposed to be there; expected file needs an update. As for this: --- 1277,1288 WHERE am.amname 'btree' AND am.amname 'gist' AND am.amname 'gin' GROUP BY amname, amsupport, opcname, amprocfamily HAVING count(*) != amsupport OR amprocfamily IS NULL; ! amname | opcname | count ! +-+--- ! minmax | int4_ops| 1 ! minmax | text_ops| 1 ! minmax | numeric_ops | 1 ! (3 rows) I think the problem is that the query is wrong. This is the complete query: SELECT amname, opcname, count(*) FROM pg_am am JOIN pg_opclass op ON opcmethod = am.oid LEFT JOIN pg_amproc p ON amprocfamily = opcfamily AND amproclefttype = amprocrighttype AND amproclefttype = opcintype WHERE am.amname 'btree' AND am.amname 'gist' AND am.amname 'gin' GROUP BY amname, amsupport, opcname, amprocfamily HAVING count(*) != amsupport OR amprocfamily IS NULL; I should be, instead, this: SELECT amname, opcname, count(*) FROM pg_am am JOIN pg_opclass op ON opcmethod = am.oid LEFT JOIN pg_amproc p ON amprocfamily = opcfamily AND amproclefttype = amprocrighttype AND amproclefttype = opcintype WHERE am.amname 'btree' AND am.amname 'gist' AND am.amname 'gin' GROUP BY amname, amsupport, opcname, amprocfamily HAVING count(*) != amsupport AND (amprocfamily IS NOT NULL); This query is supposed to check that there are no opclasses with mismatching number of support procedures; but if the left join returns a null-extended row for pg_amproc, that means there is no support proc, yet count(*) will return 1. So count(*) will not match amsupport, and the row is supposed to be excluded by the amprocfamily IS NULL clause in HAVING. Both queries return empty in HEAD, but only the second one correctly returns empty with the patch applied. -- Álvaro Herrerahttp://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] Updatable view columns
On 2013-09-17 12:53, Dean Rasheed wrote: Thanks for the review. Those changes all look sensible to me. Here's an updated patch incorporating all your fixes, and rebased to apply without offsets. Looks good to me. Marking this one ready for committer. Regards, Marko Tiikkaja -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] [PERFORM] encouraging index-only scans
On 2013-09-17 11:37:35 -0500, Jim Nasby wrote: On 9/7/13 12:34 AM, Andres Freund wrote: What I was thinking of was to keep track of the oldest xids on pages that cannot be marked all visible. I haven't thought about the statistics part much, but what if we binned the space between [RecentGlobalXmin, -nextXid) into 10 bins and counted the number of pages falling into each bin. Then after the vacuum finished we could compute how far RecentGlobalXmin would have to progress to make another vacuum worthwile by counting the number of pages from the lowest bin upwards and use the bin's upper limit as the triggering xid. Now, we'd definitely need to amend that scheme by something that handles pages that are newly written to, but it seems something like that wouldn't be too hard to implement and would make autovacuum more useful. If we're binning by XID though you're still dependent on scanning to build that range. Anything that creates dead tuples will also be be problematic, because it's going to unset VM bits on you, and you won't know if it's due to INSERTS or dead tuples. I don't think that's all that much of a problem. In the end, it's a good idea to look at pages shortly after they have been filled/been touched. Setting hint bits at that point avoid repetitive IO and in many cases we will already be able to mark them all-visible. The binning idea was really about sensibly estimating whether a new scan already makes sense which is currently very hard to judge. I generally think the current logic for triggering VACUUMs via autovacuum doesn't really make all that much sense in the days where we have the visibility map. What if we maintained XID stats for ranges of pages in a separate fork? Call it the XidStats fork. Presumably the interesting pieces would be min(xmin) and max(xmax) for pages that aren't all visible. If we did that at a granularity of, say, 1MB worth of pages[1] we're talking 8 bytes per MB, or 1 XidStats page per GB of heap. (Worst case alignment bumps that up to 2 XidStats pages per GB of heap.) Yes, I have thought about similar ideas as well, but I came to the conclusion that it's not worth it. If you want to make the boundaries precise and the xidstats fork small, you're introducing new contention points because every DML will need to make sure it's correct. Also, the amount of code that would require seems to be bigger than justified by the increase of precision when to vacuum. 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] Support for REINDEX CONCURRENTLY
On 2013-09-17 16:34:37 -0400, Robert Haas wrote: On Mon, Sep 16, 2013 at 10:38 AM, Andres Freund and...@2ndquadrant.com wrote: Actually, the shared inval code already has that knowledge, doesn't it? ISTM all we'd need is have a sequence number of SI entries which has to be queryable. Then one can simply wait till all backends have consumed up to that id which we keep track of the furthest back backend in shmem. In theory, yes, but in practice, there are a few difficulties. Agreed ;) 1. We're not in a huge hurry to ensure that sinval notifications are delivered in a timely fashion. We know that sinval resets are bad, so if a backend is getting close to needing a sinval reset, we kick it in an attempt to get it to AcceptInvalidationMessages(). But if the sinval queue isn't filling up, there's no upper bound on the amount of time that can pass before a particular sinval is read. Therefore, the amount of time that passes before an idle backend is forced to drain the sinval queue can vary widely, from a fraction of a second to minutes, hours, or days. So it's kind of unappealing to think about making user-visible behavior dependent on how long it ends up taking. Well, when we're signalling it's certainly faster than waiting for the other's snapshot to vanish which can take ages for normal backends. And we can signal when we wait for consumption without too many problems. Also, I think in most of the usecases we can simply not wait for any of the idle backends, those don't use the old definition anyway. 2. Every time we add a new kind of sinval message, we increase the frequency of sinval resets, and those are bad. So any notifications that we choose to send this way had better be pretty low-volume. In pretty much all the cases where I can see the need for something like that, we already send sinval messages, so we should be able to piggbyback on those. Considering the foregoing points, it's unclear to me whether we should try to improve sinval incrementally or replace it with something completely new. I'm sure that the above-mentioned problems are solvable, but I'm not sure how hairy it will be. On the other hand, designing something new could be pretty hairy, too. I am pretty sure there's quite a bit to improve around sinvals but I think any replacement would look surprisingly similar to what we have. So I think doing it incrementally is more realistic. And I am certainly scared by the thought of having to replace it without breaking corner cases all over. 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] [RFC] Extend namespace of valid guc names
Hi, On 2013-09-17 16:24:34 -0400, Robert Haas wrote: On Tue, Sep 17, 2013 at 10:27 AM, Andres Freund and...@2ndquadrant.com wrote: On 2013-09-17 10:23:30 -0400, Robert Haas wrote: What is the use case for this change? Check http://archives.postgresql.org/message-id/20130225213400.GF3849%40awork2.anarazel.de or the commit message of the patch. That's more or less what I figured. One thing I'm uncomfortable with is that, while this is useful for extensions, what do we do when we have a similar requirement for core? The proposed GUC name is of the format extension.user_defined_object_name.property_name; but omitting the extension name would lead to user_defined_object_name.property_name, which doesn't feel good as a method for naming core GUCs. The user defined object name might just so happen to be an extension name. I think that ship has long since sailed. postgresql.conf has allowed foo.bar style GUCs via custom_variable_classes for a long time, and these days we don't even require that but allow them generally. Also, SET, postgres -c, and SELECT set_config() already don't have the restriction to one dot in the variable name. I think if we're going to use something like that for postgres, we'll have to live with the chance of breaking applications (although I don't think it's that big for most of the variables where I can forsee the use). If it's not a good fit for pg_hba.conf, why is it a good fit for anything else we want to do? Well, for now it's primarily there for extensions, not for core code. Although somebody has already asked when I'd submit the patch because they could use it for their patch... I don't really find the pg_hba.conf comparison terribly convincing. As an extension, how would you prefer to formulate the configuration in the example? 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] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE
On 2013-09-17 12:29:51 -0400, Robert Haas wrote: But I'm skeptical that you're going to be able to accomplish that, especially without adversely affecting maintainability. I think the way that you're proposing to use lwlocks here is sufficiently different from what the rest of the system does that it's going to be hard to avoid system-wide affects that can't easily be caught during code review; I actually think extending lwlocks to allow downgrading an exclusive lock is a good idea, independent of this path, and I think there are some areas of the code where we could use that capability to increase scalability. Now, that might be because I pretty much suggested using them in such a way to solve some of the problems :P I don't think they solve the issue of this patch (holding several nbtree pages locked across heap operations) though. I agree that unpredictable deadlocks are bad. I think the fundamental problem with UPSERT, MERGE, and this proposal is what happens when the conflicting tuple is present but not visible to your scan, either because it hasn't committed yet or because it has committed but is not visible to your snapshot. I'm not clear on how you handle that in your approach. Hm. I think it should be handled exactly the way we handle it for unique indexes today. Wait till it's clear whether you can proceed. At some point we might to extend that logic to more cases, but that should be separate discussion imo. 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 for fail-back without fresh backup
On Tue, Sep 17, 2013 at 9:52 PM, Fujii Masao masao.fu...@gmail.com wrote: On Tue, Sep 17, 2013 at 3:45 PM, Samrat Revagade revagade.sam...@gmail.com wrote: syncrep.c: In function ‘SyncRepReleaseWaiters’: syncrep.c:421:6: warning: variable ‘numdataflush’ set but not used [-Wunused-but-set-variable] Sorry I forgot fix it. I have attached the patch which I modified. Attached patch combines documentation patch and source-code patch. I set up synchronous replication with synchronous_transfer = all, and then I ran pgbench -i and executed CHECKPOINT in the master. After that, when I executed CHECKPOINT in the standby, it got stuck infinitely. I guess this was cased by synchronous_transfer feature. Did you set synchronous_standby_names in the standby server? If so, the master server waits for the standby server which is set to synchronous_standby_names. Please let me know detail of this case. How does synchronous_transfer work with cascade replication? If it's set to all in the sender-side standby, it can resolve the data page inconsistency between two standbys? Currently patch supports the case which two servers are set up SYNC replication. IWO, failback safe standby is the same as SYNC replication standby. User can set synchronous_transfer in only master side. Regards, --- Sawada Masahiko -- Sent 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 for fail-back without fresh backup
On Wed, Sep 18, 2013 at 10:35 AM, Sawada Masahiko sawada.m...@gmail.com wrote: On Tue, Sep 17, 2013 at 9:52 PM, Fujii Masao masao.fu...@gmail.com wrote: I set up synchronous replication with synchronous_transfer = all, and then I ran pgbench -i and executed CHECKPOINT in the master. After that, when I executed CHECKPOINT in the standby, it got stuck infinitely. I guess this was cased by synchronous_transfer feature. Did you set synchronous_standby_names in the standby server? Yes. If so, the master server waits for the standby server which is set to synchronous_standby_names. Please let me know detail of this case. Both master and standby have the same postgresql.conf settings as follows: max_wal_senders = 4 wal_level = hot_standby wal_keep_segments = 32 synchronous_standby_names = '*' synchronous_transfer = all How does synchronous_transfer work with cascade replication? If it's set to all in the sender-side standby, it can resolve the data page inconsistency between two standbys? Currently patch supports the case which two servers are set up SYNC replication. IWO, failback safe standby is the same as SYNC replication standby. User can set synchronous_transfer in only master side. So, it's very strange that CHECKPOINT on the standby gets stuck infinitely. Regards, -- Fujii Masao -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Minor inheritance/check bug: Inconsistent behavior
On Tue, Sep 17, 2013 at 3:29 PM, Rushabh Lathia rushabh.lat...@gmail.com wrote: Hi Amit. I gone through the mail thread discussion regarding this issue and reviewed you patch. -- Patch get applied cleanly on Master branch -- Make and Make Install fine -- make check also running cleanly In the patch code changes looks good to me. This patch having two part: 1) Allowed TableOid system column in the CHECK constraint 2) Throw an error if other then TableOid system column in CHECK constraint. I noticed that you added test coverage for 1) but the test coverage for 2) is missing.. Initially I thought of keeping the test for point-2 as well, but later left it thinking it might not add much value for adding negative test for this scenario. I added the test coverage for 2) in the attached patch. Thanks for adding new test. Marking this as Ready for committer. Thanks a ton for reviewing the patch. With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com On Sun, Sep 15, 2013 at 2:31 PM, Amit Kapila amit.kapil...@gmail.com wrote: Bruce Momjian wrote: On Sun, Jun 30, 2013 at 06:57:10AM +, Amit kapila wrote: I have done the initial analysis and prepared a patch, don't know if anything more I can do until someone can give any suggestions to further proceed on this bug. So, I guess we never figured this out. I can submit this bug-fix for next commitfest if there is no objection for doing so. What is your opinion? Yes, good idea. I had rebased the patch against head and added the test case to validate it. I will upload this patch to commit fest. -- Sent 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 for fail-back without fresh backup
On Wed, Sep 18, 2013 at 11:45 AM, Fujii Masao masao.fu...@gmail.com wrote: On Wed, Sep 18, 2013 at 10:35 AM, Sawada Masahiko sawada.m...@gmail.com wrote: On Tue, Sep 17, 2013 at 9:52 PM, Fujii Masao masao.fu...@gmail.com wrote: I set up synchronous replication with synchronous_transfer = all, and then I ran pgbench -i and executed CHECKPOINT in the master. After that, when I executed CHECKPOINT in the standby, it got stuck infinitely. I guess this was cased by synchronous_transfer feature. Did you set synchronous_standby_names in the standby server? Yes. If so, the master server waits for the standby server which is set to synchronous_standby_names. Please let me know detail of this case. Both master and standby have the same postgresql.conf settings as follows: max_wal_senders = 4 wal_level = hot_standby wal_keep_segments = 32 synchronous_standby_names = '*' synchronous_transfer = all How does synchronous_transfer work with cascade replication? If it's set to all in the sender-side standby, it can resolve the data page inconsistency between two standbys? Currently patch supports the case which two servers are set up SYNC replication. IWO, failback safe standby is the same as SYNC replication standby. User can set synchronous_transfer in only master side. So, it's very strange that CHECKPOINT on the standby gets stuck infinitely. yes I think so. I was not considering that user set synchronous_standby_names in the standby server. it will ocurr I will fix it considering this case. Regards, --- Sawada Masahiko -- Sent 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 for fail-back without fresh backup
On Wed, Sep 18, 2013 at 1:05 PM, Sawada Masahiko sawada.m...@gmail.com wrote: On Wed, Sep 18, 2013 at 11:45 AM, Fujii Masao masao.fu...@gmail.com wrote: On Wed, Sep 18, 2013 at 10:35 AM, Sawada Masahiko sawada.m...@gmail.com wrote: On Tue, Sep 17, 2013 at 9:52 PM, Fujii Masao masao.fu...@gmail.com wrote: I set up synchronous replication with synchronous_transfer = all, and then I ran pgbench -i and executed CHECKPOINT in the master. After that, when I executed CHECKPOINT in the standby, it got stuck infinitely. I guess this was cased by synchronous_transfer feature. Did you set synchronous_standby_names in the standby server? Yes. If so, the master server waits for the standby server which is set to synchronous_standby_names. Please let me know detail of this case. Both master and standby have the same postgresql.conf settings as follows: max_wal_senders = 4 wal_level = hot_standby wal_keep_segments = 32 synchronous_standby_names = '*' synchronous_transfer = all How does synchronous_transfer work with cascade replication? If it's set to all in the sender-side standby, it can resolve the data page inconsistency between two standbys? Currently patch supports the case which two servers are set up SYNC replication. IWO, failback safe standby is the same as SYNC replication standby. User can set synchronous_transfer in only master side. So, it's very strange that CHECKPOINT on the standby gets stuck infinitely. Sorry I sent mail by mistake. yes I think so. It waits for corresponding WAL replicated. Behaviour of synchronous_transfer is similar to synchronous_standby_names and synchronous replication little. That is, if those parameter is set but the standby server doesn't connect to the master server, the master server waits for corresponding WAL replicated to standby server infinitely. I was not considering that user set synchronous_standby_names in the standby server. I will fix it considering this case. Regards, --- Sawada Masahiko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Patch for typo in src/bin/psql/command.c
Attached. Regards Ian Barwick psql-command-c-typo.patch Description: Binary data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] pg_stat_statements: calls under-estimation propagation
You seem to have forgotten to include the pg_stat_statements--1.2.sql and pg_stat_statements--1.1--1.2.sql in the patch. Sorry again. Please find updated patch attached. I did not add pg_stat_statements--1.2.sql. I have added that now and updated the patch again. The patch attached should contain following file changes patching file contrib/pg_stat_statements/Makefile patching file contrib/pg_stat_statements/pg_stat_statements--1.1--1.2.sql patching file contrib/pg_stat_statements/pg_stat_statements--1.2.sql patching file contrib/pg_stat_statements/pg_stat_statements.c patching file contrib/pg_stat_statements/pg_stat_statements.control patching file doc/src/sgml/pgstatstatements.sgml regards Sameer pg_stat_statements-identification-v4.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] INSERT...ON DUPLICATE KEY LOCK FOR UPDATE
On Tue, Sep 17, 2013 at 6:20 PM, Andres Freund and...@2ndquadrant.com wrote: I agree that unpredictable deadlocks are bad. I think the fundamental problem with UPSERT, MERGE, and this proposal is what happens when the conflicting tuple is present but not visible to your scan, either because it hasn't committed yet or because it has committed but is not visible to your snapshot. I'm not clear on how you handle that in your approach. Hm. I think it should be handled exactly the way we handle it for unique indexes today. Wait till it's clear whether you can proceed. That's what I do, although getting those details right has been of secondary concern for obvious reasons. At some point we might to extend that logic to more cases, but that should be separate discussion imo. This is essentially why I went and added a row locking component over your objections. Value locks (regardless of implementation) effectively stop an insertion from finishing, but not from starting. ISTM that locking the row with value locks held can cause deadlock. So, unfortunately, we cannot really discuss value locking and row locking separately, even though I see the appeal of trying to. Gaining an actual representative notion of the expense of releasing and re-acquiring the locks is too tightly coupled with how this is handled and how frequently we need to restart. Plus there may well be other issues in the same vein that we've yet to consider. -- 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